linux-kernel.vger.kernel.org archive mirror
 help / color / mirror / Atom feed
* [PATCH v1] proc/vmcore: fix clearing user buffer by properly using clear_user()
@ 2021-11-11 19:18 David Hildenbrand
  2021-11-12  7:01 ` Baoquan He
  0 siblings, 1 reply; 6+ messages in thread
From: David Hildenbrand @ 2021-11-11 19:18 UTC (permalink / raw)
  To: linux-kernel
  Cc: David Hildenbrand, Dave Young, Baoquan He, Vivek Goyal,
	Andrew Morton, Philipp Rudo, kexec, linux-mm, linux-fsdevel

To clear a user buffer we cannot simply use memset, we have to use
clear_user(). Using a kernel config based on rawhide Fedora and a
virtio-mem device that registers a vmcore_cb, I can easily trigger:

[   11.327580] systemd[1]: Starting Kdump Vmcore Save Service...
[   11.339697] kdump[420]: Kdump is using the default log level(3).
[   11.370964] kdump[453]: saving to /sysroot/var/crash/127.0.0.1-2021-11-11-14:59:22/
[   11.373997] kdump[458]: saving vmcore-dmesg.txt to /sysroot/var/crash/127.0.0.1-2021-11-11-14:59:22/
[   11.385357] kdump[465]: saving vmcore-dmesg.txt complete
[   11.386722] kdump[467]: saving vmcore
[   16.531275] BUG: unable to handle page fault for address: 00007f2374e01000
[   16.531705] #PF: supervisor write access in kernel mode
[   16.532037] #PF: error_code(0x0003) - permissions violation
[   16.532396] PGD 7a523067 P4D 7a523067 PUD 7a528067 PMD 7a525067 PTE 800000007048f867
[   16.532872] Oops: 0003 [#1] PREEMPT SMP NOPTI
[   16.533154] CPU: 0 PID: 468 Comm: cp Not tainted 5.15.0+ #6
[   16.533513] Hardware name: QEMU Standard PC (Q35 + ICH9, 2009), BIOS rel-1.14.0-27-g64f37cc530f1-prebuilt.qemu.org 04/01/2014
[   16.534198] RIP: 0010:read_from_oldmem.part.0.cold+0x1d/0x86
[   16.534552] Code: ff ff ff e8 05 ff fe ff e9 b9 e9 7f ff 48 89 de 48 c7 c7 38 3b 60 82 e8 f1 fe fe ff 83 fd 08 72 3c 49 8d 7d 08 4c 89 e9 89 e8 <49> c7 45 00 00 00 00 00 49 c7 44 05 f8 00 00 00 00 48 83 e7 f81
[   16.535670] RSP: 0018:ffffc9000073be08 EFLAGS: 00010212
[   16.535998] RAX: 0000000000001000 RBX: 00000000002fd000 RCX: 00007f2374e01000
[   16.536441] RDX: 0000000000000001 RSI: 00000000ffffdfff RDI: 00007f2374e01008
[   16.536878] RBP: 0000000000001000 R08: 0000000000000000 R09: ffffc9000073bc50
[   16.537315] R10: ffffc9000073bc48 R11: ffffffff829461a8 R12: 000000000000f000
[   16.537755] R13: 00007f2374e01000 R14: 0000000000000000 R15: ffff88807bd421e8
[   16.538200] FS:  00007f2374e12140(0000) GS:ffff88807f000000(0000) knlGS:0000000000000000
[   16.538696] CS:  0010 DS: 0000 ES: 0000 CR0: 0000000080050033
[   16.539055] CR2: 00007f2374e01000 CR3: 000000007a4aa000 CR4: 0000000000350eb0
[   16.539510] Call Trace:
[   16.539679]  <TASK>
[   16.539828]  read_vmcore+0x236/0x2c0
[   16.540063]  ? enqueue_hrtimer+0x2f/0x80
[   16.540323]  ? inode_security+0x22/0x60
[   16.540572]  proc_reg_read+0x55/0xa0
[   16.540807]  vfs_read+0x95/0x190
[   16.541022]  ksys_read+0x4f/0xc0
[   16.541238]  do_syscall_64+0x3b/0x90
[   16.541475]  entry_SYSCALL_64_after_hwframe+0x44/0xae

To fix, properly use clear_user() when required.

Fixes: 997c136f518c ("fs/proc/vmcore.c: add hook to read_from_oldmem() to check for non-ram pages")
Cc: Dave Young <dyoung@redhat.com>
Cc: Baoquan He <bhe@redhat.com>
Cc: Vivek Goyal <vgoyal@redhat.com>
Cc: Andrew Morton <akpm@linux-foundation.org>
Cc: Philipp Rudo <prudo@redhat.com>
Cc: kexec@lists.infradead.org
Cc: linux-mm@kvack.org
Cc: linux-fsdevel@vger.kernel.org
Signed-off-by: David Hildenbrand <david@redhat.com>
---
 fs/proc/vmcore.c | 20 ++++++++++++--------
 1 file changed, 12 insertions(+), 8 deletions(-)

diff --git a/fs/proc/vmcore.c b/fs/proc/vmcore.c
index 30a3b66f475a..509f85148fee 100644
--- a/fs/proc/vmcore.c
+++ b/fs/proc/vmcore.c
@@ -154,9 +154,13 @@ ssize_t read_from_oldmem(char *buf, size_t count,
 			nr_bytes = count;
 
 		/* If pfn is not ram, return zeros for sparse dump files */
-		if (!pfn_is_ram(pfn))
-			memset(buf, 0, nr_bytes);
-		else {
+		if (!pfn_is_ram(pfn)) {
+			tmp = 0;
+			if (!userbuf)
+				memset(buf, 0, nr_bytes);
+			else if (clear_user(buf, nr_bytes))
+				tmp = -EFAULT;
+		} else {
 			if (encrypted)
 				tmp = copy_oldmem_page_encrypted(pfn, buf,
 								 nr_bytes,
@@ -165,12 +169,12 @@ ssize_t read_from_oldmem(char *buf, size_t count,
 			else
 				tmp = copy_oldmem_page(pfn, buf, nr_bytes,
 						       offset, userbuf);
-
-			if (tmp < 0) {
-				up_read(&vmcore_cb_rwsem);
-				return tmp;
-			}
 		}
+		if (tmp < 0) {
+			up_read(&vmcore_cb_rwsem);
+			return tmp;
+		}
+
 		*ppos += nr_bytes;
 		count -= nr_bytes;
 		buf += nr_bytes;

base-commit: debe436e77c72fcee804fb867f275e6d31aa999c
-- 
2.31.1


^ permalink raw reply related	[flat|nested] 6+ messages in thread

* Re: [PATCH v1] proc/vmcore: fix clearing user buffer by properly using clear_user()
  2021-11-11 19:18 [PATCH v1] proc/vmcore: fix clearing user buffer by properly using clear_user() David Hildenbrand
@ 2021-11-12  7:01 ` Baoquan He
  2021-11-12  8:16   ` David Hildenbrand
  0 siblings, 1 reply; 6+ messages in thread
From: Baoquan He @ 2021-11-12  7:01 UTC (permalink / raw)
  To: David Hildenbrand
  Cc: linux-kernel, Dave Young, Vivek Goyal, Andrew Morton,
	Philipp Rudo, kexec, linux-mm, linux-fsdevel

On 11/11/21 at 08:18pm, David Hildenbrand wrote:
> To clear a user buffer we cannot simply use memset, we have to use
> clear_user(). Using a kernel config based on rawhide Fedora and a
> virtio-mem device that registers a vmcore_cb, I can easily trigger:
> 
> [   11.327580] systemd[1]: Starting Kdump Vmcore Save Service...
> [   11.339697] kdump[420]: Kdump is using the default log level(3).
> [   11.370964] kdump[453]: saving to /sysroot/var/crash/127.0.0.1-2021-11-11-14:59:22/
> [   11.373997] kdump[458]: saving vmcore-dmesg.txt to /sysroot/var/crash/127.0.0.1-2021-11-11-14:59:22/
> [   11.385357] kdump[465]: saving vmcore-dmesg.txt complete
> [   11.386722] kdump[467]: saving vmcore
> [   16.531275] BUG: unable to handle page fault for address: 00007f2374e01000
> [   16.531705] #PF: supervisor write access in kernel mode
> [   16.532037] #PF: error_code(0x0003) - permissions violation
> [   16.532396] PGD 7a523067 P4D 7a523067 PUD 7a528067 PMD 7a525067 PTE 800000007048f867
> [   16.532872] Oops: 0003 [#1] PREEMPT SMP NOPTI
> [   16.533154] CPU: 0 PID: 468 Comm: cp Not tainted 5.15.0+ #6
> [   16.533513] Hardware name: QEMU Standard PC (Q35 + ICH9, 2009), BIOS rel-1.14.0-27-g64f37cc530f1-prebuilt.qemu.org 04/01/2014
> [   16.534198] RIP: 0010:read_from_oldmem.part.0.cold+0x1d/0x86
> [   16.534552] Code: ff ff ff e8 05 ff fe ff e9 b9 e9 7f ff 48 89 de 48 c7 c7 38 3b 60 82 e8 f1 fe fe ff 83 fd 08 72 3c 49 8d 7d 08 4c 89 e9 89 e8 <49> c7 45 00 00 00 00 00 49 c7 44 05 f8 00 00 00 00 48 83 e7 f81
> [   16.535670] RSP: 0018:ffffc9000073be08 EFLAGS: 00010212
> [   16.535998] RAX: 0000000000001000 RBX: 00000000002fd000 RCX: 00007f2374e01000
> [   16.536441] RDX: 0000000000000001 RSI: 00000000ffffdfff RDI: 00007f2374e01008
> [   16.536878] RBP: 0000000000001000 R08: 0000000000000000 R09: ffffc9000073bc50
> [   16.537315] R10: ffffc9000073bc48 R11: ffffffff829461a8 R12: 000000000000f000
> [   16.537755] R13: 00007f2374e01000 R14: 0000000000000000 R15: ffff88807bd421e8
> [   16.538200] FS:  00007f2374e12140(0000) GS:ffff88807f000000(0000) knlGS:0000000000000000
> [   16.538696] CS:  0010 DS: 0000 ES: 0000 CR0: 0000000080050033
> [   16.539055] CR2: 00007f2374e01000 CR3: 000000007a4aa000 CR4: 0000000000350eb0
> [   16.539510] Call Trace:
> [   16.539679]  <TASK>
> [   16.539828]  read_vmcore+0x236/0x2c0
> [   16.540063]  ? enqueue_hrtimer+0x2f/0x80
> [   16.540323]  ? inode_security+0x22/0x60
> [   16.540572]  proc_reg_read+0x55/0xa0
> [   16.540807]  vfs_read+0x95/0x190
> [   16.541022]  ksys_read+0x4f/0xc0
> [   16.541238]  do_syscall_64+0x3b/0x90
> [   16.541475]  entry_SYSCALL_64_after_hwframe+0x44/0xae
> 
> To fix, properly use clear_user() when required.

Looks a great fix to me, thanks for fixing this. 

Check the code, clear_user invokes access_ok to do check, then call
memset(). It's unclear to me how the bug is triggered, could you
please tell more so that I can learn? 


> 
> Fixes: 997c136f518c ("fs/proc/vmcore.c: add hook to read_from_oldmem() to check for non-ram pages")
> Cc: Dave Young <dyoung@redhat.com>
> Cc: Baoquan He <bhe@redhat.com>
> Cc: Vivek Goyal <vgoyal@redhat.com>
> Cc: Andrew Morton <akpm@linux-foundation.org>
> Cc: Philipp Rudo <prudo@redhat.com>
> Cc: kexec@lists.infradead.org
> Cc: linux-mm@kvack.org
> Cc: linux-fsdevel@vger.kernel.org
> Signed-off-by: David Hildenbrand <david@redhat.com>
> ---
>  fs/proc/vmcore.c | 20 ++++++++++++--------
>  1 file changed, 12 insertions(+), 8 deletions(-)
> 
> diff --git a/fs/proc/vmcore.c b/fs/proc/vmcore.c
> index 30a3b66f475a..509f85148fee 100644
> --- a/fs/proc/vmcore.c
> +++ b/fs/proc/vmcore.c
> @@ -154,9 +154,13 @@ ssize_t read_from_oldmem(char *buf, size_t count,
>  			nr_bytes = count;
>  
>  		/* If pfn is not ram, return zeros for sparse dump files */
> -		if (!pfn_is_ram(pfn))
> -			memset(buf, 0, nr_bytes);
> -		else {
> +		if (!pfn_is_ram(pfn)) {
> +			tmp = 0;
> +			if (!userbuf)
> +				memset(buf, 0, nr_bytes);
> +			else if (clear_user(buf, nr_bytes))
> +				tmp = -EFAULT;
> +		} else {
>  			if (encrypted)
>  				tmp = copy_oldmem_page_encrypted(pfn, buf,
>  								 nr_bytes,
> @@ -165,12 +169,12 @@ ssize_t read_from_oldmem(char *buf, size_t count,
>  			else
>  				tmp = copy_oldmem_page(pfn, buf, nr_bytes,
>  						       offset, userbuf);
> -
> -			if (tmp < 0) {
> -				up_read(&vmcore_cb_rwsem);
> -				return tmp;
> -			}
>  		}
> +		if (tmp < 0) {
> +			up_read(&vmcore_cb_rwsem);
> +			return tmp;
> +		}
> +
>  		*ppos += nr_bytes;
>  		count -= nr_bytes;
>  		buf += nr_bytes;
> 
> base-commit: debe436e77c72fcee804fb867f275e6d31aa999c
> -- 
> 2.31.1
> 


^ permalink raw reply	[flat|nested] 6+ messages in thread

* Re: [PATCH v1] proc/vmcore: fix clearing user buffer by properly using clear_user()
  2021-11-12  7:01 ` Baoquan He
