| Message ID | 20191119154059.9440-1-mlombard@redhat.com (mailing list archive) |
|---|---|
| State | Changes Requested |
| Headers | show |
| Series | scsi_debug: check if the max_queue parameter is valid | expand |
Hi Maurizio! > Passing an invalid value to max_queue may cause memory corruption > or kernel freeze. Instead of dealing with each of these parameters individually, maybe it would be a good idea to go through all the int parameters and identify the ones that really should be unsigned? And then tweak their input validators accordingly.
Sanity check makes sense. Reviewed-by: Chad Dupuis <cdupuis1@gmail.com> On Tue, 2019-11-19 at 16:40 +0100, Maurizio Lombardi wrote: > Passing an invalid value to max_queue may cause memory corruption > or kernel freeze. > > E.g. > > [ 1841.074356] INFO: task rmmod:18774 blocked for more than 120 > seconds. > [ 1841.074400] Not tainted 4.18.0-151.el8.ppc64le #1 > [ 1841.074435] "echo 0 > /proc/sys/kernel/hung_task_timeout_secs" > disables this message. > [ 1841.074486] rmmod D 0 18774 17507 0x00040080 > [ 1841.074549] Call Trace: > [ 1841.074569] [c0000001eae2b380] [c00000018ae47600] > 0xc00000018ae47600 (unreliable) > [ 1841.074622] [c0000001eae2b550] [c00000000001f9a0] > __switch_to+0x2e0/0x4e0 > [ 1841.074666] [c0000001eae2b5b0] [c000000000d716b4] > __schedule+0x2c4/0x8e0 > [ 1841.074712] [c0000001eae2b680] [c000000000d71d28] > schedule+0x58/0x120 > [ 1841.074755] [c0000001eae2b6b0] [c000000000188a44] > async_synchronize_cookie_domain+0x174/0x360 > [ 1841.074848] [c0000001eae2b780] [d000000002e228b0] > sd_remove+0x78/0x120 [sd_mod] > [ 1841.074908] [c0000001eae2b7c0] [c0000000008af084] > device_release_driver_internal+0x2d4/0x3f0 > [ 1841.074971] [c0000001eae2b810] [c0000000008ac368] > bus_remove_device+0x128/0x270 > [ 1841.075025] [c0000001eae2b890] [c0000000008a6828] > device_del+0x298/0x590 > [ 1841.075074] [c0000001eae2b940] [c00000000091dfe0] > __scsi_remove_device+0x190/0x1f0 > [ 1841.075127] [c0000001eae2b980] [c000000000919bb4] > scsi_forget_host+0xa4/0xb0 > [ 1841.075180] [c0000001eae2b9b0] [c000000000907a8c] > scsi_remove_host+0xac/0x3a0 > [ 1841.075241] [c0000001eae2ba40] [d000000002624d9c] > sdebug_driver_remove+0x44/0x150 [scsi_debug] > [ 1841.075302] [c0000001eae2bad0] [c0000000008af084] > device_release_driver_internal+0x2d4/0x3f0 > [ 1841.075364] [c0000001eae2bb20] [c0000000008ac368] > bus_remove_device+0x128/0x270 > [ 1841.075417] [c0000001eae2bba0] [c0000000008a6828] > device_del+0x298/0x590 > [ 1841.075461] [c0000001eae2bc50] [c0000000008a6b50] > device_unregister+0x30/0xa0 > [ 1841.075515] [c0000001eae2bcc0] [d000000002623a8c] > sdebug_remove_adapter+0xe4/0x130 [scsi_debug] > [ 1841.075599] [c0000001eae2bd00] [d00000000262eeb8] > scsi_debug_exit+0x50/0x1348 [scsi_debug] > [ 1841.075652] [c0000001eae2bd60] [c0000000002453f0] > sys_delete_module+0x210/0x380 > [ 1841.075706] [c0000001eae2be30] [c00000000000b388] > system_call+0x5c/0x70 > > Signed-off-by: Maurizio Lombardi <mlombard@redhat.com> > --- > drivers/scsi/scsi_debug.c | 5 +++++ > 1 file changed, 5 insertions(+) > > diff --git a/drivers/scsi/scsi_debug.c b/drivers/scsi/scsi_debug.c > index 44cb054d5e66..c2779012d968 100644 > --- a/drivers/scsi/scsi_debug.c > +++ b/drivers/scsi/scsi_debug.c > @@ -5268,6 +5268,11 @@ static int __init scsi_debug_init(void) > return -EINVAL; > } > > + if (sdebug_max_queue <= 0 || sdebug_max_queue > > SDEBUG_CANQUEUE) { > + pr_err("max_queue must be > 0 and <= %d\n", > SDEBUG_CANQUEUE); > + return -EINVAL; > + } > + > if (sdebug_guard > 1) { > pr_err("guard must be 0 or 1\n"); > return -EINVAL;
Hi Martin! Dne 20.11.2019 v 04:20 Martin K. Petersen napsal(a): > > Instead of dealing with each of these parameters individually, maybe it > would be a good idea to go through all the int parameters and identify > the ones that really should be unsigned? And then tweak their input > validators accordingly. > Ok I will go through the parameters and convert them to unsigned where it makes sense. Maurizio
diff --git a/drivers/scsi/scsi_debug.c b/drivers/scsi/scsi_debug.c index 44cb054d5e66..c2779012d968 100644 --- a/drivers/scsi/scsi_debug.c +++ b/drivers/scsi/scsi_debug.c @@ -5268,6 +5268,11 @@ static int __init scsi_debug_init(void) return -EINVAL; } + if (sdebug_max_queue <= 0 || sdebug_max_queue > SDEBUG_CANQUEUE) { + pr_err("max_queue must be > 0 and <= %d\n", SDEBUG_CANQUEUE); + return -EINVAL; + } + if (sdebug_guard > 1) { pr_err("guard must be 0 or 1\n"); return -EINVAL;
Passing an invalid value to max_queue may cause memory corruption or kernel freeze. E.g. [ 1841.074356] INFO: task rmmod:18774 blocked for more than 120 seconds. [ 1841.074400] Not tainted 4.18.0-151.el8.ppc64le #1 [ 1841.074435] "echo 0 > /proc/sys/kernel/hung_task_timeout_secs" disables this message. [ 1841.074486] rmmod D 0 18774 17507 0x00040080 [ 1841.074549] Call Trace: [ 1841.074569] [c0000001eae2b380] [c00000018ae47600] 0xc00000018ae47600 (unreliable) [ 1841.074622] [c0000001eae2b550] [c00000000001f9a0] __switch_to+0x2e0/0x4e0 [ 1841.074666] [c0000001eae2b5b0] [c000000000d716b4] __schedule+0x2c4/0x8e0 [ 1841.074712] [c0000001eae2b680] [c000000000d71d28] schedule+0x58/0x120 [ 1841.074755] [c0000001eae2b6b0] [c000000000188a44] async_synchronize_cookie_domain+0x174/0x360 [ 1841.074848] [c0000001eae2b780] [d000000002e228b0] sd_remove+0x78/0x120 [sd_mod] [ 1841.074908] [c0000001eae2b7c0] [c0000000008af084] device_release_driver_internal+0x2d4/0x3f0 [ 1841.074971] [c0000001eae2b810] [c0000000008ac368] bus_remove_device+0x128/0x270 [ 1841.075025] [c0000001eae2b890] [c0000000008a6828] device_del+0x298/0x590 [ 1841.075074] [c0000001eae2b940] [c00000000091dfe0] __scsi_remove_device+0x190/0x1f0 [ 1841.075127] [c0000001eae2b980] [c000000000919bb4] scsi_forget_host+0xa4/0xb0 [ 1841.075180] [c0000001eae2b9b0] [c000000000907a8c] scsi_remove_host+0xac/0x3a0 [ 1841.075241] [c0000001eae2ba40] [d000000002624d9c] sdebug_driver_remove+0x44/0x150 [scsi_debug] [ 1841.075302] [c0000001eae2bad0] [c0000000008af084] device_release_driver_internal+0x2d4/0x3f0 [ 1841.075364] [c0000001eae2bb20] [c0000000008ac368] bus_remove_device+0x128/0x270 [ 1841.075417] [c0000001eae2bba0] [c0000000008a6828] device_del+0x298/0x590 [ 1841.075461] [c0000001eae2bc50] [c0000000008a6b50] device_unregister+0x30/0xa0 [ 1841.075515] [c0000001eae2bcc0] [d000000002623a8c] sdebug_remove_adapter+0xe4/0x130 [scsi_debug] [ 1841.075599] [c0000001eae2bd00] [d00000000262eeb8] scsi_debug_exit+0x50/0x1348 [scsi_debug] [ 1841.075652] [c0000001eae2bd60] [c0000000002453f0] sys_delete_module+0x210/0x380 [ 1841.075706] [c0000001eae2be30] [c00000000000b388] system_call+0x5c/0x70 Signed-off-by: Maurizio Lombardi <mlombard@redhat.com> --- drivers/scsi/scsi_debug.c | 5 +++++ 1 file changed, 5 insertions(+)