[PATCH] fix: scsi: srp_reconnect_rport: unbalanced scsi_block_targets/scsi_target_unblock

[PATCH] fix: scsi: srp_reconnect_rport: unbalanced scsi_block_targets/scsi_target_unblock

[WenTao Liang]

In srp_reconnect_rport(), scsi_block_targets() is called only when
rport->state is not FAIL_FAST and not LOST. However,
scsi_target_unblock() is called unconditionally on the success path and
on some error paths, causing an extra kref_put on sdev_gendev when block
was never called.

Introduce a 'blocked' flag to track whether scsi_block_targets() was
called, and only call scsi_target_unblock() when blocked is true.

Cc: stable@vger.kernel.org
Fixes: 09345f65058b ("[SCSI] add srp transport class")
Signed-off-by: WenTao Liang <vulab@iscas.ac.cn>
---
 drivers/scsi/scsi_transport_srp.c | 8 ++++++--
 1 file changed, 6 insertions(+), 2 deletions(-)

diff --git a/drivers/scsi/scsi_transport_srp.c b/drivers/scsi/scsi_transport_srp.c
index d71ab5fdb758..b9f67e143b52 100644
--- a/drivers/scsi/scsi_transport_srp.c
+++ b/drivers/scsi/scsi_transport_srp.c
@@ -535,6 +535,7 @@ int srp_reconnect_rport(struct srp_rport *rport)
 	struct srp_internal *i = to_srp_internal(shost->transportt);
 	struct scsi_device *sdev;
 	int res;
+	bool blocked = false;
 
 	pr_debug("SCSI host %s\n", dev_name(&shost->shost_gendev));
 
@@ -549,6 +550,7 @@ int srp_reconnect_rport(struct srp_rport *rport)
 		 * treats SDEV_TRANSPORT_OFFLINE like SDEV_BLOCK.
 		 */
 		scsi_block_targets(shost, &shost->shost_gendev);
+		blocked = true;
 	res = rport->state != SRP_RPORT_LOST ? i->f->reconnect(rport) : -ENODEV;
 	pr_debug("%s (state %d): transport.reconnect() returned %d\n",
 		 dev_name(&shost->shost_gendev), rport->state, res);
@@ -558,7 +560,8 @@ int srp_reconnect_rport(struct srp_rport *rport)
 
 		rport->failed_reconnects = 0;
 		srp_rport_set_state(rport, SRP_RPORT_RUNNING);
-		scsi_target_unblock(&shost->shost_gendev, SDEV_RUNNING);
+		if (blocked)
+			scsi_target_unblock(&shost->shost_gendev, SDEV_RUNNING);
 		/*
 		 * If the SCSI error handler has offlined one or more devices,
 		 * invoking scsi_target_unblock() won't change the state of
@@ -579,7 +582,8 @@ int srp_reconnect_rport(struct srp_rport *rport)
 		__rport_fail_io_fast(rport);
 		__srp_start_tl_fail_timers(rport);
 	} else if (rport->state != SRP_RPORT_BLOCKED) {
-		scsi_target_unblock(&shost->shost_gendev,
+		if (blocked)
+			scsi_target_unblock(&shost->shost_gendev,
 				    SDEV_TRANSPORT_OFFLINE);
 	}
 	mutex_unlock(&rport->mutex);
-- 
2.39.5 (Apple Git-154)

Re: [PATCH] fix: scsi: srp_reconnect_rport: unbalanced scsi_block_targets/scsi_target_unblock

[sashiko-bot]

Thank you for your contribution! Sashiko AI review found 1 potential issue(s) to consider:
- [High] Missing curly braces around the `if` block causes `blocked = true;` to execute unconditionally, defeating the intended fix.
--

commit c11303cf8aa2900f23dfb0d36f22857ab2b9cb38
Author: WenTao Liang <vulab@iscas.ac.cn>

fix: scsi: srp_reconnect_rport: unbalanced scsi_block_targets/scsi_target_unblock

This commit introduces a blocked flag to srp_reconnect_rport() to track
if scsi_block_targets() was called. This is intended to prevent an extra
scsi_target_unblock() call which results in an unbalanced kref_put.

> diff --git a/drivers/scsi/scsi_transport_srp.c b/drivers/scsi/scsi_transport_srp.c
> index a61cbb079ab4e..d7c2608b73ef3 100644
> --- a/drivers/scsi/scsi_transport_srp.c
> +++ b/drivers/scsi/scsi_transport_srp.c
[ ... ]
> @@ -549,6 +550,7 @@ int srp_reconnect_rport(struct srp_rport *rport)
>  		 * treats SDEV_TRANSPORT_OFFLINE like SDEV_BLOCK.
>  		 */
>  		scsi_block_targets(shost, &shost->shost_gendev);
> +		blocked = true;

[Severity: High]
Does this missing brace cause the blocked flag to be set unconditionally?

Looking at srp_reconnect_rport(), the preceding if statement lacks curly
braces:

	if (rport->state != SRP_RPORT_FAIL_FAST && rport->state != SRP_RPORT_LOST)
		/* ... */
		scsi_block_targets(shost, &shost->shost_gendev);
		blocked = true;

Because blocked = true is treated as a separate sequential statement, it will
execute regardless of the rport->state check.

Does this entirely defeat the fix by leaving the unbalanced
scsi_target_unblock() reference counting issue active?

-- 
Sashiko AI review · https://sashiko.dev/#/patchset/20260626161402.55116-1-vulab@iscas.ac.cn?part=1

Re: [PATCH] fix: scsi: srp_reconnect_rport: unbalanced scsi_block_targets/scsi_target_unblock

[James Bottomley]

On Sat, 2026-06-27 at 00:14 +0800, WenTao Liang wrote:
> In srp_reconnect_rport(), scsi_block_targets() is called only when
> rport->state is not FAIL_FAST and not LOST. However,
> scsi_target_unblock() is called unconditionally on the success path
> and on some error paths, causing an extra kref_put on sdev_gendev
> when block was never called.

I don't believe this is true: block and unblock calls don't have an
unbalanced kref in them (by design, since you're supposed to be able to
have unblock called on a running device and be a no-op).

Regards,

James