| Message ID | 20230731084034.37021-1-oleksandr@redhat.com (mailing list archive) |
|---|---|
| Headers | show |
| Series | scsi: qedf: sanitise uaccess | expand |
On Mon, 2023-07-31 at 10:40 +0200, Oleksandr Natalenko wrote: > qedf driver, debugfs part of it specifically, touches __user pointers > directly for printing out info to userspace via sprintf(), which may > cause crash like this: > > BUG: unable to handle kernel paging request at 00007ffd1d6b43a0 > IP: [<ffffffffaa7a882a>] string.isra.7+0x6a/0xf0 > Oops: 0003 [#1] SMP > Call Trace: > [<ffffffffaa7a9f31>] vsnprintf+0x201/0x6a0 > [<ffffffffaa7aa556>] sprintf+0x56/0x80 > [<ffffffffc04227ed>] qedf_dbg_stop_io_on_error_cmd_read+0x6d/0x90 > [qedf] > [<ffffffffaa65bb2f>] vfs_read+0x9f/0x170 > [<ffffffffaa65cb82>] SyS_pread64+0x92/0xc0 > > Avoid this by preparing the info in a kernel buffer first, either > allocated on stack for small printouts, or via vmalloc() for big > ones, > and then copying it to the userspace properly. > > Changes since v1 [1]: > > * use scnprintf() for on-stack buffers too > * adjust an on-stack buffer size in qedf_dbg_debug_cmd_read() to be > a > multiple of 8, and also size it properly > * accumulate acks and reviews > > [1] > https://lore.kernel.org/lkml/20230728065819.139694-1-oleksandr@redhat.com/ > > Oleksandr Natalenko (3): > scsi: qedf: do not touch __user pointer in > qedf_dbg_stop_io_on_error_cmd_read() directly > scsi: qedf: do not touch __user pointer in > qedf_dbg_debug_cmd_read() > directly > scsi: qedf: do not touch __user pointer in > qedf_dbg_fp_int_cmd_read() > directly > > drivers/scsi/qedf/qedf_dbg.h | 2 ++ > drivers/scsi/qedf/qedf_debugfs.c | 35 +++++++++++++++++++----------- > -- > 2 files changed, 23 insertions(+), 14 deletions(-) > In case it's needed For the series v2 against 6.5-rc3 Reviewed-by: Laurence Oberman <loberman@redhat.com> Tested-by: Laurence Oberman <loberman@redhat.com> Test notes Linux segstorage5 6.5.0-rc3+ [root@segstorage5 qedf]# cd host2 [root@segstorage5 host2]# ls clear_stats debug driver_stats fp_int io_trace offload_stats stop_io_on_error [root@segstorage5 host2]# cat stop_io_on_error false [root@segstorage5 host2]# cat fp_int Fastpath I/O completions #0: 844 #1: 990 #2: 1036 #3: 1116 #4: 953 #5: 822 #6: 882 #7: 1073 #8: 1030 #9: 992 #10: 789 #11: 705 #12: 490 #13: 532 #14: 646 #15: 705 [root@segstorage5 host2]# cat debug debug mask = 0x2
Oleksandr, > qedf driver, debugfs part of it specifically, touches __user pointers > directly for printing out info to userspace via sprintf(), which may > cause crash like this: Applied to 6.6/scsi-staging, thanks!
On pondělí 31. července 2023 20:43:34 CEST Martin K. Petersen wrote: > > Oleksandr, > > > qedf driver, debugfs part of it specifically, touches __user pointers > > directly for printing out info to userspace via sprintf(), which may > > cause crash like this: > > Applied to 6.6/scsi-staging, thanks! Thank you all.
On Mon, 31 Jul 2023 10:40:31 +0200, Oleksandr Natalenko wrote: > qedf driver, debugfs part of it specifically, touches __user pointers > directly for printing out info to userspace via sprintf(), which may > cause crash like this: > > BUG: unable to handle kernel paging request at 00007ffd1d6b43a0 > IP: [<ffffffffaa7a882a>] string.isra.7+0x6a/0xf0 > Oops: 0003 [#1] SMP > Call Trace: > [<ffffffffaa7a9f31>] vsnprintf+0x201/0x6a0 > [<ffffffffaa7aa556>] sprintf+0x56/0x80 > [<ffffffffc04227ed>] qedf_dbg_stop_io_on_error_cmd_read+0x6d/0x90 [qedf] > [<ffffffffaa65bb2f>] vfs_read+0x9f/0x170 > [<ffffffffaa65cb82>] SyS_pread64+0x92/0xc0 > > [...] Applied to 6.6/scsi-queue, thanks! [1/3] scsi: qedf: do not touch __user pointer in qedf_dbg_stop_io_on_error_cmd_read() directly https://git.kernel.org/mkp/scsi/c/7d3d20dee4f6 [2/3] scsi: qedf: do not touch __user pointer in qedf_dbg_debug_cmd_read() directly https://git.kernel.org/mkp/scsi/c/31b5991a9a91 [3/3] scsi: qedf: do not touch __user pointer in qedf_dbg_fp_int_cmd_read() directly https://git.kernel.org/mkp/scsi/c/25dbc20deab5
qedf driver, debugfs part of it specifically, touches __user pointers directly for printing out info to userspace via sprintf(), which may cause crash like this: BUG: unable to handle kernel paging request at 00007ffd1d6b43a0 IP: [<ffffffffaa7a882a>] string.isra.7+0x6a/0xf0 Oops: 0003 [#1] SMP Call Trace: [<ffffffffaa7a9f31>] vsnprintf+0x201/0x6a0 [<ffffffffaa7aa556>] sprintf+0x56/0x80 [<ffffffffc04227ed>] qedf_dbg_stop_io_on_error_cmd_read+0x6d/0x90 [qedf] [<ffffffffaa65bb2f>] vfs_read+0x9f/0x170 [<ffffffffaa65cb82>] SyS_pread64+0x92/0xc0 Avoid this by preparing the info in a kernel buffer first, either allocated on stack for small printouts, or via vmalloc() for big ones, and then copying it to the userspace properly. Changes since v1 [1]: * use scnprintf() for on-stack buffers too * adjust an on-stack buffer size in qedf_dbg_debug_cmd_read() to be a multiple of 8, and also size it properly * accumulate acks and reviews [1] https://lore.kernel.org/lkml/20230728065819.139694-1-oleksandr@redhat.com/ Oleksandr Natalenko (3): scsi: qedf: do not touch __user pointer in qedf_dbg_stop_io_on_error_cmd_read() directly scsi: qedf: do not touch __user pointer in qedf_dbg_debug_cmd_read() directly scsi: qedf: do not touch __user pointer in qedf_dbg_fp_int_cmd_read() directly drivers/scsi/qedf/qedf_dbg.h | 2 ++ drivers/scsi/qedf/qedf_debugfs.c | 35 +++++++++++++++++++------------- 2 files changed, 23 insertions(+), 14 deletions(-)