@ 2021-11-12  8:16   ` David Hildenbrand
  2021-11-12  9:01     ` Baoquan He
  0 siblings, 1 reply; 6+ messages in thread
From: David Hildenbrand @ 2021-11-12  8:16 UTC (permalink / raw)
  To: Baoquan He
  Cc: linux-kernel, Dave Young, Vivek Goyal, Andrew Morton,
	Philipp Rudo, kexec, linux-mm, linux-fsdevel

On 12.11.21 08:01, Baoquan He wrote:
> On 11/11/21 at 08:18pm, David Hildenbrand wrote:
>> To clear a user buffer we cannot simply use memset, we have to use
>> clear_user(). Using a kernel config based on rawhide Fedora and a
>> virtio-mem device that registers a vmcore_cb, I can easily trigger:
>>
>> [   11.327580] systemd[1]: Starting Kdump Vmcore Save Service...
>> [   11.339697] kdump[420]: Kdump is using the default log level(3).
>> [   11.370964] kdump[453]: saving to /sysroot/var/crash/127.0.0.1-2021-11-11-14:59:22/
>> [   11.373997] kdump[458]: saving vmcore-dmesg.txt to /sysroot/var/crash/127.0.0.1-2021-11-11-14:59:22/
>> [   11.385357] kdump[465]: saving vmcore-dmesg.txt complete
>> [   11.386722] kdump[467]: saving vmcore
>> [   16.531275] BUG: unable to handle page fault for address: 00007f2374e01000
>> [   16.531705] #PF: supervisor write access in kernel mode
>> [   16.532037] #PF: error_code(0x0003) - permissions violation
>> [   16.532396] PGD 7a523067 P4D 7a523067 PUD 7a528067 PMD 7a525067 PTE 800000007048f867
>> [   16.532872] Oops: 0003 [#1] PREEMPT SMP NOPTI
>> [   16.533154] CPU: 0 PID: 468 Comm: cp Not tainted 5.15.0+ #6
>> [   16.533513] Hardware name: QEMU Standard PC (Q35 + ICH9, 2009), BIOS rel-1.14.0-27-g64f37cc530f1-prebuilt.qemu.org 04/01/2014
>> [   16.534198] RIP: 0010:read_from_oldmem.part.0.cold+0x1d/0x86
>> [   16.534552] Code: ff ff ff e8 05 ff fe ff e9 b9 e9 7f ff 48 89 de 48 c7 c7 38 3b 60 82 e8 f1 fe fe ff 83 fd 08 72 3c 49 8d 7d 08 4c 89 e9 89 e8 <49> c7 45 00 00 00 00 00 49 c7 44 05 f8 00 00 00 00 48 83 e7 f81
>> [   16.535670] RSP: 0018:ffffc9000073be08 EFLAGS: 00010212
>> [   16.535998] RAX: 0000000000001000 RBX: 00000000002fd000 RCX: 00007f2374e01000
>> [   16.536441] RDX: 0000000000000001 RSI: 00000000ffffdfff RDI: 00007f2374e01008
>> [   16.536878] RBP: 0000000000001000 R08: 0000000000000000 R09: ffffc9000073bc50
>> [   16.537315] R10: ffffc9000073bc48 R11: ffffffff829461a8 R12: 000000000000f000
>> [   16.537755] R13: 00007f2374e01000 R14: 0000000000000000 R15: ffff88807bd421e8
>> [   16.538200] FS:  00007f2374e12140(0000) GS:ffff88807f000000(0000) knlGS:0000000000000000
>> [   16.538696] CS:  0010 DS: 0000 ES: 0000 CR0: 0000000080050033
>> [   16.539055] CR2: 00007f2374e01000 CR3: 000000007a4aa000 CR4: 0000000000350eb0
>> [   16.539510] Call Trace:
>> [   16.539679]  <TASK>
>> [   16.539828]  read_vmcore+0x236/0x2c0
>> [   16.540063]  ? enqueue_hrtimer+0x2f/0x80
>> [   16.540323]  ? inode_security+0x22/0x60
>> [   16.540572]  proc_reg_read+0x55/0xa0
>> [   16.540807]  vfs_read+0x95/0x190
>> [   16.541022]  ksys_read+0x4f/0xc0
>> [   16.541238]  do_syscall_64+0x3b/0x90
>> [   16.541475]  entry_SYSCALL_64_after_hwframe+0x44/0xae
>>
>> To fix, properly use clear_user() when required.
> 
> Looks a great fix to me, thanks for fixing this. 
> 
> Check the code, clear_user invokes access_ok to do check, then call
> memset(). It's unclear to me how the bug is triggered, could you
> please tell more so that I can learn? 
>
TBH, I was testing virtio-mem+vmcore before without running into this
issue, but after I retested with upstream in a different setup
(different kernel config but eventually also different CPU features), I
ran into this.


Note that you were looking at the generic __clear_user() implementation,
the x86-64 variant is different, see arch/x86/lib/usercopy_64.c

I can spot that it triggers stac()/clac() (X86_SMAP):
https://en.wikipedia.org/wiki/Supervisor_Mode_Access_Prevention

"that allows supervisor mode programs to optionally set user-space
memory mappings so that access to those mappings from supervisor mode
will cause a trap. This makes it harder for malicious programs to
"trick" the kernel into using instructions or data from a user-space
program"

Yes, that's most probably it :)

-- 
Thanks,

David / dhildenb


^ permalink raw reply	[flat|nested] 6+ messages in thread

* Re: [PATCH v1] proc/vmcore: fix clearing user buffer by properly using clear_user()
  2021-11-12  8:16   ` David Hildenbrand
@ 2021-11-12  9:01     ` Baoquan He
  2021-11-12  9:08       ` David Hildenbrand
  0 siblings, 1 reply; 6+ messages in thread
From: Baoquan He @ 2021-11-12  9:01 UTC (permalink / raw)
  To: David Hildenbrand
  Cc: linux-kernel, Dave Young, Vivek Goyal, Andrew Morton,
	Philipp Rudo, kexec, linux-mm, linux-fsdevel

On 11/12/21 at 09:16am, David Hildenbrand wrote:
> On 12.11.21 08:01, Baoquan He wrote:
> > On 11/11/21 at 08:18pm, David Hildenbrand wrote:
> >> To clear a user buffer we cannot simply use memset, we have to use
> >> clear_user(). Using a kernel config based on rawhide Fedora and a
> >> virtio-mem device that registers a vmcore_cb, I can easily trigger:
> >>
> >> [   11.327580] systemd[1]: Starting Kdump Vmcore Save Service...
> >> [   11.339697] kdump[420]: Kdump is using the default log level(3).
> >> [   11.370964] kdump[453]: saving to /sysroot/var/crash/127.0.0.1-2021-11-11-14:59:22/
> >> [   11.373997] kdump[458]: saving vmcore-dmesg.txt to /sysroot/var/crash/127.0.0.1-2021-11-11-14:59:22/
> >> [   11.385357] kdump[465]: saving vmcore-dmesg.txt complete
> >> [   11.386722] kdump[467]: saving vmcore
> >> [   16.531275] BUG: unable to handle page fault for address: 00007f2374e01000
> >> [   16.531705] #PF: supervisor write access in kernel mode
> >> [   16.532037] #PF: error_code(0x0003) - permissions violation
> >> [   16.532396] PGD 7a523067 P4D 7a523067 PUD 7a528067 PMD 7a525067 PTE 800000007048f867
> >> [   16.532872] Oops: 0003 [#1] PREEMPT SMP NOPTI
> >> [   16.533154] CPU: 0 PID: 468 Comm: cp Not tainted 5.15.0+ #6
> >> [   16.533513] Hardware name: QEMU Standard PC (Q35 + ICH9, 2009), BIOS rel-1.14.0-27-g64f37cc530f1-prebuilt.qemu.org 04/01/2014
> >> [   16.534198] RIP: 0010:read_from_oldmem.part.0.cold+0x1d/0x86
> >> [   16.534552] Code: ff ff ff e8 05 ff fe ff e9 b9 e9 7f ff 48 89 de 48 c7 c7 38 3b 60 82 e8 f1 fe fe ff 83 fd 08 72 3c 49 8d 7d 08 4c 89 e9 89 e8 <49> c7 45 00 00 00 00 00 49 c7 44 05 f8 00 00 00 00 48 83 e7 f81
> >> [   16.535670] RSP: 0018:ffffc9000073be08 EFLAGS: 00010212
> >> [   16.535998] RAX: 0000000000001000 RBX: 00000000002fd000 RCX: 00007f2374e01000
> >> [   16.536441] RDX: 0000000000000001 RSI: 00000000ffffdfff RDI: 00007f2374e01008
> >> [   16.536878] RBP: 0000000000001000 R08: 0000000000000000 R09: ffffc9000073bc50
> >> [   16.537315] R10: ffffc9000073bc48 R11: ffffffff829461a8 R12: 000000000000f000
> >> [   16.537755] R13: 00007f2374e01000 R14: 0000000000000000 R15: ffff88807bd421e8
> >> [   16.538200] FS:  00007f2374e12140(0000) GS:ffff88807f000000(0000) knlGS:0000000000000000
> >> [   16.538696] CS:  0010 DS: 0000 ES: 0000 CR0: 0000000080050033
> >> [   16.539055] CR2: 00007f2374e01000 CR3: 000000007a4aa000 CR4: 0000000000350eb0
> >> [   16.539510] Call Trace:
> >> [   16.539679]  <TASK>
> >> [   16.539828]  read_vmcore+0x236/0x2c0
> >> [   16.540063]  ? enqueue_hrtimer+0x2f/0x80
> >> [   16.540323]  ? inode_security+0x22/0x60
> >> [   16.540572]  proc_reg_read+0x55/0xa0
> >> [   16.540807]  vfs_read+0x95/0x190
> >> [   16.541022]  ksys_read+0x4f/0xc0
> >> [   16.541238]  do_syscall_64+0x3b/0x90
> >> [   16.541475]  entry_SYSCALL_64_after_hwframe+0x44/0xae
> >>
> >> To fix, properly use clear_user() when required.
> > 
> > Looks a great fix to me, thanks for fixing this. 
> > 
> > Check the code, clear_user invokes access_ok to do check, then call
> > memset(). It's unclear to me how the bug is triggered, could you
> > please tell more so that I can learn? 
> >
> TBH, I was testing virtio-mem+vmcore before without running into this
> issue, but after I retested with upstream in a different setup
> (different kernel config but eventually also different CPU features), I
> ran into this.
> 
> 
> Note that you were looking at the generic __clear_user() implementation,
> the x86-64 variant is different, see arch/x86/lib/usercopy_64.c
> 
> I can spot that it triggers stac()/clac() (X86_SMAP):
> https://en.wikipedia.org/wiki/Supervisor_Mode_Access_Prevention
> 
> "that allows supervisor mode programs to optionally set user-space
> memory mappings so that access to those mappings from supervisor mode
> will cause a trap. This makes it harder for malicious programs to
> "trick" the kernel into using instructions or data from a user-space
> program"

OK, probably. I thought it's triggered in access_ok(), and tried to
figure out why. But seems we should do something to check this in
access_ok(), otherwise the logic of clear_user/_clear_user is not so
reasonable. Anyway, I have learned it, thanks a lot for digging it out.

By the way, I can't open above wiki article, found below commit from
hpa. Maybe we can add some into log to tell this, not strong opinin,
leave it to you.

For this patch, looks good to me.

Acked-by: Baoquan He <bhe@redhat.com>

~~~~~~~~~
commit 63bcff2a307b9bcc712a8251eb27df8b2e117967
Author: H. Peter Anvin <hpa@linux.intel.com>
Date:   Fri Sep 21 12:43:12 2012 -0700

    x86, smap: Add STAC and CLAC instructions to control user space access
    
    When Supervisor Mode Access Prevention (SMAP) is enabled, access to
    userspace from the kernel is controlled by the AC flag.  To make the
    performance of manipulating that flag acceptable, there are two new
    instructions, STAC and CLAC, to set and clear it.
    
    This patch adds those instructions, via alternative(), when the SMAP
    feature is enabled.  It also adds X86_EFLAGS_AC unconditionally to the
    SYSCALL entry mask; there is simply no reason to make that one
    conditional.
    
    Signed-off-by: H. Peter Anvin <hpa@linux.intel.com>
    Link: http://lkml.kernel.org/r/1348256595-29119-9-git-send-email-hpa@linux.intel.com


^ permalink raw reply	[flat|nested] 6+ messages in thread

* Re: [PATCH v1] proc/vmcore: fix clearing user buffer by properly using clear_user()
  2021-11-12  9:01     ` Baoquan He
@ 2021-11-12  9:08       ` David Hildenbrand
  2021-11-12 13:23         ` Baoquan He
  0 siblings, 1 reply; 6+ messages in thread
From: David Hildenbrand @ 2021-11-12  9:08 UTC (permalink / raw)
  To: Baoquan He
  Cc: Linux Kernel Mailing List, Dave Young, Vivek Goyal,
	Andrew Morton, Philipp Rudo, kexec, Linux MM, linux-fsdevel

> > "that allows supervisor mode programs to optionally set user-space
> > memory mappings so that access to those mappings from supervisor mode
> > will cause a trap. This makes it harder for malicious programs to
> > "trick" the kernel into using instructions or data from a user-space
> > program"
>
> OK, probably. I thought it's triggered in access_ok(), and tried to
> figure out why. But seems we should do something to check this in
> access_ok(), otherwise the logic of clear_user/_clear_user is not so
> reasonable. Anyway, I have learned it, thanks a lot for digging it out.
>
> By the way, I can't open above wiki article, found below commit from
> hpa. Maybe we can add some into log to tell this, not strong opinin,
> leave it to you.

Yes, now that we know the root cause I'll add some more details to the
patch description and resend -- thanks Baoquan!


^ permalink raw reply	[flat|nested] 6+ messages in thread

* Re: [PATCH v1] proc/vmcore: fix clearing user buffer by properly using clear_user()
  2021-11-12  9:08       ` David Hildenbrand
@ 2021-11-12 13:23         ` Baoquan He
  0 siblings, 0 replies; 6+ messages in thread
From: Baoquan He @ 2021-11-12 13:23 UTC (permalink / raw)
  To: David Hildenbrand
  Cc: Linux Kernel Mailing List, Dave Young, Vivek Goyal,
	Andrew Morton, Philipp Rudo, kexec, Linux MM, linux-fsdevel

On 11/12/21 at 10:08am, David Hildenbrand wrote:
> > > "that allows supervisor mode programs to optionally set user-space
> > > memory mappings so that access to those mappings from supervisor mode
> > > will cause a trap. This makes it harder for malicious programs to
> > > "trick" the kernel into using instructions or data from a user-space
> > > program"
> >
> > OK, probably. I thought it's triggered in access_ok(), and tried to
> > figure out why. But seems we should do something to check this in
> > access_ok(), otherwise the logic of clear_user/_clear_user is not so
> > reasonable. Anyway, I have learned it, thanks a lot for digging it out.
> >
> > By the way, I can't open above wiki article, found below commit from
> > hpa. Maybe we can add some into log to tell this, not strong opinin,
> > leave it to you.
> 
> Yes, now that we know the root cause I'll add some more details to the
> patch description and resend -- thanks Baoquan!

Thanks for sending v2.


^ permalink raw reply	[flat|nested] 6+ messages in thread

end of thread, other threads:[~2021-11-12 13:24 UTC | newest]

Thread overview: 6+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2021-11-11 19:18 [PATCH v1] proc/vmcore: fix clearing user buffer by properly using clear_user() David Hildenbrand
2021-11-12  7:01 ` Baoquan He
2021-11-12  8:16   ` David Hildenbrand
2021-11-12  9:01     ` Baoquan He
2021-11-12  9:08       ` David Hildenbrand
2021-11-12 13:23         ` Baoquan He

This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).