Re: [PATCH v3 1/3] scsi: libsas: Handle errors in sas_ex_add_parent_port()

Thank you for your contribution! Sashiko AI review found 1 potential issue(s) to consider:

Pre-existing issues:
- [High] Unchecked return value of `sas_port_alloc()` in `sas_ex_discover_expander()` leads to a NULL pointer dereference and kernel panic.
--

--- Patch [1]: [PATCH v3 1/3] scsi: libsas: Handle errors in sas_ex_add_parent_port() ---
commit b53b16ccede6280045a463721b18531d23ff3fb8
Author: Eshaan Deshmukh <[email protected]>

scsi: libsas: Handle errors in sas_ex_add_parent_port()

This patch changes sas_ex_add_parent_port() to return error codes instead
of using BUG_ON() when sas_port_alloc() or sas_port_add() fail. It also
updates sas_ex_discover_dev() to handle these returned errors.

> diff --git a/drivers/scsi/libsas/sas_expander.c b/drivers/scsi/libsas/sas_expander.c
> index f471ab464a78b..f1a1417b09819 100644
> --- a/drivers/scsi/libsas/sas_expander.c
> +++ b/drivers/scsi/libsas/sas_expander.c
> @@ -33,19 +33,29 @@ static void sas_port_add_ex_phy(struct sas_port *port, struct ex_phy *ex_phy)
>  	ex_phy->phy_state = PHY_DEVICE_DISCOVERED;
>  }
>  
> -static void sas_ex_add_parent_port(struct domain_device *dev, int phy_id)
> +static int sas_ex_add_parent_port(struct domain_device *dev, int phy_id)
>  {
>  	struct expander_device *ex = &dev->ex_dev;
>  	struct ex_phy *ex_phy = &ex->ex_phy[phy_id];
> +	int error;
>  
>  	if (!ex->parent_port) {
>  		ex->parent_port = sas_port_alloc(&dev->rphy->dev, phy_id);
> -		/* FIXME: error handling */
> -		BUG_ON(!ex->parent_port);
> -		BUG_ON(sas_port_add(ex->parent_port));
> +		if (!ex->parent_port)
> +			return -ENOMEM;

[Severity: High]
This is a pre-existing issue, but since this patch fixes the missing error
handling here, does a similar problem exist in sas_ex_discover_expander()
in drivers/scsi/libsas/sas_expander.c?

It looks like sas_port_alloc() is called and its result is passed directly to
sas_port_add() without a NULL check:

    phy->port = sas_port_alloc(&parent->rphy->dev, phy_id);
    /* FIXME: better error handling */
    BUG_ON(sas_port_add(phy->port) != 0);

If sas_port_alloc() fails due to memory exhaustion and returns NULL, would
passing this NULL pointer to sas_port_add() immediately dereference it while
checking list_empty(&port->phy_list), resulting in a kernel panic?

[ ... ]

-- 
Sashiko AI review · https://sashiko.dev/#/patchset/[email protected]?part=1