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

[sashiko-bot@kernel.org]

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