Skip to content

Commit 13c7e3f

Browse files
torvaldsExactExampl
authored andcommitted
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 <torvalds@linux-foundation.org> 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 <bbasu@nvidia.com> Reviewed-by: mobile promotions <svcmobile_promotions@nvidia.com> GVS: Gerrit_Virtual_Submit Tested-by: Amulya Yarlagadda <ayarlagadda@nvidia.com> Tested-by: mobile promotions <svcmobile_promotions@nvidia.com>
1 parent 086ed23 commit 13c7e3f

3 files changed

Lines changed: 16 additions & 13 deletions

File tree

include/linux/uaccess.h

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -145,7 +145,7 @@ extern long strnlen_unsafe_user(const void __user *unsafe_addr, long count);
145145
probe_kernel_read(&retval, addr, sizeof(retval))
146146

147147
#ifndef user_access_begin
148-
#define user_access_begin() do { } while (0)
148+
#define user_access_begin(ptr,len) access_ok(VERIFY_READ, ptr, len)
149149
#define user_access_end() do { } while (0)
150150
#define unsafe_get_user(x, ptr, err) do { if (unlikely(__get_user(x, ptr))) goto err; } while (0)
151151
#define unsafe_put_user(x, ptr, err) do { if (unlikely(__put_user(x, ptr))) goto err; } while (0)

lib/strncpy_from_user.c

Lines changed: 5 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -116,10 +116,11 @@ long strncpy_from_user(char *dst, const char __user *src, long count)
116116

117117
kasan_check_write(dst, count);
118118
check_object_size(dst, count, false);
119-
user_access_begin();
120-
retval = do_strncpy_from_user(dst, src, count, max);
121-
user_access_end();
122-
return retval;
119+
if (user_access_begin(src, max)) {
120+
retval = do_strncpy_from_user(dst, src, count, max);
121+
user_access_end();
122+
return retval;
123+
}
123124
}
124125
return -EFAULT;
125126
}

lib/strnlen_user.c

Lines changed: 10 additions & 8 deletions
Original file line numberDiff line numberDiff line change
@@ -115,10 +115,11 @@ long strnlen_user(const char __user *str, long count)
115115
unsigned long max = max_addr - src_addr;
116116
long retval;
117117

118-
user_access_begin();
119-
retval = do_strnlen_user(str, count, max);
120-
user_access_end();
121-
return retval;
118+
if (user_access_begin(str, max)) {
119+
retval = do_strnlen_user(str, count, max);
120+
user_access_end();
121+
return retval;
122+
}
122123
}
123124
return 0;
124125
}
@@ -149,10 +150,11 @@ long strlen_user(const char __user *str)
149150
unsigned long max = max_addr - src_addr;
150151
long retval;
151152

152-
user_access_begin();
153-
retval = do_strnlen_user(str, ~0ul, max);
154-
user_access_end();
155-
return retval;
153+
if (user_access_begin(str, max)) {
154+
retval = do_strnlen_user(str, ~0ul, max);
155+
user_access_end();
156+
return retval;
157+
}
156158
}
157159
return 0;
158160
}

0 commit comments

Comments
 (0)