From 660716ee5f6d3a650b5af8c58951b0aef43f5c47 Mon Sep 17 00:00:00 2001 From: Linus Torvalds Date: Fri, 4 Jan 2019 12:56:09 -0800 Subject: [PATCH] BACKPORT: make 'user_access_begin()' do 'access_ok()' Originally, the rule used to be that you'd have to do access_ok() separately, and then user_access_begin() before actually doing the direct (optimized) user access. But experience has shown that people then decide not to do access_ok() at all, and instead rely on it being implied by other operations or similar. Which makes it very hard to verify that the access has actually been range-checked. If you use the unsafe direct user accesses, hardware features (either SMAP - Supervisor Mode Access Protection - on x86, or PAN - Privileged Access Never - on ARM) do force you to use user_access_begin(). But nothing really forces the range check. By putting the range check into user_access_begin(), we actually force people to do the right thing (tm), and the range check vill be visible near the actual accesses. We have way too long a history of people trying to avoid them. Bug 200630541 CVE-2018-20669 Signed-off-by: Linus Torvalds Change-Id: I1d25229cd41495b782e6f2bc67a4bb638dec4540 Reviewed-on: https://git-master.nvidia.com/r/c/linux-4.9/+/2374142 (cherry picked from commit e77a94059008f0f812474d05a9f75accecaeb583) Reviewed-on: https://git-master.nvidia.com/r/c/linux-4.9/+/2466485 Reviewed-by: Bibek Basu Reviewed-by: mobile promotions GVS: Gerrit_Virtual_Submit Tested-by: Amulya Yarlagadda Tested-by: mobile promotions --- include/linux/uaccess.h | 2 +- lib/strncpy_from_user.c | 9 +++++---- lib/strnlen_user.c | 18 ++++++++++-------- 3 files changed, 16 insertions(+), 13 deletions(-) diff --git a/include/linux/uaccess.h b/include/linux/uaccess.h index cc5ba47062e8..5329c868136b 100644 --- a/include/linux/uaccess.h +++ b/include/linux/uaccess.h @@ -141,7 +141,7 @@ extern long strnlen_unsafe_user(const void __user *unsafe_addr, long count); probe_kernel_read(&retval, addr, sizeof(retval)) #ifndef user_access_begin -#define user_access_begin() do { } while (0) +#define user_access_begin(ptr,len) access_ok(VERIFY_READ, ptr, len) #define user_access_end() do { } while (0) #define unsafe_get_user(x, ptr, err) do { if (unlikely(__get_user(x, ptr))) goto err; } while (0) #define unsafe_put_user(x, ptr, err) do { if (unlikely(__put_user(x, ptr))) goto err; } while (0) diff --git a/lib/strncpy_from_user.c b/lib/strncpy_from_user.c index 5a07f19059c3..54df8bccf7e8 100644 --- a/lib/strncpy_from_user.c +++ b/lib/strncpy_from_user.c @@ -114,10 +114,11 @@ long strncpy_from_user(char *dst, const char __user *src, long count) kasan_check_write(dst, count); check_object_size(dst, count, false); - user_access_begin(); - retval = do_strncpy_from_user(dst, src, count, max); - user_access_end(); - return retval; + if (user_access_begin(src, max)) { + retval = do_strncpy_from_user(dst, src, count, max); + user_access_end(); + return retval; + } } return -EFAULT; } diff --git a/lib/strnlen_user.c b/lib/strnlen_user.c index 9ff4f3bbb1aa..822b39e5c1bc 100644 --- a/lib/strnlen_user.c +++ b/lib/strnlen_user.c @@ -113,10 +113,11 @@ long strnlen_user(const char __user *str, long count) unsigned long max = max_addr - src_addr; long retval; - user_access_begin(); - retval = do_strnlen_user(str, count, max); - user_access_end(); - return retval; + if (user_access_begin(str, max)) { + retval = do_strnlen_user(str, count, max); + user_access_end(); + return retval; + } } return 0; } @@ -147,10 +148,11 @@ long strlen_user(const char __user *str) unsigned long max = max_addr - src_addr; long retval; - user_access_begin(); - retval = do_strnlen_user(str, ~0ul, max); - user_access_end(); - return retval; + if (user_access_begin(str, max)) { + retval = do_strnlen_user(str, ~0ul, max); + user_access_end(); + return retval; + } } return 0; }