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@xxxxxxxxxx>
> ---
> 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;
> }
>
--
Damien Le Moal
Western Digital Research
