[PATCH] scsi: scsi_dh_alua: Use common code in alua_rtpg()

[PATCH] scsi: scsi_dh_alua: Use common code in alua_rtpg()

[Markus Elfring]

From: Markus Elfring <elfring@users.sourceforge.net>
Date: Thu, 18 Jun 2026 14:54:13 +0200

Use an additional label so that a bit of common code can be better reused
at the end of this function implementation.

This issue was detected by using the Coccinelle software.

Signed-off-by: Markus Elfring <elfring@users.sourceforge.net>
---
 drivers/scsi/device_handler/scsi_dh_alua.c | 12 ++++++------
 1 file changed, 6 insertions(+), 6 deletions(-)

diff --git a/drivers/scsi/device_handler/scsi_dh_alua.c b/drivers/scsi/device_handler/scsi_dh_alua.c
index 80ab0ff921d4..016150701064 100644
--- a/drivers/scsi/device_handler/scsi_dh_alua.c
+++ b/drivers/scsi/device_handler/scsi_dh_alua.c
@@ -582,8 +582,8 @@ static int alua_rtpg(struct scsi_device *sdev, struct alua_port_group *pg)
 			sdev_printk(KERN_INFO, sdev,
 				    "%s: ignoring rtpg result %d\n",
 				    ALUA_DH_NAME, retval);
-			kfree(buff);
-			return SCSI_DH_OK;
+			err = SCSI_DH_OK;
+			goto free_buff;
 		}
 		if (retval < 0 || !scsi_sense_valid(&sense_hdr)) {
 			sdev_printk(KERN_INFO, sdev,
@@ -632,15 +632,14 @@ static int alua_rtpg(struct scsi_device *sdev, struct alua_port_group *pg)
 			sdev_printk(KERN_ERR, sdev, "%s: rtpg retry\n",
 				    ALUA_DH_NAME);
 			scsi_print_sense_hdr(sdev, ALUA_DH_NAME, &sense_hdr);
-			kfree(buff);
-			return err;
+			goto free_buff;
 		}
 		sdev_printk(KERN_ERR, sdev, "%s: rtpg failed\n",
 			    ALUA_DH_NAME);
 		scsi_print_sense_hdr(sdev, ALUA_DH_NAME, &sense_hdr);
-		kfree(buff);
 		pg->expiry = 0;
-		return SCSI_DH_IO;
+		err = SCSI_DH_IO;
+		goto free_buff;
 	}
 
 	len = get_unaligned_be32(&buff[0]) + 4;
@@ -770,6 +769,7 @@ static int alua_rtpg(struct scsi_device *sdev, struct alua_port_group *pg)
 		break;
 	}
 	spin_unlock_irqrestore(&pg->lock, flags);
+free_buff:
 	kfree(buff);
 	return err;
 }
-- 
2.54.0

Re: [PATCH] scsi: scsi_dh_alua: Use common code in alua_rtpg()

[sashiko-bot]

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

Pre-existing issues:
- [High] A race condition in `alua_rtpg` can cause ALUA port group state updates to be permanently dropped.
--

commit a4fc4f8579d1bb0ef6314519e4902a1bf335fb2b
Author: Markus Elfring <elfring@users.sourceforge.net>
Subject: scsi: scsi_dh_alua: Use common code in alua_rtpg()

This commit introduces a goto label to consolidate the memory freeing
and return logic in alua_rtpg(). It replaces multiple redundant kfree()
calls throughout the error handling paths of the function.

> diff --git a/drivers/scsi/device_handler/scsi_dh_alua.c b/drivers/scsi/device_handler/scsi_dh_alua.c
> index 80ab0ff921d43..0161507010643 100644
> --- a/drivers/scsi/device_handler/scsi_dh_alua.c
> +++ b/drivers/scsi/device_handler/scsi_dh_alua.c

[ ... ]

> @@ -632,15 +632,14 @@ static int alua_rtpg(struct scsi_device *sdev, struct alua_port_group *pg)
>  			sdev_printk(KERN_ERR, sdev, "%s: rtpg retry\n",
>  				    ALUA_DH_NAME);
>  			scsi_print_sense_hdr(sdev, ALUA_DH_NAME, &sense_hdr);
> -			kfree(buff);
> -			return err;
> +			goto free_buff;
>  		}
>  		sdev_printk(KERN_ERR, sdev, "%s: rtpg failed\n",
>  			    ALUA_DH_NAME);
>  		scsi_print_sense_hdr(sdev, ALUA_DH_NAME, &sense_hdr);
> -		kfree(buff);
>  		pg->expiry = 0;
> -		return SCSI_DH_IO;
> +		err = SCSI_DH_IO;
> +		goto free_buff;
>  	}
>  
>  	len = get_unaligned_be32(&buff[0]) + 4;

[Severity: High]
This is a pre-existing issue, but was not introduced by the patch under
review. While we are consolidating the return paths in alua_rtpg(), does
this function still have a race condition when applying the new access
state?

Looking at the loop that processes the descriptors further down in
alua_rtpg():

    if (spin_trylock_irqsave(&tmp_pg->lock, flags)) {
        if ((tmp_pg == pg) ||
            !(tmp_pg->flags & ALUA_PG_RUNNING)) {
            struct alua_dh_data *h;

            tmp_pg->state = desc[0] & 0x0f;

If this trylock fails because the lock is held by a function that doesn't
set the ALUA_PG_RUN_RTPG flag (like alua_bus_detach() or
alua_set_params()), the new ALUA access state is silently discarded without
queuing a retry.

Could this cause the port group's access state to become permanently stale
(e.g., stuck in SCSI_ACCESS_STATE_STANDBY despite the hardware
transitioning to an active state), ultimately failing all I/O to
non-optimal paths and causing data unavailability?

> @@ -770,6 +769,7 @@ static int alua_rtpg(struct scsi_device *sdev, struct alua_port_group *pg)
>  		break;
>  	}
>  	spin_unlock_irqrestore(&pg->lock, flags);
> +free_buff:
>  	kfree(buff);
>  	return err;
>  }

-- 
Sashiko AI review · https://sashiko.dev/#/patchset/366c376e-ef8a-490d-a580-570f2f51ec81@web.de?part=1