diff mbox series

[1/3] ata: libata-transport: fix error handling in ata_tport_add()

Message ID 20221107125942.143769-2-yangyingliang@huawei.com
State New
Headers show
Series ata: libata-transport: fix some error handing | expand

Commit Message

Yang Yingliang Nov. 7, 2022, 12:59 p.m. UTC
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(-)

Comments

Damien Le Moal Nov. 8, 2022, 6:47 a.m. UTC | #1
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 mbox series

Patch

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: