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

[John Garry <john.garry@linux.dev>]

On 9/23/26 08:37, John Garry wrote:
> On 9/23/26 00:26, Bart Van Assche wrote:
>> If fetch_to_dev_buffer() fails in resp_write_same(), the function jumps
>> to 'out' without releasing the data write lock acquired earlier via
>> sdeb_data_write_lock(). Jump to 'unlock' instead so that
>> sdeb_data_write_unlock() is called on the error path. This bug has been
>> discovered by building the scsi_debug driver with Clang and modified
>> lock context annotations. The modified lock context annotations are
>> available in patch "scsi: scsi_debug: Improve lock context annotations".
>>
>> Fixes: 84f3a3c01d70 ("scsi: scsi_debug: Atomic write support")
>> Signed-off-by: Bart Van Assche <bvanassche@acm.org>
>> ---
>>   drivers/scsi/scsi_debug.c | 3 ++-
>>   1 file changed, 2 insertions(+), 1 deletion(-)
>>
>> diff --git a/drivers/scsi/scsi_debug.c b/drivers/scsi/scsi_debug.c
>> index 6941809dfdb7..b854102e6caa 100644
>> --- 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;
> 
> at @unlock we lose the error code in ret - is that intentional?
> 

Now I notice that sashiko spotted this too. I will await until issues 
spotted by sashiko elsewhere at attended to before checking further.

>>       } else if (sdebug_verbose && !ndob && (ret < lb_size))
>>           sdev_printk(KERN_INFO, scp->device,
>>                   "%s: %s: lb size=%u, IO sent=%d bytes\n",
>> @@ -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;
>>   out:
>