| Message ID | 20221107084928.795538-1-yangyingliang@huawei.com |
|---|---|
| State | New |
| Headers | show |
| Series | ata: libata-transport: fix double ata_host_put() in ata_tport_add() | expand |
On 11/7/22 17:49, Yang Yingliang wrote: > In the error path in ata_tport_add(), the refcount of ata_host is > already be put in ata_tport_release() after calling put_device(), > it makes the host and ports are released earlier and the ports are > set to null, then it leads a null-ptr-deref in ata_host_stop(): The fix looks OK, but I did not thoroughly check because the commit message does not parse well. Please make shorter, simpler sentences that can be easily understood (and that will be easier to write too). Describe the call chain that leads to the problem because you are mentioning functions that are nowhere to be seen in ata_tport_add(). > > Unable to handle kernel NULL pointer dereference at virtual address 0000000000000008 > CPU: 7 PID: 18671 Comm: modprobe Kdump: loaded Tainted: G E 6.1.0-rc3+ #8 > pstate: 80400009 (Nzcv daif +PAN -UAO -TCO -DIT -SSBS BTYPE=--) > pc : ata_host_stop+0x3c/0x84 [libata] > lr : release_nodes+0x64/0xd0 > Call trace: > ata_host_stop+0x3c/0x84 [libata] > release_nodes+0x64/0xd0 > devres_release_all+0xbc/0x1b0 > device_unbind_cleanup+0x20/0x70 > really_probe+0x158/0x320 > __driver_probe_device+0x84/0x120 > driver_probe_device+0x44/0x120 > __driver_attach+0xb4/0x220 > bus_for_each_dev+0x78/0xdc > driver_attach+0x2c/0x40 > bus_add_driver+0x184/0x240 > driver_register+0x80/0x13c > __pci_register_driver+0x4c/0x60 > ahci_pci_driver_init+0x30/0x1000 [ahci] > > Fix this by remove redundant ata_host_put() in the error path.> > Fixes: 2623c7a5f279 ("libata: add refcounting to ata_host") > Signed-off-by: Yang Yingliang <yangyingliang@huawei.com> > --- > drivers/ata/libata-transport.c | 1 - > 1 file changed, 1 deletion(-) > > diff --git a/drivers/ata/libata-transport.c b/drivers/ata/libata-transport.c > index a7e9a75410a3..105da3ec5eaa 100644 > --- a/drivers/ata/libata-transport.c > +++ b/drivers/ata/libata-transport.c > @@ -317,7 +317,6 @@ int ata_tport_add(struct device *parent, > tport_err: > transport_destroy_device(dev); > put_device(dev); > - ata_host_put(ap->host); > return error; > } >
diff --git a/drivers/ata/libata-transport.c b/drivers/ata/libata-transport.c index a7e9a75410a3..105da3ec5eaa 100644 --- a/drivers/ata/libata-transport.c +++ b/drivers/ata/libata-transport.c @@ -317,7 +317,6 @@ int ata_tport_add(struct device *parent, tport_err: transport_destroy_device(dev); put_device(dev); - ata_host_put(ap->host); return error; }
In the error path in ata_tport_add(), the refcount of ata_host is already be put in ata_tport_release() after calling put_device(), it makes the host and ports are released earlier and the ports are set to null, then it leads a null-ptr-deref in ata_host_stop(): Unable to handle kernel NULL pointer dereference at virtual address 0000000000000008 CPU: 7 PID: 18671 Comm: modprobe Kdump: loaded Tainted: G E 6.1.0-rc3+ #8 pstate: 80400009 (Nzcv daif +PAN -UAO -TCO -DIT -SSBS BTYPE=--) pc : ata_host_stop+0x3c/0x84 [libata] lr : release_nodes+0x64/0xd0 Call trace: ata_host_stop+0x3c/0x84 [libata] release_nodes+0x64/0xd0 devres_release_all+0xbc/0x1b0 device_unbind_cleanup+0x20/0x70 really_probe+0x158/0x320 __driver_probe_device+0x84/0x120 driver_probe_device+0x44/0x120 __driver_attach+0xb4/0x220 bus_for_each_dev+0x78/0xdc driver_attach+0x2c/0x40 bus_add_driver+0x184/0x240 driver_register+0x80/0x13c __pci_register_driver+0x4c/0x60 ahci_pci_driver_init+0x30/0x1000 [ahci] Fix this by remove redundant ata_host_put() in the error path. Fixes: 2623c7a5f279 ("libata: add refcounting to ata_host") Signed-off-by: Yang Yingliang <yangyingliang@huawei.com> --- drivers/ata/libata-transport.c | 1 - 1 file changed, 1 deletion(-)