path: root/drivers
diff options
authorLinus Torvalds <torvalds@linux-foundation.org>2019-01-04 12:56:09 -0800
committerLinus Torvalds <torvalds@linux-foundation.org>2019-01-04 12:56:09 -0800
commit594cc251fdd0d231d342d88b2fdff4bc42fb0690 (patch)
tree259269a399e6504a7cf8739201cf172d1cbb197a /drivers
parent0b2c8f8b6b0c7530e2866c95862546d0da2057b0 (diff)
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. Signed-off-by: Linus Torvalds <torvalds@linux-foundation.org>
Diffstat (limited to 'drivers')
1 files changed, 13 insertions, 2 deletions
diff --git a/drivers/gpu/drm/i915/i915_gem_execbuffer.c b/drivers/gpu/drm/i915/i915_gem_execbuffer.c
index 55d8f9b8777f..485b259127c3 100644
--- a/drivers/gpu/drm/i915/i915_gem_execbuffer.c
+++ b/drivers/gpu/drm/i915/i915_gem_execbuffer.c
@@ -1624,7 +1624,9 @@ end_user:
* happened we would make the mistake of assuming that the
* relocations were valid.
- user_access_begin();
+ if (!user_access_begin(urelocs, size))
+ goto end_user;
for (copied = 0; copied < nreloc; copied++)
@@ -2606,7 +2608,16 @@ i915_gem_execbuffer2_ioctl(struct drm_device *dev, void *data,
unsigned int i;
/* Copy the new buffer offsets back to the user's exec list. */
- user_access_begin();
+ /*
+ * Note: count * sizeof(*user_exec_list) does not overflow,
+ * because we checked 'count' in check_buffer_count().
+ *
+ * And this range already got effectively checked earlier
+ * when we did the "copy_from_user()" above.
+ */
+ if (!user_access_begin(user_exec_list, count * sizeof(*user_exec_list)))
+ goto end_user;
for (i = 0; i < args->buffer_count; i++) {
if (!(exec2_list[i].offset & UPDATE))