Re: [PATCH v2 3/4] scsi: scsi_debug: Map the region written by WRITE ATOMIC (16)

[sashiko-bot@kernel.org]

Thank you for your contribution! Sashiko AI review found 1 potential issue(s) to consider:
- [Medium] scsi_debug: data race between WRITE ATOMIC and UNMAP
--

commit 658af810db2f7f36283a9bf13a41e6b597ba68ec
Author: Niklas Cassel <cassel@kernel.org>

scsi: scsi_debug: Map the region written by WRITE ATOMIC (16)

This commit updates resp_atomic_write() to call map_region() when logical
block provisioning is enabled. This ensures GET LBA STATUS correctly
reports the region as mapped after an atomic write, mirroring the behavior
of standard writes.

> diff --git a/drivers/scsi/scsi_debug.c b/drivers/scsi/scsi_debug.c
> index ddcc7a920089e..5a08f02130ef6 100644
> --- a/drivers/scsi/scsi_debug.c
> +++ b/drivers/scsi/scsi_debug.c
> @@ -6213,6 +6213,9 @@ static int resp_atomic_write(struct scsi_cmnd *scp,
>  	}
>  
>  	ret = do_device_access(sip, scp, 0, lba, len, 0, true, true);
> +	if (unlikely(scsi_debug_lbp()))
> +		map_region(sip, lba, len);

[Severity: Medium]
Could this introduce a data race with concurrent unmap operations?

In resp_unmap(), unmap_region() modifies the data buffer via memset() and
clears map bits while protected only by sdeb_meta_write_lock(sip), never
acquiring the data lock macc_data_lck. Normal writes like resp_write_dt0()
explicitly acquire sdeb_meta_write_lock(sip) when scsi_debug_lbp() is true
to safely serialize data access with concurrent unmap operations.

However, resp_atomic_write() does not acquire sdeb_meta_write_lock(sip)
when scsi_debug_lbp() is true. It only holds the data lock via
sdeb_data_lock(sip, atomic) during do_device_access(). Since unmap holds
macc_meta_lck and the atomic write holds macc_data_lck, they appear to be
unsynchronized, leading to potential data corruption in the scsi_debug
simulated RAM disk buffer and provisioning map.

While a later patch in this series adds sdeb_meta_write_lock(sip) to
resp_atomic_write(), it appears to do so only for the
sdebug_dev_is_zoned(devip) condition, leaving the scsi_debug_lbp() race
unmitigated in the final state of the series.

Should sdeb_meta_write_lock(sip) be acquired to protect the provisioning
map modifications?

> +
>  	if (unlikely(ret == -1))
>  		return DID_ERROR << 16;
>  	if (unlikely(ret != len * sdebug_sector_size))

-- 
Sashiko AI review · https://sashiko.dev/#/patchset/20260917110316.1104863-6-cassel@kernel.org?part=3