[PATCH] scsi: libsas: Fix a NULL pointer dereference in sas_ex_discover_expander()

From: Zhou Qingyang <zhou1615@umn.edu>
To: zhou1615@umn.edu
Cc: kjlu@umn.edu, "James E.J. Bottomley" <jejb@linux.ibm.com>,
	"Martin K. Petersen" <martin.petersen@oracle.com>,
	John Garry <john.garry@huawei.com>,
	Jason Yan <yanaijie@huawei.com>,
	Himanshu Madhani <himanshu.madhani@oracle.com>,
	Jack Wang <jinpu.wang@ionos.com>,
	Luo Jiaxing <luojiaxing@huawei.com>,
	Bart Van Assche <bvanassche@acm.org>,
	James Bottomley <James.Bottomley@SteelEye.com>,
	linux-scsi@vger.kernel.org, linux-kernel@vger.kernel.org
Subject: [PATCH] scsi: libsas: Fix a NULL pointer dereference in sas_ex_discover_expander()
Date: Wed,  1 Dec 2021 01:16:26 +0800	[thread overview]
Message-ID: <20211130171629.201026-1-zhou1615@umn.edu> (raw)

In sas_ex_discover_expander(), sas_port_alloc() is assigned to phy->port
and used in sas_port_add(). sas_port_add() further passes phy->port to
list_empty(), and there is a dereference of it in list_empty(), which
could lead to a NULL pointer dereference on failure of
sas_port_alloc().

This patch imitates the same error-handling logic in
sas_ex_discover_end_dev().

Fix this bug by adding checks for phy->port and sas_port_add().

This bug was found by a static analyzer. The analysis employs
differential checking to identify inconsistent security operations
(e.g., checks or kfrees) between two code paths and confirms that the
inconsistent operations are not recovered in the current function or
the callers, so they constitute bugs.

Note that, as a bug found by static analysis, it can be a false
positive or hard to trigger. Multiple researchers have cross-reviewed
the bug.

Builds with CONFIG_SCSI_SAS_LIBSAS=m show no new warnings,
and our static analyzer no longer warns about this code.

Fixes:  2908d778ab3e ("[SCSI] aic94xx: new driver")
Signed-off-by: Zhou Qingyang <zhou1615@umn.edu>
---
 drivers/scsi/libsas/sas_expander.c | 11 +++++++++--
 1 file changed, 9 insertions(+), 2 deletions(-)

diff --git a/drivers/scsi/libsas/sas_expander.c b/drivers/scsi/libsas/sas_expander.c
index c2150a818423..7530b1773d6b 100644
--- a/drivers/scsi/libsas/sas_expander.c
+++ b/drivers/scsi/libsas/sas_expander.c
@@ -957,9 +957,16 @@ static struct domain_device *sas_ex_discover_expander(
 		return NULL;
 
 	phy->port = sas_port_alloc(&parent->rphy->dev, phy_id);
-	/* FIXME: better error handling */
-	BUG_ON(sas_port_add(phy->port) != 0);
+	if (unlikely(!phy->port)) {
+		sas_put_device(child);
+		return NULL;
+	}
 
+	if (sas_port_add(phy->port) != 0) {
+		sas_port_free(phy->port);
+		sas_put_device(child);
+		return NULL;
+	}
 
 	switch (phy->attached_dev_type) {
 	case SAS_EDGE_EXPANDER_DEVICE:
-- 
2.25.1

