Greeting,
FYI, we noticed the following commit (built with gcc-9):
commit: 6aded12b10e0c9536ee2c8ee33a1f7ed52f9cb34 ("scsi: core: Remove struct scsi_request")
url: https://github.com/0day-ci/linux/commits/Krzysztof-Kozlowski/ufs-qcom-drop-custom-Android-boot-parameters/20220320-190652
in testcase: boot
on test machine: qemu-system-x86_64 -enable-kvm -cpu SandyBridge -smp 2 -m 16G
caused below changes (please refer to attached dmesg/kmsg for entire log/backtrace):
+------------------------------------------+------------+------------+
| | dbb4c84d87 | 6aded12b10 |
+------------------------------------------+------------+------------+
| boot_successes | 344 | 313 |
| boot_failures | 0 | 29 |
| kernel_BUG_at_mm/usercopy.c | 0 | 29 |
| invalid_opcode:#[##] | 0 | 29 |
| EIP:usercopy_abort | 0 | 29 |
| Kernel_panic-not_syncing:Fatal_exception | 0 | 29 |
+------------------------------------------+------------+------------+
If you fix the issue, kindly add following tag
Reported-by: kernel test robot <[email protected]>
[ 34.497756][ T331] kernel BUG at mm/usercopy.c:100!
[ 34.498182][ T331] invalid opcode: 0000 [#1] SMP
[ 34.498563][ T331] CPU: 1 PID: 331 Comm: scsi_id Not tainted 5.17.0-rc1-00234-g6aded12b10e0 #1
[ 34.499235][ T331] Hardware name: QEMU Standard PC (i440FX + PIIX, 1996), BIOS 1.12.0-1 04/01/2014
[ 34.499930][ T331] EIP: usercopy_abort (mm/usercopy.c:100 (discriminator 24))
[ 34.500323][ T331] Code: c4 b9 4c 8c 9f c4 ff 75 0c ff 75 08 56 52 53 50 57 51 68 cd 8c 9f c4 e8 16 d6 ff ff 83 05 18 d3 4e c6 01 83 15 1c d3 4e c6 00 <0f> 0b 83 05 20 d3 4e c6 01 83 15 24 d3 4e c6 00 83 05 28 d3 4e c6
All code
========
0: c4 (bad)
1: b9 4c 8c 9f c4 mov $0xc49f8c4c,%ecx
6: ff 75 0c pushq 0xc(%rbp)
9: ff 75 08 pushq 0x8(%rbp)
c: 56 push %rsi
d: 52 push %rdx
e: 53 push %rbx
f: 50 push %rax
10: 57 push %rdi
11: 51 push %rcx
12: 68 cd 8c 9f c4 pushq $0xffffffffc49f8ccd
17: e8 16 d6 ff ff callq 0xffffffffffffd632
1c: 83 05 18 d3 4e c6 01 addl $0x1,-0x39b12ce8(%rip) # 0xffffffffc64ed33b
23: 83 15 1c d3 4e c6 00 adcl $0x0,-0x39b12ce4(%rip) # 0xffffffffc64ed346
2a:* 0f 0b ud2 <-- trapping instruction
2c: 83 05 20 d3 4e c6 01 addl $0x1,-0x39b12ce0(%rip) # 0xffffffffc64ed353
33: 83 15 24 d3 4e c6 00 adcl $0x0,-0x39b12cdc(%rip) # 0xffffffffc64ed35e
3a: 83 .byte 0x83
3b: 05 28 d3 4e c6 add $0xc64ed328,%eax
Code starting with the faulting instruction
===========================================
0: 0f 0b ud2
2: 83 05 20 d3 4e c6 01 addl $0x1,-0x39b12ce0(%rip) # 0xffffffffc64ed329
9: 83 15 24 d3 4e c6 00 adcl $0x0,-0x39b12cdc(%rip) # 0xffffffffc64ed334
10: 83 .byte 0x83
11: 05 28 d3 4e c6 add $0xc64ed328,%eax
[ 34.501804][ T331] EAX: 0000005e EBX: c4cd44b7 ECX: ee0f79ac EDX: 01000000
[ 34.502351][ T331] ESI: c4cd44b7 EDI: c4ad1a53 EBP: f4f73e38 ESP: f4f73e08
[ 34.502904][ T331] DS: 007b ES: 007b FS: 00d8 GS: 0033 SS: 0068 EFLAGS: 00010246
[ 34.503495][ T331] CR0: 80050033 CR2: b7aaf138 CR3: 34d19000 CR4: 00040690
[ 34.504046][ T331] DR0: 00000000 DR1: 00000000 DR2: 00000000 DR3: 00000000
[ 34.504579][ T331] DR6: fffe0ff0 DR7: 00000400
[ 34.504983][ T331] Call Trace:
[ 34.505258][ T331] __check_object_size (mm/usercopy.c:157 mm/usercopy.c:267)
[ 34.505664][ T331] sg_io (include/linux/uaccess.h:192 drivers/scsi/scsi_ioctl.c:352 drivers/scsi/scsi_ioctl.c:449)
[ 34.505976][ T331] ? _copy_from_user (arch/x86/include/asm/uaccess_32.h:26 lib/usercopy.c:16)
[ 34.506382][ T331] scsi_ioctl (drivers/scsi/scsi_ioctl.c:859 drivers/scsi/scsi_ioctl.c:913)
[ 34.506726][ T331] sd_ioctl (drivers/scsi/sd.c:1501)
[ 34.507064][ T331] ? scsi_disk_put (drivers/scsi/sd.c:1475)
[ 34.507431][ T331] blkdev_ioctl (block/ioctl.c:588)
[ 34.507801][ T331] ? __might_fault (mm/memory.c:5272)
[ 34.508168][ T331] ? blkdev_common_ioctl (block/ioctl.c:533)
[ 34.508597][ T331] vfs_ioctl (fs/ioctl.c:52)
[ 34.508930][ T331] __ia32_sys_ioctl (fs/ioctl.c:874 fs/ioctl.c:860 fs/ioctl.c:860)
[ 34.509281][ T331] __do_fast_syscall_32 (arch/x86/entry/common.c:112 arch/x86/entry/common.c:178)
[ 34.509682][ T331] do_fast_syscall_32 (arch/x86/entry/common.c:203)
[ 34.510061][ T331] do_SYSENTER_32 (arch/x86/entry/common.c:247)
[ 34.510403][ T331] entry_SYSENTER_32 (arch/x86/entry/entry_32.S:869)
[ 34.510775][ T331] EIP: 0xb7f49545
[ 34.511076][ T331] Code: c4 01 10 03 03 74 c0 01 10 05 03 74 b8 01 10 06 03 74 b4 01 10 07 03 74 b0 01 10 08 03 74 d8 01 00 51 52 55 89 e5 0f 34 cd 80 <5d> 5a 59 c3 90 90 90 90 8d 76 00 58 b8 77 00 00 00 cd 80 90 8d 76
All code
========
0: c4 01 10 03 (bad)
4: 03 74 c0 01 add 0x1(%rax,%rax,8),%esi
8: 10 05 03 74 b8 01 adc %al,0x1b87403(%rip) # 0x1b87411
e: 10 06 adc %al,(%rsi)
10: 03 74 b4 01 add 0x1(%rsp,%rsi,4),%esi
14: 10 07 adc %al,(%rdi)
16: 03 74 b0 01 add 0x1(%rax,%rsi,4),%esi
1a: 10 08 adc %cl,(%rax)
1c: 03 74 d8 01 add 0x1(%rax,%rbx,8),%esi
20: 00 51 52 add %dl,0x52(%rcx)
23: 55 push %rbp
24: 89 e5 mov %esp,%ebp
26: 0f 34 sysenter
28: cd 80 int $0x80
2a:* 5d pop %rbp <-- trapping instruction
2b: 5a pop %rdx
2c: 59 pop %rcx
2d: c3 retq
2e: 90 nop
2f: 90 nop
30: 90 nop
31: 90 nop
32: 8d 76 00 lea 0x0(%rsi),%esi
35: 58 pop %rax
36: b8 77 00 00 00 mov $0x77,%eax
3b: cd 80 int $0x80
3d: 90 nop
3e: 8d .byte 0x8d
3f: 76 .byte 0x76
Code starting with the faulting instruction
===========================================
0: 5d pop %rbp
1: 5a pop %rdx
2: 59 pop %rcx
3: c3 retq
4: 90 nop
5: 90 nop
6: 90 nop
7: 90 nop
8: 8d 76 00 lea 0x0(%rsi),%esi
b: 58 pop %rax
c: b8 77 00 00 00 mov $0x77,%eax
11: cd 80 int $0x80
13: 90 nop
14: 8d .byte 0x8d
15: 76 .byte 0x76
[ 34.512523][ T331] EAX: ffffffda EBX: 00000003 ECX: 00002285 EDX: bf904984
[ 34.512531][ T331] ESI: bf904984 EDI: bf904984 EBP: bf904fdc ESP: bf904918
[ 34.512539][ T331] DS: 007b ES: 007b FS: 0000 GS: 0033 SS: 007b EFLAGS: 00000296
[ 34.512548][ T331] Modules linked in:
[ 34.512813][ T331] ---[ end trace 0000000000000000 ]---
[ 34.512818][ T331] EIP: usercopy_abort (mm/usercopy.c:100 (discriminator 24))
[ 34.512855][ T331] Code: c4 b9 4c 8c 9f c4 ff 75 0c ff 75 08 56 52 53 50 57 51 68 cd 8c 9f c4 e8 16 d6 ff ff 83 05 18 d3 4e c6 01 83 15 1c d3 4e c6 00 <0f> 0b 83 05 20 d3 4e c6 01 83 15 24 d3 4e c6 00 83 05 28 d3 4e c6
All code
========
0: c4 (bad)
1: b9 4c 8c 9f c4 mov $0xc49f8c4c,%ecx
6: ff 75 0c pushq 0xc(%rbp)
9: ff 75 08 pushq 0x8(%rbp)
c: 56 push %rsi
d: 52 push %rdx
e: 53 push %rbx
f: 50 push %rax
10: 57 push %rdi
11: 51 push %rcx
12: 68 cd 8c 9f c4 pushq $0xffffffffc49f8ccd
17: e8 16 d6 ff ff callq 0xffffffffffffd632
1c: 83 05 18 d3 4e c6 01 addl $0x1,-0x39b12ce8(%rip) # 0xffffffffc64ed33b
23: 83 15 1c d3 4e c6 00 adcl $0x0,-0x39b12ce4(%rip) # 0xffffffffc64ed346
2a:* 0f 0b ud2 <-- trapping instruction
2c: 83 05 20 d3 4e c6 01 addl $0x1,-0x39b12ce0(%rip) # 0xffffffffc64ed353
33: 83 15 24 d3 4e c6 00 adcl $0x0,-0x39b12cdc(%rip) # 0xffffffffc64ed35e
3a: 83 .byte 0x83
3b: 05 28 d3 4e c6 add $0xc64ed328,%eax
Code starting with the faulting instruction
===========================================
0: 0f 0b ud2
2: 83 05 20 d3 4e c6 01 addl $0x1,-0x39b12ce0(%rip) # 0xffffffffc64ed329
9: 83 15 24 d3 4e c6 00 adcl $0x0,-0x39b12cdc(%rip) # 0xffffffffc64ed334
10: 83 .byte 0x83
11: 05 28 d3 4e c6 add $0xc64ed328,%eax
To reproduce:
# build kernel
cd linux
cp config-5.17.0-rc1-00234-g6aded12b10e0 .config
make HOSTCC=gcc-9 CC=gcc-9 ARCH=i386 olddefconfig prepare modules_prepare bzImage modules
make HOSTCC=gcc-9 CC=gcc-9 ARCH=i386 INSTALL_MOD_PATH=<mod-install-dir> modules_install
cd <mod-install-dir>
find lib/ | cpio -o -H newc --quiet | gzip > modules.cgz
git clone https://github.com/intel/lkp-tests.git
cd lkp-tests
bin/lkp qemu -k <bzImage> -m modules.cgz job-script # job-script is attached in this email
# if come across any failure that blocks the test,
# please remove ~/.lkp and /lkp dir to run from a clean state.
--
0-DAY CI Kernel Test Service
https://01.org/lkp
The actual warning is;
[ 34.496096][ T331] usercopy: Kernel memory overwrite attempt detected to spans multiple pages (off set 0, size 6)!
This is for the cmnd field in struct scsi_cmnd, which is allocated by
the block layer as part of the request allocator. So with a specific
packing it can legitimately span pages.
Kees: how can we annotate that this is ok?
On Wed, Mar 23, 2022 at 08:40:30AM -0700, Kees Cook wrote:
> Regardless, I'm concerned that disabling PAGESPAN will just uncover
> further checks, though. Where is allocation happening? The check is here:
blk_mq_alloc_rqs, using alloc_pages_node. This hasn't actually changed
with this comment. Just the size of the allocation shrunk, probably
leading to the span of pages.
> I *think* the allocation is happening in scsi_ioctl_reset()? But that's
> a plain kmalloc(), so I'm not sure why PAGESPAN would have tripped...
> are there other allocation paths?
scsi_ioctl_reset is the odd one out and does also allocate a request,
but that request is never used for user copies (and that whole hacky
side path needs to go away, there is a huge series that needs to be
finished to sort this out).
On Wed, Mar 23, 2022 at 08:40:30AM -0700, Kees Cook wrote:
> On Wed, Mar 23, 2022 at 08:14:10AM +0100, Christoph Hellwig wrote:
> > The actual warning is;
> >
> > [ 34.496096][ T331] usercopy: Kernel memory overwrite attempt detected to spans multiple pages (off set 0, size 6)!
> >
> > This is for the cmnd field in struct scsi_cmnd, which is allocated by
> > the block layer as part of the request allocator. So with a specific
> > packing it can legitimately span pages.
> >
> > Kees: how can we annotate that this is ok?
>
> The main problem is that CONFIG_HARDENED_USERCOPY_PAGESPAN=y is broken
> (and nothing should be setting it).
>
> This series removes it:
> https://lore.kernel.org/linux-hardening/[email protected]/
>
> Matthew, what's the status of that series? Will it make the current
> merge window?
I thought you were going to merge it! I haven't put it in any of my
public trees.
> As for the SCSI changes, I'm a bit worried about type confusion, as I
> don't see anything actually validating types/sizes when converting:
>
> static inline void *blk_mq_rq_to_pdu(struct request *rq)
> {
> return rq + 1;
> }
>
> But I guess that ship has sailed. :P
>
> Regardless, I'm concerned that disabling PAGESPAN will just uncover
> further checks, though. Where is allocation happening? The check is here:
>
> static int scsi_fill_sghdr_rq(struct scsi_device *sdev, struct request *rq,
> struct sg_io_hdr *hdr, fmode_t mode)
> {
> struct scsi_cmnd *scmd = blk_mq_rq_to_pdu(rq);
>
> if (hdr->cmd_len < 6)
> return -EMSGSIZE;
> if (copy_from_user(scmd->cmnd, hdr->cmdp, hdr->cmd_len))
> return -EFAULT;
> ...
> }
>
> I don't see any earlier marking for this copy_from_user(), so I assume
> the old allocation was a plain kmalloc().
>
> For comparision, a related marking can be seen for a copy_to_user() case
> in commit 0afe76e88c57 ("scsi: Define usercopy region in scsi_sense_cache
> slab cache")
>
> I *think* the allocation is happening in scsi_ioctl_reset()? But that's
> a plain kmalloc(), so I'm not sure why PAGESPAN would have tripped...
> are there other allocation paths?
>
> --
> Kees Cook
>
On Wed, Mar 23, 2022 at 08:14:10AM +0100, Christoph Hellwig wrote:
> The actual warning is;
>
> [ 34.496096][ T331] usercopy: Kernel memory overwrite attempt detected to spans multiple pages (off set 0, size 6)!
>
> This is for the cmnd field in struct scsi_cmnd, which is allocated by
> the block layer as part of the request allocator. So with a specific
> packing it can legitimately span pages.
>
> Kees: how can we annotate that this is ok?
The main problem is that CONFIG_HARDENED_USERCOPY_PAGESPAN=y is broken
(and nothing should be setting it).
This series removes it:
https://lore.kernel.org/linux-hardening/[email protected]/
Matthew, what's the status of that series? Will it make the current
merge window?
As for the SCSI changes, I'm a bit worried about type confusion, as I
don't see anything actually validating types/sizes when converting:
static inline void *blk_mq_rq_to_pdu(struct request *rq)
{
return rq + 1;
}
But I guess that ship has sailed. :P
Regardless, I'm concerned that disabling PAGESPAN will just uncover
further checks, though. Where is allocation happening? The check is here:
static int scsi_fill_sghdr_rq(struct scsi_device *sdev, struct request *rq,
struct sg_io_hdr *hdr, fmode_t mode)
{
struct scsi_cmnd *scmd = blk_mq_rq_to_pdu(rq);
if (hdr->cmd_len < 6)
return -EMSGSIZE;
if (copy_from_user(scmd->cmnd, hdr->cmdp, hdr->cmd_len))
return -EFAULT;
...
}
I don't see any earlier marking for this copy_from_user(), so I assume
the old allocation was a plain kmalloc().
For comparision, a related marking can be seen for a copy_to_user() case
in commit 0afe76e88c57 ("scsi: Define usercopy region in scsi_sense_cache
slab cache")
I *think* the allocation is happening in scsi_ioctl_reset()? But that's
a plain kmalloc(), so I'm not sure why PAGESPAN would have tripped...
are there other allocation paths?
--
Kees Cook
On Wed, Mar 23, 2022 at 08:40:30AM -0700, Kees Cook wrote:
> This series removes it:
> https://lore.kernel.org/linux-hardening/[email protected]/
If HARDENED_USERCOPY_PAGESPAN is so broken we really should remove it
ASAP independent of the other patches in the series.
On Wed, Mar 23, 2022 at 04:47:39PM +0100, Christoph Hellwig wrote:
> On Wed, Mar 23, 2022 at 08:40:30AM -0700, Kees Cook wrote:
> > Regardless, I'm concerned that disabling PAGESPAN will just uncover
> > further checks, though. Where is allocation happening? The check is here:
>
> blk_mq_alloc_rqs, using alloc_pages_node. This hasn't actually changed
> with this comment. Just the size of the allocation shrunk, probably
> leading to the span of pages.
Okay, the page allocator _should_ be fine for that. In the mean time,
lkp should probably just disable PAGESPAN.
> > I *think* the allocation is happening in scsi_ioctl_reset()? But that's
> > a plain kmalloc(), so I'm not sure why PAGESPAN would have tripped...
> > are there other allocation paths?
>
> scsi_ioctl_reset is the odd one out and does also allocate a request,
> but that request is never used for user copies (and that whole hacky
> side path needs to go away, there is a huge series that needs to be
> finished to sort this out).
Gotcha!
--
Kees Cook
On Wed, Mar 23, 2022 at 03:42:34PM +0000, Matthew Wilcox wrote:
> On Wed, Mar 23, 2022 at 08:40:30AM -0700, Kees Cook wrote:
> > On Wed, Mar 23, 2022 at 08:14:10AM +0100, Christoph Hellwig wrote:
> > > The actual warning is;
> > >
> > > [ 34.496096][ T331] usercopy: Kernel memory overwrite attempt detected to spans multiple pages (off set 0, size 6)!
> > >
> > > This is for the cmnd field in struct scsi_cmnd, which is allocated by
> > > the block layer as part of the request allocator. So with a specific
> > > packing it can legitimately span pages.
> > >
> > > Kees: how can we annotate that this is ok?
> >
> > The main problem is that CONFIG_HARDENED_USERCOPY_PAGESPAN=y is broken
> > (and nothing should be setting it).
> >
> > This series removes it:
> > https://lore.kernel.org/linux-hardening/[email protected]/
> >
> > Matthew, what's the status of that series? Will it make the current
> > merge window?
>
> I thought you were going to merge it! I haven't put it in any of my
> public trees.
LOL. Okay, you'd mentioned another check, so I wasn't sure. I can go
ahead and snag it, but I'll likely wait until the next window and let it
live in -next for a while, unless you think it should get YOLOed in. :)
-Kees
--
Kees Cook