diff mbox series

scsi: scsi_transport_sas: fix error handling in sas_port_add()

Message ID 20221111132452.2385508-1-yangyingliang@huawei.com (mailing list archive)
State Superseded
Headers show
Series scsi: scsi_transport_sas: fix error handling in sas_port_add() | expand

Commit Message

Yang Yingliang Nov. 11, 2022, 1:24 p.m. UTC
In sas_port_add(), the return value of transport_add_device() is
not checked. As a result, it causes null-ptr-deref while removing
device, because transport_remove_device() is called to remove the
device that was not added.

Unable to handle kernel NULL pointer dereference at virtual address 0000000000000108
pc : device_del+0x54/0x3d0
lr : device_del+0x37c/0x3d0
Call trace:
 device_del+0x54/0x3d0
 attribute_container_class_device_del+0x28/0x38
 transport_remove_classdev+0x6c/0x80
 attribute_container_device_trigger+0x108/0x110
 transport_remove_device+0x28/0x38
 sas_port_delete+0x110/0x148 [scsi_transport_sas]
 do_sas_phy_delete+0x78/0x80 [scsi_transport_sas]
 device_for_each_child+0x68/0xb0
 sas_remove_children+0x30/0x50 [scsi_transport_sas]
 sas_rphy_remove+0x38/0x78 [scsi_transport_sas]
 sas_port_delete+0x30/0x148 [scsi_transport_sas]
 do_sas_phy_delete+0x78/0x80 [scsi_transport_sas]
 device_for_each_child+0x68/0xb0
 sas_remove_children+0x30/0x50 [scsi_transport_sas]
 sas_remove_host+0x20/0x38 [scsi_transport_sas]
 scsih_remove+0xd8/0x420 [mpt3sas]

Fix this by checking and handling return value of transport_add_device()
in sas_port_add().

Fixes: 65c92b09acf0 ("[SCSI] scsi_transport_sas: introduce a sas_port entity")
Signed-off-by: Yang Yingliang <yangyingliang@huawei.com>
---
 drivers/scsi/scsi_transport_sas.c | 6 +++++-
 1 file changed, 5 insertions(+), 1 deletion(-)

Comments

John Garry Nov. 11, 2022, 1:46 p.m. UTC | #1
On 11/11/2022 13:24, Yang Yingliang wrote:
> In sas_port_add(), the return value of transport_add_device() is
> not checked. As a result, it causes null-ptr-deref while removing
> device, because transport_remove_device() is called to remove the
> device that was not added.

This makes it sound like we have the null-ptr-deref always, which would 
not be the case.

You need to make it clear that we don't check for an error in the add 
and we may later go on to try to remove a device which was never 
successfully added, causing the null-ptr-deref.

> 
Apart from comment, above:
Reviewed-by: John Garry <john.g.garry@oracle.com>
diff mbox series

Patch

diff --git a/drivers/scsi/scsi_transport_sas.c b/drivers/scsi/scsi_transport_sas.c
index accc0afa8f77..e090486258a5 100644
--- a/drivers/scsi/scsi_transport_sas.c
+++ b/drivers/scsi/scsi_transport_sas.c
@@ -959,7 +959,11 @@  int sas_port_add(struct sas_port *port)
 	if (error)
 		return error;
 
-	transport_add_device(&port->dev);
+	error = transport_add_device(&port->dev);
+	if (error) {
+		device_del(&port->dev);
+		return error;
+	}
 	transport_configure_device(&port->dev);
 
 	return 0;