Re: [PATCH 1/7] scsi: scsi_debug: Fix a locking bug in resp_write_same()

[sashiko-bot@kernel.org]

Thank you for your contribution! Sashiko AI review found 1 potential issue(s) to consider:
- [Medium] scsi: scsi_debug: error masked in resp_write_same()
--

commit 9e547db0a77591a35683cc5b28aeb6cb6d633c73
Author: Bart Van Assche <bvanassche@acm.org>

scsi: scsi_debug: Fix a locking bug in resp_write_same()

This commit fixes a missing lock release in resp_write_same() when
fetch_to_dev_buffer() fails. It redirects the error path to a new
unlock label to ensure sdeb_data_write_unlock() is called.

> diff --git a/drivers/scsi/scsi_debug.c b/drivers/scsi/scsi_debug.c
> --- a/drivers/scsi/scsi_debug.c
> +++ b/drivers/scsi/scsi_debug.c
> @@ -5402,7 +5402,7 @@ static int resp_write_same(struct scsi_cmnd *scp, u64 lba, u32 num,
>  
>  	if (-1 == ret) {
>  		ret = DID_ERROR << 16;
> -		goto out;
> +		goto unlock;
>  	} else if (sdebug_verbose && !ndob && (ret < lb_size))

[ ... ]

> @@ -5419,6 +5419,7 @@ static int resp_write_same(struct scsi_cmnd *scp, u64 lba, u32 num,
>  	/* If ZBC zone then bump its write pointer */
>  	if (sdebug_dev_is_zoned(devip))
>  		zbc_inc_wp(devip, lba, num);
> +unlock:
>  	sdeb_data_write_unlock(sip);
>  	ret = 0;

[Severity: Medium]
Does this unconditionally overwrite the error code?

By jumping to the new unlock label when fetch_to_dev_buffer() fails, ret
is initially set to DID_ERROR << 16, but then it appears ret = 0 is
executed unconditionally right below the unlock label.

Could this silently mask I/O failures by returning 0 on the error path?

>  out:
>  	if (meta_data_locked)
>  		sdeb_meta_write_unlock(sip);
>  	return ret;
>  }

-- 
Sashiko AI review · https://sashiko.dev/#/patchset/cover.1790119506.git.bvanassche@acm.org?part=1