On 11/7/22 21:59, Yang Yingliang wrote:
> If transport_add_device() fails in ata_tlink_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_tlink_delete().
See my comment on patch 1 for how to make this more readable.
Other than that, the fix looks OK.
>
> Unable to handle kernel NULL pointer dereference at virtual address 00000000000000d0
> CPU: 33 PID: 13850 Comm: rmmod Kdump: loaded Tainted: G W 6.1.0-rc3+ #12
> 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_tlink_delete+0x88/0xb0 [libata]
> ata_tport_delete+0x2c/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_tlink_add().
>
> Fixes: d9027470b886 ("[libata] Add ATA transport class")
> Signed-off-by: Yang Yingliang <yangyingliang@xxxxxxxxxx>
> ---
> 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 ef53bdfbcbb2..aac9336e8153 100644
> --- a/drivers/ata/libata-transport.c
> +++ b/drivers/ata/libata-transport.c
> @@ -458,7 +458,9 @@ int ata_tlink_add(struct ata_link *link)
> goto tlink_err;
> }
>
> - transport_add_device(dev);
> + error = transport_add_device(dev);
> + if (error)
> + goto tlink_transport_err;
> transport_configure_device(dev);
>
> ata_for_each_dev(ata_dev, link, ALL) {
> @@ -473,6 +475,7 @@ int ata_tlink_add(struct ata_link *link)
> ata_tdev_delete(ata_dev);
> }
> transport_remove_device(dev);
> + tlink_transport_err:
> device_del(dev);
> tlink_err:
> transport_destroy_device(dev);
--
Damien Le Moal
Western Digital Research
