On Tue, Jun 11, 2019 at 4:38 PM Andrey Konovalov andreyknvl@google.com wrote:
On Sat, Jun 8, 2019 at 6:02 AM Kees Cook keescook@chromium.org wrote:
On Mon, Jun 03, 2019 at 06:55:10PM +0200, Andrey Konovalov wrote:
This patch is a part of a series that extends arm64 kernel ABI to allow to pass tagged user pointers (with the top byte set to something else other than 0x00) as syscall arguments.
In copy_mount_options a user address is being subtracted from TASK_SIZE. If the address is lower than TASK_SIZE, the size is calculated to not allow the exact_copy_from_user() call to cross TASK_SIZE boundary. However if the address is tagged, then the size will be calculated incorrectly.
Untag the address before subtracting.
Reviewed-by: Catalin Marinas catalin.marinas@arm.com Signed-off-by: Andrey Konovalov andreyknvl@google.com
One thing I just noticed in the commit titles... "arm64" is in the prefix, but these are arch-indep areas. Should the ", arm64" be left out?
I would expect, instead:
fs/namespace: untag user pointers in copy_mount_optionsHm, I've added the arm64 tag in all of the patches because they are related to changes in arm64 kernel ABI. I can remove it from all the patches that only touch common code if you think that it makes sense.
I'll keep the arm64 tags in commit titles for v17. Please reply explicitly if you think I should remove them. Thanks! :)
Thanks!
Reviewed-by: Kees Cook keescook@chromium.org
-Kees
fs/namespace.c | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-)
diff --git a/fs/namespace.c b/fs/namespace.c index b26778bdc236..2e85712a19ed 100644 --- a/fs/namespace.c +++ b/fs/namespace.c @@ -2993,7 +2993,7 @@ void *copy_mount_options(const void __user * data) * the remainder of the page. */ /* copy_from_user cannot cross TASK_SIZE ! */
size = TASK_SIZE - (unsigned long)data;
size = TASK_SIZE - (unsigned long)untagged_addr(data); if (size > PAGE_SIZE) size = PAGE_SIZE;-- 2.22.0.rc1.311.g5d7573a151-goog
-- Kees Cook