Hi,
I suggest to include the commit: 594cc251fdd0 make 'user_access_begin()' do 'access_ok()' for CVE-2018-20669.
stable version to apply to: kernel-4.14.y and kernel-4.19.y.
From the discussion below, I checked the latest kernel and found that we should also apply other 4 patches. (total 5 patches) https://lkml.org/lkml/2020/5/12/943
patch list: commit ab10ae1c3bef lib: Reduce user_access_begin() boundaries in strncpy_from_user() and strnlen_user() commit 6e693b3ffecb x86: uaccess: Inhibit speculation past access_ok() in user_access_begin() commit 9cb2feb4d21d arch/openrisc: Fix issues with access_ok() commit 94bd8a05cd4d Fix 'acccess_ok()' on alpha and SH commit 594cc251fdd0 make 'user_access_begin()' do 'access_ok()'
Where only commit 6e693b3ffecb does not need backport modifications. I attach my backport patches in this email.
I merged the patches with kernel-4.19.127 and kernel-4.14.183 without conflicts. Build with arm64 defconfig and bootup on arm64 QEMU environment.
cheers, Miles
On Thu, Jun 11, 2020 at 01:58:20AM +0800, Miles Chen wrote:
Hi,
I suggest to include the commit: 594cc251fdd0 make 'user_access_begin()' do 'access_ok()' for CVE-2018-20669.
stable version to apply to: kernel-4.14.y and kernel-4.19.y.
From the discussion below, I checked the latest kernel and found that we should also apply other 4 patches. (total 5 patches) https://lkml.org/lkml/2020/5/12/943
patch list: commit ab10ae1c3bef lib: Reduce user_access_begin() boundaries in strncpy_from_user() and strnlen_user() commit 6e693b3ffecb x86: uaccess: Inhibit speculation past access_ok() in user_access_begin() commit 9cb2feb4d21d arch/openrisc: Fix issues with access_ok() commit 94bd8a05cd4d Fix 'acccess_ok()' on alpha and SH commit 594cc251fdd0 make 'user_access_begin()' do 'access_ok()'
Where only commit 6e693b3ffecb does not need backport modifications. I attach my backport patches in this email.
I merged the patches with kernel-4.19.127 and kernel-4.14.183 without conflicts. Build with arm64 defconfig and bootup on arm64 QEMU environment.
cheers, Miles
From ac351de9ddd86ef717a3f89236dc5f6b2a108cc7 Mon Sep 17 00:00:00 2001 From: Linus Torvalds torvalds@linux-foundation.org Date: Fri, 4 Jan 2019 12:56:09 -0800 Subject: [PATCH] BACKPORT: make 'user_access_begin()' do 'access_ok()'
upstream commit 594cc251fdd0 ("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: 135368228 Change-Id: I4ca0e4566ea080fa148c5e768bb1a0b6f7201c01 Signed-off-by: Linus Torvalds torvalds@linux-foundation.org
No need for "Bug:" or "Change-Id:" for patches for stable trees.
Also, can you please sign off on these as well?
Can you fix that up and resend? I'll be glad to queue them up then.
thanks,
greg k-h
On Wed, 2020-06-10 at 20:02 +0200, Greg KH wrote:
On Thu, Jun 11, 2020 at 01:58:20AM +0800, Miles Chen wrote:
Hi,
I suggest to include the commit: 594cc251fdd0 make 'user_access_begin()' do 'access_ok()' for CVE-2018-20669.
stable version to apply to: kernel-4.14.y and kernel-4.19.y.
From the discussion below, I checked the latest kernel and found that we should also apply other 4 patches. (total 5 patches) https://lkml.org/lkml/2020/5/12/943
patch list: commit ab10ae1c3bef lib: Reduce user_access_begin() boundaries in strncpy_from_user() and strnlen_user() commit 6e693b3ffecb x86: uaccess: Inhibit speculation past access_ok() in user_access_begin() commit 9cb2feb4d21d arch/openrisc: Fix issues with access_ok() commit 94bd8a05cd4d Fix 'acccess_ok()' on alpha and SH commit 594cc251fdd0 make 'user_access_begin()' do 'access_ok()'
Where only commit 6e693b3ffecb does not need backport modifications. I attach my backport patches in this email.
I merged the patches with kernel-4.19.127 and kernel-4.14.183 without conflicts. Build with arm64 defconfig and bootup on arm64 QEMU environment.
cheers, Miles
From ac351de9ddd86ef717a3f89236dc5f6b2a108cc7 Mon Sep 17 00:00:00 2001 From: Linus Torvalds torvalds@linux-foundation.org Date: Fri, 4 Jan 2019 12:56:09 -0800 Subject: [PATCH] BACKPORT: make 'user_access_begin()' do 'access_ok()'
upstream commit 594cc251fdd0 ("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: 135368228 Change-Id: I4ca0e4566ea080fa148c5e768bb1a0b6f7201c01 Signed-off-by: Linus Torvalds torvalds@linux-foundation.org
No need for "Bug:" or "Change-Id:" for patches for stable trees.
Also, can you please sign off on these as well?
Can you fix that up and resend? I'll be glad to queue them up then.
thanks,
Remove the "Bug/Change-Id" from 0001-BACKPORT-make-user_access_begin-do-access_ok.patch.
Actually, I got the patch from https://android-review.googlesource.com/c/kernel/common/+/1114632 Todd backported the patch but there is no Todd's signed-off-by in his patch. Should I add "Signed-off-by: Todd Kjos tkjos@google.com" as well?
cheers Miles
greg k-h
On Thu, Jun 11, 2020 at 09:37:42AM +0800, Miles Chen wrote:
On Wed, 2020-06-10 at 20:02 +0200, Greg KH wrote:
On Thu, Jun 11, 2020 at 01:58:20AM +0800, Miles Chen wrote:
Hi,
I suggest to include the commit: 594cc251fdd0 make 'user_access_begin()' do 'access_ok()' for CVE-2018-20669.
stable version to apply to: kernel-4.14.y and kernel-4.19.y.
From the discussion below, I checked the latest kernel and found that we should also apply other 4 patches. (total 5 patches) https://lkml.org/lkml/2020/5/12/943
patch list: commit ab10ae1c3bef lib: Reduce user_access_begin() boundaries in strncpy_from_user() and strnlen_user() commit 6e693b3ffecb x86: uaccess: Inhibit speculation past access_ok() in user_access_begin() commit 9cb2feb4d21d arch/openrisc: Fix issues with access_ok() commit 94bd8a05cd4d Fix 'acccess_ok()' on alpha and SH commit 594cc251fdd0 make 'user_access_begin()' do 'access_ok()'
Where only commit 6e693b3ffecb does not need backport modifications. I attach my backport patches in this email.
I merged the patches with kernel-4.19.127 and kernel-4.14.183 without conflicts. Build with arm64 defconfig and bootup on arm64 QEMU environment.
cheers, Miles
From ac351de9ddd86ef717a3f89236dc5f6b2a108cc7 Mon Sep 17 00:00:00 2001 From: Linus Torvalds torvalds@linux-foundation.org Date: Fri, 4 Jan 2019 12:56:09 -0800 Subject: [PATCH] BACKPORT: make 'user_access_begin()' do 'access_ok()'
upstream commit 594cc251fdd0 ("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: 135368228 Change-Id: I4ca0e4566ea080fa148c5e768bb1a0b6f7201c01 Signed-off-by: Linus Torvalds torvalds@linux-foundation.org
No need for "Bug:" or "Change-Id:" for patches for stable trees.
Also, can you please sign off on these as well?
Can you fix that up and resend? I'll be glad to queue them up then.
thanks,
Remove the "Bug/Change-Id" from 0001-BACKPORT-make-user_access_begin-do-access_ok.patch.
Actually, I got the patch from https://android-review.googlesource.com/c/kernel/common/+/1114632 Todd backported the patch but there is no Todd's signed-off-by in his patch. Should I add "Signed-off-by: Todd Kjos tkjos@google.com" as well?
Hm, nah, I'll fix these up, we don't need the Android-specific headers on the patch either. Thanks for the backports, I'll go try to queue them up now...
greg k-h
On Thu, Jun 11, 2020 at 09:37:42AM +0800, Miles Chen wrote:
@@ -2601,7 +2603,17 @@ 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(VERIFY_WRITE, 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)) continue;
No one seems to have test-built this code, it fails here on the 4.14.y kernel :(
I'll go fix it up, but please, always at the very least, test build your patches before sending them out...
thanks,
greg k-h
On Thu, 2020-06-11 at 13:15 +0200, Greg KH wrote:
On Thu, Jun 11, 2020 at 09:37:42AM +0800, Miles Chen wrote:
@@ -2601,7 +2603,17 @@ 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(VERIFY_WRITE, 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)) continue;
No one seems to have test-built this code, it fails here on the 4.14.y kernel :(
I'll go fix it up, but please, always at the very least, test build your patches before sending them out...
thanks,
Sorry for the breakage. It won't happen next time.
cheers, Miles
greg k-h
linux-stable-mirror@lists.linaro.org
-
Greg KH -
Miles Chen