Message ID | 20221107125942.143769-2-yangyingliang@huawei.com |
---|---|
State | New |
Headers | show |
Series | ata: libata-transport: fix some error handing | expand |
On 11/7/22 21:59, Yang Yingliang wrote: > If transport_add_device() fails in ata_tport_add(), it's not handled, > it will lead kernel crash because of trying to delete not added device > in transport_remove_device() called from ata_tport_delete(). Simplify your sentences to make them easier to understand: In ata_tport_add(), the return value of transport_add_device() is not checked. As a result, another error after that function call leads to a kernel crash (null pointer dereference) because transport_remove_device() is called to remove a device that was not added. Please fix this. The patch itself is OK. > > Unable to handle kernel NULL pointer dereference at virtual address 00000000000000d0 > CPU: 12 PID: 13605 Comm: rmmod Kdump: loaded Tainted: G W 6.1.0-rc3+ #8 > pstate: 60400009 (nZCv daif +PAN -UAO -TCO -DIT -SSBS BTYPE=--) > pc : device_del+0x48/0x39c > lr : device_del+0x44/0x39c > Call trace: > 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 > ata_tport_delete+0x34/0x60 [libata] > ata_port_detach+0x148/0x1b0 [libata] > ata_pci_remove_one+0x50/0x80 [libata] > ahci_remove_one+0x4c/0x8c [ahci] > > Fix this by checking and handling return value of transport_add_device() > in ata_tport_add(). > > Fixes: d9027470b886 ("[libata] Add ATA transport class") > Signed-off-by: Yang Yingliang <yangyingliang@huawei.com> > --- > drivers/ata/libata-transport.c | 5 ++++- > 1 file changed, 4 insertions(+), 1 deletion(-) > > diff --git a/drivers/ata/libata-transport.c b/drivers/ata/libata-transport.c > index 105da3ec5eaa..ef53bdfbcbb2 100644 > --- a/drivers/ata/libata-transport.c > +++ b/drivers/ata/libata-transport.c > @@ -301,7 +301,9 @@ int ata_tport_add(struct device *parent, > pm_runtime_enable(dev); > pm_runtime_forbid(dev); > > - transport_add_device(dev); > + error = transport_add_device(dev); > + if (error) > + goto tport_transport_add_err; > transport_configure_device(dev); > > error = ata_tlink_add(&ap->link); > @@ -312,6 +314,7 @@ int ata_tport_add(struct device *parent, > > tport_link_err: > transport_remove_device(dev); > + tport_transport_add_err: > device_del(dev); > > tport_err:
diff --git a/drivers/ata/libata-transport.c b/drivers/ata/libata-transport.c index 105da3ec5eaa..ef53bdfbcbb2 100644 --- a/drivers/ata/libata-transport.c +++ b/drivers/ata/libata-transport.c @@ -301,7 +301,9 @@ int ata_tport_add(struct device *parent, pm_runtime_enable(dev); pm_runtime_forbid(dev); - transport_add_device(dev); + error = transport_add_device(dev); + if (error) + goto tport_transport_add_err; transport_configure_device(dev); error = ata_tlink_add(&ap->link); @@ -312,6 +314,7 @@ int ata_tport_add(struct device *parent, tport_link_err: transport_remove_device(dev); + tport_transport_add_err: device_del(dev); tport_err:
If transport_add_device() fails in ata_tport_add(), it's not handled, it will lead kernel crash because of trying to delete not added device in transport_remove_device() called from ata_tport_delete(). Unable to handle kernel NULL pointer dereference at virtual address 00000000000000d0 CPU: 12 PID: 13605 Comm: rmmod Kdump: loaded Tainted: G W 6.1.0-rc3+ #8 pstate: 60400009 (nZCv daif +PAN -UAO -TCO -DIT -SSBS BTYPE=--) pc : device_del+0x48/0x39c lr : device_del+0x44/0x39c Call trace: 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 ata_tport_delete+0x34/0x60 [libata] ata_port_detach+0x148/0x1b0 [libata] ata_pci_remove_one+0x50/0x80 [libata] ahci_remove_one+0x4c/0x8c [ahci] Fix this by checking and handling return value of transport_add_device() in ata_tport_add(). Fixes: d9027470b886 ("[libata] Add ATA transport class") Signed-off-by: Yang Yingliang <yangyingliang@huawei.com> --- drivers/ata/libata-transport.c | 5 ++++- 1 file changed, 4 insertions(+), 1 deletion(-)