| Message ID | 20221110142917.373925-1-yangyingliang@huawei.com (mailing list archive) |
|---|---|
| State | Changes Requested |
| Headers | show |
| Series | scsi: core: sysfs: fix possible null-ptr-deref | expand |
On 11/10/22 8:29 AM, Yang Yingliang wrote: > In scsi_sysfs_add_host(), transport_register_device() may return > error, if it's not handled, it will cause a null-ptr-deref while > removing module, because transport_remove_device() is called to > delete a device that not added. > > Unable to handle kernel NULL pointer dereference at virtual address 00000000000000d0 > CPU: 12 PID: 14570 Comm: rmmod Kdump: loaded Not tainted 6.1.0-rc3+ #25 > pstate: 80400009 (Nzcv daif +PAN -UAO -TCO -DIT -SSBS BTYPE=--) > pc : device_del+0x48/0x39c > lr : device_del+0x44/0x39c > device_del+0x48/0x39c > attribute_container_class_device_del+0x28/0x40 > transport_remove_classdev+0x60/0x7c > attribute_container_device_trigger+0x118/0x120 > transport_remove_device+0x20/0x30 > scsi_remove_host+0x150/0x26c > sas_remove_host+0x54/0x70 [scsi_transport_sas] > hisi_sas_v3_remove+0x5c/0xbc [hisi_sas_v3_hw] > > Fix this by checking and handling return value of transport_register_device(). > > Fixes: 1da177e4c3f4 ("Linux-2.6.12-rc2") > Signed-off-by: Yang Yingliang <yangyingliang@huawei.com> > --- > drivers/scsi/scsi_sysfs.c | 6 +++++- > 1 file changed, 5 insertions(+), 1 deletion(-) > > diff --git a/drivers/scsi/scsi_sysfs.c b/drivers/scsi/scsi_sysfs.c > index cac7c902cf70..41de795f1ab2 100644 > --- a/drivers/scsi/scsi_sysfs.c > +++ b/drivers/scsi/scsi_sysfs.c > @@ -1607,7 +1607,11 @@ EXPORT_SYMBOL(scsi_register_interface); > **/ > int scsi_sysfs_add_host(struct Scsi_Host *shost) > { > - transport_register_device(&shost->shost_gendev); > + int ret; > + > + ret = transport_register_device(&shost->shost_gendev); > + if (ret) > + return ret; > transport_configure_device(&shost->shost_gendev); > return 0; > } Do we need to do something similar every time transport_add_device fails? For example, in ata_tport_add if transport_add_device and we later do transport_destroy_device in ata_tlink_delete? The SAS and FC classes do something similar and in SCSI-ml we have other cases for device and target additions.
Hi Mike, On 2022/11/11 2:03, Mike Christie wrote: > On 11/10/22 8:29 AM, Yang Yingliang wrote: >> In scsi_sysfs_add_host(), transport_register_device() may return >> error, if it's not handled, it will cause a null-ptr-deref while >> removing module, because transport_remove_device() is called to >> delete a device that not added. >> >> Unable to handle kernel NULL pointer dereference at virtual address 00000000000000d0 >> CPU: 12 PID: 14570 Comm: rmmod Kdump: loaded Not tainted 6.1.0-rc3+ #25 >> pstate: 80400009 (Nzcv daif +PAN -UAO -TCO -DIT -SSBS BTYPE=--) >> pc : device_del+0x48/0x39c >> lr : device_del+0x44/0x39c >> device_del+0x48/0x39c >> attribute_container_class_device_del+0x28/0x40 >> transport_remove_classdev+0x60/0x7c >> attribute_container_device_trigger+0x118/0x120 >> transport_remove_device+0x20/0x30 >> scsi_remove_host+0x150/0x26c >> sas_remove_host+0x54/0x70 [scsi_transport_sas] >> hisi_sas_v3_remove+0x5c/0xbc [hisi_sas_v3_hw] >> >> Fix this by checking and handling return value of transport_register_device(). >> >> Fixes: 1da177e4c3f4 ("Linux-2.6.12-rc2") >> Signed-off-by: Yang Yingliang <yangyingliang@huawei.com> >> --- >> drivers/scsi/scsi_sysfs.c | 6 +++++- >> 1 file changed, 5 insertions(+), 1 deletion(-) >> >> diff --git a/drivers/scsi/scsi_sysfs.c b/drivers/scsi/scsi_sysfs.c >> index cac7c902cf70..41de795f1ab2 100644 >> --- a/drivers/scsi/scsi_sysfs.c >> +++ b/drivers/scsi/scsi_sysfs.c >> @@ -1607,7 +1607,11 @@ EXPORT_SYMBOL(scsi_register_interface); >> **/ >> int scsi_sysfs_add_host(struct Scsi_Host *shost) >> { >> - transport_register_device(&shost->shost_gendev); >> + int ret; >> + >> + ret = transport_register_device(&shost->shost_gendev); >> + if (ret) >> + return ret; >> transport_configure_device(&shost->shost_gendev); >> return 0; >> } > Do we need to do something similar every time transport_add_device > fails? For example, in ata_tport_add if transport_add_device > and we later do transport_destroy_device in ata_tlink_delete? > The SAS and FC classes do something similar and in SCSI-ml we > have other cases for device and target additions. Yes, we need, and I think the normal call sequence of using transport class is: Add path: transport_setup_device() // allocate device transport_add_device() // add device, if it fails, need call transport_destroy_device() transport_configure_device() Remove path: transport_remove_device() // remove device transport_destroy_device() // free device If device is not added, can not call remove function, just call destroy function to free the memory. Thanks, Yang > > .
diff --git a/drivers/scsi/scsi_sysfs.c b/drivers/scsi/scsi_sysfs.c index cac7c902cf70..41de795f1ab2 100644 --- a/drivers/scsi/scsi_sysfs.c +++ b/drivers/scsi/scsi_sysfs.c @@ -1607,7 +1607,11 @@ EXPORT_SYMBOL(scsi_register_interface); **/ int scsi_sysfs_add_host(struct Scsi_Host *shost) { - transport_register_device(&shost->shost_gendev); + int ret; + + ret = transport_register_device(&shost->shost_gendev); + if (ret) + return ret; transport_configure_device(&shost->shost_gendev); return 0; }
In scsi_sysfs_add_host(), transport_register_device() may return error, if it's not handled, it will cause a null-ptr-deref while removing module, because transport_remove_device() is called to delete a device that not added. Unable to handle kernel NULL pointer dereference at virtual address 00000000000000d0 CPU: 12 PID: 14570 Comm: rmmod Kdump: loaded Not tainted 6.1.0-rc3+ #25 pstate: 80400009 (Nzcv daif +PAN -UAO -TCO -DIT -SSBS BTYPE=--) pc : device_del+0x48/0x39c lr : device_del+0x44/0x39c device_del+0x48/0x39c attribute_container_class_device_del+0x28/0x40 transport_remove_classdev+0x60/0x7c attribute_container_device_trigger+0x118/0x120 transport_remove_device+0x20/0x30 scsi_remove_host+0x150/0x26c sas_remove_host+0x54/0x70 [scsi_transport_sas] hisi_sas_v3_remove+0x5c/0xbc [hisi_sas_v3_hw] Fix this by checking and handling return value of transport_register_device(). Fixes: 1da177e4c3f4 ("Linux-2.6.12-rc2") Signed-off-by: Yang Yingliang <yangyingliang@huawei.com> --- drivers/scsi/scsi_sysfs.c | 6 +++++- 1 file changed, 5 insertions(+), 1 deletion(-)