Re: [PATCH] scsi: mpi3mr: unwind failed remote-PHY allocations

[sashiko-bot@kernel.org]

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

Pre-existing issues:
- [High] Missing error handling for `sas_rphy_add()` failure causes invalid driver state and a kernel crash during device teardown.
--

commit 5ef24e402f3bccdd53263da60efdaa50aeca068d
Author: Slavin Liu <bolin.liu@seu.edu.cn>

scsi: mpi3mr: unwind failed remote-PHY allocations

Remote-PHY allocation can fail after the SAS port and its PHY links
were registered. Remove those links, delete the port and clear the
pending flag before entering the existing private-port cleanup.

> diff --git a/drivers/scsi/mpi3mr/mpi3mr_transport.c b/drivers/scsi/mpi3mr/mpi3mr_transport.c
> index 29013d0fea547..2ce2c6c07f284 100644
> --- a/drivers/scsi/mpi3mr/mpi3mr_transport.c
> +++ b/drivers/scsi/mpi3mr/mpi3mr_transport.c
> @@ -1468,6 +1468,17 @@ static struct mpi3mr_sas_port *mpi3mr_sas_port_add(struct mpi3mr_ioc *mrioc,
>  		rphy = sas_expander_alloc(port,
>  		    mr_sas_port->remote_identify.device_type);
>  	}
> +	if (!rphy) {
> +		list_for_each_entry(mr_sas_phy, &mr_sas_port->phy_list,
> +				    port_siblings) {
> +			sas_port_delete_phy(port, mr_sas_phy->phy);
> +			mr_sas_phy->phy_belongs_to_port = 0;
> +		}
> +		sas_port_delete(port);
> +		if (tgtdev)
> +			tgtdev->dev_spec.sas_sata_inf.pend_sas_rphy_add = 0;
> +		goto out_fail;
> +	}
>  	rphy->identify = mr_sas_port->remote_identify;
>  
>  	if (mrioc->current_event)
>  		mrioc->current_event->pending_at_sml = 1;
>  
>  	if ((sas_rphy_add(rphy))) {
>  		ioc_err(mrioc, "failure at %s:%d/%s()!\n",
>  		    __FILE__, __LINE__, __func__);
>  	}
>  	if (mr_sas_port->remote_identify.device_type == SAS_END_DEVICE) {
>  		tgtdev->dev_spec.sas_sata_inf.pend_sas_rphy_add = 0;
>  		tgtdev->dev_spec.sas_sata_inf.sas_transport_attached = 1;
>  		mpi3mr_tgtdev_put(tgtdev);
>  	}

[Severity: High]
This isn't a bug introduced by this patch, but is it safe to proceed
after sas_rphy_add() fails in mpi3mr_sas_port_add()?

If sas_rphy_add(rphy) fails here, it logs an error but execution continues
normally, marking the transport as attached.

However, when sas_rphy_add() internally attempts device_add(&rphy->dev) and
fails, it returns an error but leaves port->rphy pointing to the failed rphy.

Later during device teardown, sas_port_delete() will do:

	if (port->rphy) {
		sas_rphy_delete(port->rphy);
		...
	}

sas_rphy_delete() then calls device_del(). Calling device_del() on a device
that failed device_add() results in a kernel crash, because device_del()
dereferences dev->p which was cleaned up and set to NULL.

Could this error path be updated to unwind the failed rphy allocation and
goto out_fail similarly to the new !rphy check?

-- 
Sashiko AI review · https://sashiko.dev/#/patchset/20260913125217.110026-1-bolin.liu@seu.edu.cn?part=1