Re: [PATCH 4/4] fixup! zfcp: trace return values of sysfs unit add store

[sashiko-bot@kernel.org]

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

Pre-existing issues:
- [High] Unbalanced atomic_dec on error path causes state corruption in `zfcp_unit_add()`, permanently bricking the port.
--

--- Patch [4]: [PATCH 4/4] fixup! zfcp: trace return values of sysfs unit add store ---
commit ccd7f94929b859d2a706671b750a2197236c689b
Author: Nihar Panda <niharp@linux.ibm.com>

fixup! zfcp: trace return values of sysfs unit add store

upstream discussions...

> diff --git a/drivers/s390/scsi/zfcp_sysfs.c b/drivers/s390/scsi/zfcp_sysfs.c
> index 729c9664a48f8..597e04cdcc976 100644
> --- a/drivers/s390/scsi/zfcp_sysfs.c
> +++ b/drivers/s390/scsi/zfcp_sysfs.c
> @@ -443,7 +443,7 @@ static ssize_t zfcp_sysfs_unit_add_store(struct device *dev,
>  {
>  	struct zfcp_port *port = container_of(dev, struct zfcp_port, dev);
>  	struct zfcp_adapter *adapter = port->adapter;
> -	u64 fcp_lun;
> +	u64 fcp_lun = 0x0ULL;
>  	int retval = -EINVAL;
>  
>  	if (kstrtoull(buf, 0, (unsigned long long *)&fcp_lun)) {
>  		zfcp_dbf_hba_uas("syuast1", 3, adapter, port->wwpn,
>  				 fcp_lun, retval);
>  		return retval;
>  	}
> 
>  	flush_work(&port->rport_work);
> 
>  	retval = zfcp_unit_add(port, fcp_lun);

[Severity: High]
This is a pre-existing issue, but does zfcp_unit_add() have an unbalanced
atomic_dec() on its error path that can cause state corruption?

If device_register() fails in zfcp_unit_add(), put_device() is called which
invokes zfcp_unit_release():

drivers/s390/scsi/zfcp_unit.c:zfcp_unit_release() {
	...
	atomic_dec(&unit->port->units);
	kfree(unit);
}

However, atomic_inc(&port->units) is only called after a successful
device_register() in zfcp_unit_add():

drivers/s390/scsi/zfcp_unit.c:zfcp_unit_add() {
	...
	if (device_register(&unit->dev)) {
		put_device(&unit->dev);
		retval = -ENOMEM;
		goto out;
	}

	atomic_inc(&port->units);
	...
}

Because the decrement happens even if the increment hasn't, a failure in
device_register() would drop port->units to -1. Since -1 is used as a
sentinel by zfcp_sysfs_port_is_removing() to determine if a port is
permanently removed, would this failure permanently prevent all future
zfcp_unit_add() operations and SCSI scans on this port?

-- 
Sashiko AI review · https://sashiko.dev/#/patchset/20260724053934.2781087-1-niharp@linux.ibm.com?part=4