Re: [PATCH 2/7] scsi: scsi_debug: Split resp_write_same()

[sashiko-bot@kernel.org]

Thank you for your contribution! Sashiko AI review found 1 potential issue(s) to consider:
- [High] scsi: scsi_debug: error code clobbered in __resp_write_same()
--

commit 696b9f54842f667ad884ebcdfbd0d4e29f77e149
Author: Bart Van Assche <bvanassche@acm.org>

scsi: scsi_debug: Split resp_write_same()
    
Since the Clang thread-safety analyzer does not support conditional
locking, split the function resp_write_same() such that all locking
statements end up in the same branch of an if-statement.

> diff --git a/drivers/scsi/scsi_debug.c b/drivers/scsi/scsi_debug.c
> index b854102e6caa1..a44886a4c8539 100644
> --- a/drivers/scsi/scsi_debug.c
> +++ b/drivers/scsi/scsi_debug.c
[ ... ]
> @@ -5422,9 +5415,25 @@ static int resp_write_same(struct scsi_cmnd *scp, u64 lba, u32 num,
>  unlock:
>  	sdeb_data_write_unlock(sip);
>  	ret = 0;
> -out:
> -	if (meta_data_locked)
> +	return ret;

[Severity: High]
If fetch_to_dev_buffer() fails and returns -1, the code earlier in
__resp_write_same() sets ret = DID_ERROR << 16 and jumps to the unlock label:

	if (-1 == ret) {
		ret = DID_ERROR << 16;
		goto unlock;
	}

Does this code unconditionally overwrite the captured error code and return 0,
silently masking the write failure?

> +}

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