[PATCH] scsi_debug: add support to corrupt data and/or reftag

[PATCH] scsi_debug: add support to corrupt data and/or reftag

[Christoph Hellwig]

Add a new debugfs file to inject corruptions of the data and/or reftag.
This will be used to detect that protection information and/or file
system checksumming can detect random bit errors or misplaced writes.

To use this echo the start LBA, number of logical blocks and type of
corruption into the new "corrupt" debugfs file for each scsi_debug
device.

For example:

	echo lba=42,num=1,bit_errors=2 > /sys/kernel/debug/scsi_debug/1:0:0:0/corrupt
or
	echo lba=2,num=4,reftag_adjust=8 > /sys/kernel/debug/scsi_debug/1:0:0:0/corrupt

The injection will be used by new xfstests test cases.

Signed-off-by: Christoph Hellwig <hch@lst.de>
---
 drivers/scsi/scsi_debug.c | 153 ++++++++++++++++++++++++++++++++++++++
 1 file changed, 153 insertions(+)

diff --git a/drivers/scsi/scsi_debug.c b/drivers/scsi/scsi_debug.c
index 9d1c9c41d0f9..aa51eccb7061 100644
--- a/drivers/scsi/scsi_debug.c
+++ b/drivers/scsi/scsi_debug.c
@@ -44,6 +44,7 @@
 #include <linux/debugfs.h>
 #include <linux/async.h>
 #include <linux/cleanup.h>
+#include <linux/parser.h>
 
 #include <net/checksum.h>
 
@@ -1214,6 +1215,94 @@ static const struct file_operations sdebug_error_fops = {
 	.release = single_release,
 };
 
+enum corrupt_options {
+	Opt_lba			= (1u << 0),
+	Opt_num			= (1u << 1),
+	Opt_bit_errors		= (1u << 2),
+	Opt_reftag_adjust	= (1u << 3),
+
+	Opt_invalid,
+};
+
+static const match_table_t corrupt_tokens = {
+	{ Opt_lba,			"lba=%u"		},
+	{ Opt_num,			"num=%u"		},
+	{ Opt_bit_errors,		"bit_errors=%u"		},
+	{ Opt_reftag_adjust,		"reftag_adjust=%d"	},
+	{ Opt_invalid,			NULL,			},
+};
+
+static int corrupt_lbas(struct sdebug_dev_info *devip, u64 lba, u32 num,
+		u32 nr_bit_errors, s32 reftag_adjust);
+static ssize_t sdebug_corrupt_write(struct file *file, const char __user *ubuf,
+		size_t count, loff_t *ppos)
+{
+	struct scsi_device *sdev = file->f_inode->i_private;
+	struct sdebug_dev_info *devip = sdev->hostdata;
+	substring_t args[MAX_OPT_ARGS];
+	char *buf, *options, *p;
+	int error = 0;
+	u64 lba = 0;
+	u32 num = 1;
+	u32 nr_bit_errors = 0;
+	s32 reftag_adjust = 0;
+
+	buf = memdup_user_nul(ubuf, count);
+	if (IS_ERR(buf))
+		return PTR_ERR(buf);
+
+	options = buf;
+	while ((p = strsep(&options, ",\n")) != NULL) {
+		if (!*p)
+			continue;
+		switch (match_token(p, corrupt_tokens, args)) {
+		case Opt_lba:
+			error = match_u64(args, &lba);
+			break;
+		case Opt_num:
+			error = match_uint(args, &num);
+			break;
+		case Opt_bit_errors:
+			error = match_uint(args, &nr_bit_errors);
+			break;
+		case Opt_reftag_adjust:
+			error = match_uint(args, &reftag_adjust);
+			break;
+		default:
+			pr_warn("unknown parameter or missing value '%s'\n", p);
+			error = -EINVAL;
+		}
+		if (error)
+			return error;
+	}
+
+	if (num == 0) {
+		pr_warn("invalid number of logical blocks: %u\n", num);
+		error = -EINVAL;
+		goto out_free_buf;
+	}
+
+	if (reftag_adjust &&
+	    (sdebug_dif != T10_PI_TYPE1_PROTECTION &&
+	     sdebug_dif != T10_PI_TYPE2_PROTECTION)) {
+		pr_warn("reftag adjust requires type 1 or type 2 PI\n");
+		error = -EINVAL;
+		goto out_free_buf;
+	}
+
+	error = corrupt_lbas(devip, lba, num, nr_bit_errors, reftag_adjust);
+
+out_free_buf:
+	kfree(buf);
+	if (error)
+		return error;
+	return count;
+}
+
+static const struct file_operations sdebug_corrupt_fops = {
+	.write		= sdebug_corrupt_write,
+};
+
 static int sdebug_target_reset_fail_show(struct seq_file *m, void *p)
 {
 	struct scsi_target *starget = (struct scsi_target *)m->private;
@@ -4908,6 +4997,68 @@ static int resp_write_tape(struct scsi_cmnd *scp, struct sdebug_dev_info *devip)
 	return 0;
 }
 
+static int corrupt_lbas(struct sdebug_dev_info *devip, u64 lba, u32 num,
+		u32 nr_bit_errors, s32 reftag_adjust)
+{
+	struct sdeb_store_info *sip = devip2sip(devip, true);
+	bool meta_data_locked = false;
+	u8 *fsp = sip->storep;
+	u32 block, b, i;
+	int error = 0;
+
+	if (sdebug_dev_is_zoned(devip) ||
+	    sdebug_dix ||
+	    scsi_debug_lbp())  {
+		sdeb_meta_write_lock(sip);
+		meta_data_locked = true;
+	}
+
+	if (num >= sdebug_capacity || lba > sdebug_capacity - num) {
+		pr_err("logical blocks out of bounds: %llu:%u", lba, num);
+		error = -EINVAL;
+		goto out_unlock;
+	}
+
+	if (scsi_debug_lbp() && !map_state(sip, lba, &num)) {
+		pr_err("can't modify unmapped logical blocks: %llu:%u",
+			lba, num);
+		error = -EINVAL;
+		goto out_unlock;
+	}
+
+	for (b = 0; b < nr_bit_errors; b++) {
+		unsigned int bit;
+
+		bit = get_random_u32_below(sdebug_sector_size * BITS_PER_BYTE);
+
+		div_u64_rem(lba, sdebug_store_sectors, &block);
+		for (i = 0; i < num; i++) {
+			u8 *p = fsp + (block * sdebug_sector_size);
+
+			p[bit / BITS_PER_BYTE] ^= (1 << (bit % BITS_PER_BYTE));
+			pr_info("flipping bit %u in LBA %llu\n", bit, lba + i);
+
+			if (++block >= sdebug_store_sectors)
+				block = 0;
+		}
+	}
+
+	if (reftag_adjust) {
+		for (i = 0; i < num; i++) {
+			struct t10_pi_tuple *sdt = dif_store(sip, lba + i);
+
+			be32_add_cpu(&sdt->ref_tag, reftag_adjust);
+			pr_info("adjusting reftag by %d sectors for LBA %llu\n",
+				reftag_adjust, lba + i);
+		}
+	}
+
+out_unlock:
+	if (meta_data_locked)
+		sdeb_meta_write_unlock(sip);
+	return error;
+}
+
 static int resp_write_dt0(struct scsi_cmnd *scp, struct sdebug_dev_info *devip)
 {
 	bool check_prot;
@@ -6670,6 +6821,8 @@ static int scsi_debug_sdev_configure(struct scsi_device *sdp,
 	if (IS_ERR_OR_NULL(dentry))
 		pr_info("failed to create error file for device %s\n",
 			dev_name(&sdp->sdev_gendev));
+	debugfs_create_file("corrupt", 0600, devip->debugfs_entry, sdp,
+				&sdebug_corrupt_fops);
 
 	return 0;
 }
-- 
2.53.0

Re: [PATCH] scsi_debug: add support to corrupt data and/or reftag

[sashiko-bot]

Thank you for your contribution! Sashiko AI review found 9 potential issue(s) to consider:
- [High] `map_state()` unintentionally overwrites the user-requested `num` parameter, severely altering the number of corrupted blocks.
- [High] Modifying sector data and PI without acquiring the required data locks leads to data races.
- [High] Writing to the `corrupt` debugfs file triggers a `BUG_ON` kernel panic if the module is loaded with `fake_rw=1`.
- [High] NULL pointer dereference when modifying `ref_tag` if `sdebug_dix` is not enabled.
- [Medium] User buffer is leaked when parsing an invalid option in `sdebug_corrupt_write`.
- [Medium] `match_uint()` is incorrectly used for the signed `reftag_adjust` parameter, preventing negative adjustments.
- [Medium] Bit errors are incorrectly correlated across all sectors in a multi-block request.
- [Medium] Off-by-one check prevents corrupting the full capacity of the device.
- [Medium] Kernel log flooding and CPU spin via unbounded `pr_info()` calls.
--

--- Patch [1]: [PATCH] scsi_debug: add support to corrupt data and/or reftag ---
commit 0839e34574724c1b365a8698f556ce1b2b9bb134
Author: Christoph Hellwig <hch@lst.de>

scsi_debug: add support to corrupt data and/or reftag
    
Add a new debugfs file to inject corruptions of the data and/or reftag.
This will be used to detect that protection information and/or file
system checksumming can detect random bit errors or misplaced writes.

> diff --git a/drivers/scsi/scsi_debug.c b/drivers/scsi/scsi_debug.c
> index 4a95e6bae38b0..7d5b55b788f48 100644
> --- a/drivers/scsi/scsi_debug.c
> +++ b/drivers/scsi/scsi_debug.c

[ ... ]

> @@ -1168,6 +1169,94 @@ static const struct file_operations sdebug_error_fops = {
> +static ssize_t sdebug_corrupt_write(struct file *file, const char __user *ubuf,
> +		size_t count, loff_t *ppos)
> +{

[ ... ]

> +	while ((p = strsep(&options, ",\n")) != NULL) {
> +		if (!*p)
> +			continue;
> +		switch (match_token(p, corrupt_tokens, args)) {

[ ... ]

> +		case Opt_reftag_adjust:
> +			error = match_uint(args, &reftag_adjust);

[Severity: Medium]
Is match_uint() the correct function to parse this parameter?

Since reftag_adjust is defined as an s32, match_uint() will reject negative
adjustments, preventing users from simulating reference tag skew using
negative values.

> +			break;
> +		default:
> +			pr_warn("unknown parameter or missing value '%s'\n", p);
> +			error = -EINVAL;
> +		}
> +		if (error)
> +			return error;

[Severity: Medium]
Does this leak the buffer allocated by memdup_user_nul() earlier in
sdebug_corrupt_write()?

It looks like returning directly here bypasses the kfree(buf) cleanup path at
the end of the function when an invalid option is provided.

> +	}

[ ... ]

> @@ -4862,6 +4951,68 @@ static int resp_write_tape(struct scsi_cmnd *scp, struct sdebug_dev_info *devip)
>  	return 0;
>  }
>  
> +static int corrupt_lbas(struct sdebug_dev_info *devip, u64 lba, u32 num,
> +		u32 nr_bit_errors, s32 reftag_adjust)
> +{
> +	struct sdeb_store_info *sip = devip2sip(devip, true);

[Severity: High]
Will this trigger a BUG_ON() panic if the module is loaded with fake_rw=1?

Calling devip2sip() with the bug_if_fake_rw parameter set to true explicitly
triggers an assertion when sdebug_fake_rw is enabled. This seems to allow a
user writing to the debugfs file to crash the system.

> +	bool meta_data_locked = false;
> +	u8 *fsp = sip->storep;
> +	u32 block, b, i;
> +	int error = 0;
> +
> +	if (sdebug_dev_is_zoned(devip) ||
> +	    sdebug_dix ||
> +	    scsi_debug_lbp())  {
> +		sdeb_meta_write_lock(sip);
> +		meta_data_locked = true;
> +	}
> +
> +	if (num >= sdebug_capacity || lba > sdebug_capacity - num) {

[Severity: Medium]
Is this bounds check overly strict?

Checking if num >= sdebug_capacity prevents operations that exactly span
the entire capacity of the device when starting at LBA 0. Should this
check be num > sdebug_capacity instead?

> +		pr_err("logical blocks out of bounds: %llu:%u", lba, num);
> +		error = -EINVAL;
> +		goto out_unlock;
> +	}
> +
> +	if (scsi_debug_lbp() && !map_state(sip, lba, &num)) {

[Severity: High]
Does map_state() unintentionally overwrite the requested block count?

map_state() writes the size of the mapped extent into its third parameter.
Since the user's requested block count is passed by reference here, this
discards the user's value and could cause the operation to corrupt an
unintended number of blocks, potentially up to the entire mapped extent.

> +		pr_err("can't modify unmapped logical blocks: %llu:%u",
> +			lba, num);
> +		error = -EINVAL;
> +		goto out_unlock;
> +	}
> +
> +	for (b = 0; b < nr_bit_errors; b++) {
> +		unsigned int bit;
> +
> +		bit = get_random_u32_below(sdebug_sector_size * BITS_PER_BYTE);
> +
> +		div_u64_rem(lba, sdebug_store_sectors, &block);
> +		for (i = 0; i < num; i++) {
> +			u8 *p = fsp + (block * sdebug_sector_size);
> +
> +			p[bit / BITS_PER_BYTE] ^= (1 << (bit % BITS_PER_BYTE));

[Severity: High]
Could this lead to data races when modifying sector data concurrently with
standard block layer I/O?

It appears corrupt_lbas() reads and modifies the data store without acquiring
the necessary sector data locks, such as sdeb_data_sector_lock().

[Severity: Medium]
Also, does the outer loop calculate a single bit offset that is then
applied identically across all sectors in a multi-block request?

It looks like the random bit offset is chosen once in the outer loop,
causing the exact same bit to be flipped in every sector handled by the
inner loop, rather than randomizing the error per sector.

> +			pr_info("flipping bit %u in LBA %llu\n", bit, lba + i);

[Severity: Medium]
Could these unconditional pr_info() calls cause severe performance
degradation or log flooding?

They are placed directly inside nested loops bounded by user inputs, with
no rate limiting.

> +
> +			if (++block >= sdebug_store_sectors)
> +				block = 0;
> +		}
> +	}
> +
> +	if (reftag_adjust) {
> +		for (i = 0; i < num; i++) {
> +			struct t10_pi_tuple *sdt = dif_store(sip, lba + i);
> +
> +			be32_add_cpu(&sdt->ref_tag, reftag_adjust);

[Severity: High]
Could this lead to a NULL pointer dereference if the module is loaded
with dif=1 but dix=0?

The prerequisite check in sdebug_corrupt_write() validates that sdebug_dif
is Type 1 or Type 2, but it fails to check if sdebug_dix is enabled. The
backing array for protection information (sip->dif_storep) is only allocated
if sdebug_dix is non-zero, so dif_store() might return an unallocated address
that gets dereferenced here.

> +			pr_info("adjusting reftag by %d sectors for LBA %llu\n",
> +				reftag_adjust, lba + i);
> +		}
> +	}

-- 
Sashiko AI review · https://sashiko.dev/#/patchset/20260723142154.107268-1-hch@lst.de?part=1

scsi_debug: add support to corrupt data and/or reftag v2

[Christoph Hellwig]

This will be used for testing PI support in xfstests.

Changes since v1:
 - use match_int to parse the  reftag_adjust option
 - don't bypass error cleanup on parsing failure
 - allow to corrupt the entire capacity
 - set the bug_if_fake_rw to devip2sip to false
 - fix the check for unmapped LBAs
 - require DIX for reftag corruption injection
 - make the kernel messages a bit less verbose
 - add calls to sdeb_data_lock and sdeb_data_sector_lock

[PATCH] scsi_debug: add support to corrupt data and/or reftag

[Christoph Hellwig]

Add a new debugfs file to inject corruptions of the data and/or reftag.
This will be used to detect that protection information and/or file
system checksumming can detect random bit errors or misplaced writes.

To use this echo the start LBA, number of logical blocks and type of
corruption into the new "corrupt" debugfs file for each scsi_debug
device.

For example:

	echo lba=42,num=1,bit_errors=2 > /sys/kernel/debug/scsi_debug/1:0:0:0/corrupt
or
	echo lba=2,num=4,reftag_adjust=8 > /sys/kernel/debug/scsi_debug/1:0:0:0/corrupt

The injection will be used by new xfstests test cases.

Signed-off-by: Christoph Hellwig <hch@lst.de>
---
 drivers/scsi/scsi_debug.c | 163 ++++++++++++++++++++++++++++++++++++++
 1 file changed, 163 insertions(+)

diff --git a/drivers/scsi/scsi_debug.c b/drivers/scsi/scsi_debug.c
index 4fb2dc403962..b63190298e0c 100644
--- a/drivers/scsi/scsi_debug.c
+++ b/drivers/scsi/scsi_debug.c
@@ -44,6 +44,7 @@
 #include <linux/debugfs.h>
 #include <linux/async.h>
 #include <linux/cleanup.h>
+#include <linux/parser.h>
 
 #include <net/checksum.h>
 
@@ -1214,6 +1215,95 @@ static const struct file_operations sdebug_error_fops = {
 	.release = single_release,
 };
 
+enum corrupt_options {
+	Opt_lba			= (1u << 0),
+	Opt_num			= (1u << 1),
+	Opt_bit_errors		= (1u << 2),
+	Opt_reftag_adjust	= (1u << 3),
+
+	Opt_invalid,
+};
+
+static const match_table_t corrupt_tokens = {
+	{ Opt_lba,			"lba=%u"		},
+	{ Opt_num,			"num=%u"		},
+	{ Opt_bit_errors,		"bit_errors=%u"		},
+	{ Opt_reftag_adjust,		"reftag_adjust=%d"	},
+	{ Opt_invalid,			NULL,			},
+};
+
+static int corrupt_lbas(struct sdebug_dev_info *devip, u64 lba, u32 num,
+		u32 nr_bit_errors, s32 reftag_adjust);
+static ssize_t sdebug_corrupt_write(struct file *file, const char __user *ubuf,
+		size_t count, loff_t *ppos)
+{
+	struct scsi_device *sdev = file->f_inode->i_private;
+	struct sdebug_dev_info *devip = sdev->hostdata;
+	substring_t args[MAX_OPT_ARGS];
+	char *buf, *options, *p;
+	int error = 0;
+	u64 lba = 0;
+	u32 num = 1;
+	u32 nr_bit_errors = 0;
+	s32 reftag_adjust = 0;
+
+	buf = memdup_user_nul(ubuf, count);
+	if (IS_ERR(buf))
+		return PTR_ERR(buf);
+
+	options = buf;
+	while ((p = strsep(&options, ",\n")) != NULL) {
+		if (!*p)
+			continue;
+		switch (match_token(p, corrupt_tokens, args)) {
+		case Opt_lba:
+			error = match_u64(args, &lba);
+			break;
+		case Opt_num:
+			error = match_uint(args, &num);
+			break;
+		case Opt_bit_errors:
+			error = match_uint(args, &nr_bit_errors);
+			break;
+		case Opt_reftag_adjust:
+			error = match_int(args, &reftag_adjust);
+			break;
+		default:
+			pr_warn("unknown parameter or missing value '%s'\n", p);
+			error = -EINVAL;
+		}
+		if (error)
+			goto out_free_buf;
+	}
+
+	if (num == 0) {
+		pr_warn("invalid number of logical blocks: %u\n", num);
+		error = -EINVAL;
+		goto out_free_buf;
+	}
+
+	if (reftag_adjust &&
+	    (!sdebug_dix ||
+	     (sdebug_dif != T10_PI_TYPE1_PROTECTION &&
+	      sdebug_dif != T10_PI_TYPE2_PROTECTION))) {
+		pr_warn("reftag adjust requires type 1 or type 2 PI with DIX\n");
+		error = -EINVAL;
+		goto out_free_buf;
+	}
+
+	error = corrupt_lbas(devip, lba, num, nr_bit_errors, reftag_adjust);
+
+out_free_buf:
+	kfree(buf);
+	if (error)
+		return error;
+	return count;
+}
+
+static const struct file_operations sdebug_corrupt_fops = {
+	.write		= sdebug_corrupt_write,
+};
+
 static int sdebug_target_reset_fail_show(struct seq_file *m, void *p)
 {
 	struct scsi_target *starget = (struct scsi_target *)m->private;
@@ -4908,6 +4998,77 @@ static int resp_write_tape(struct scsi_cmnd *scp, struct sdebug_dev_info *devip)
 	return 0;
 }
 
+static int corrupt_lbas(struct sdebug_dev_info *devip, u64 lba, u32 num,
+		u32 nr_bit_errors, s32 reftag_adjust)
+{
+	struct sdeb_store_info *sip = devip2sip(devip, false);
+	bool meta_data_locked = false;
+	u8 *fsp = sip->storep;
+	u32 block, num_mapped, b, i;
+	int error = 0;
+
+	if (sdebug_dev_is_zoned(devip) ||
+	    sdebug_dix ||
+	    scsi_debug_lbp())  {
+		sdeb_meta_write_lock(sip);
+		meta_data_locked = true;
+	}
+
+	if (num > sdebug_capacity || lba > sdebug_capacity - num) {
+		pr_err("logical blocks out of bounds: %llu:%u", lba, num);
+		error = -EINVAL;
+		goto out_unlock;
+	}
+
+	if (scsi_debug_lbp() &&
+	    (!map_state(sip, lba, &num_mapped) || num > num_mapped)) {
+		pr_err("can't modify unmapped logical blocks: %llu:%u",
+			lba, num);
+		error = -EINVAL;
+		goto out_unlock;
+	}
+
+	/*
+	 * For each possible bit error, flip the same bit in each corrupted
+	 * block to simulate repeated patterns like errors in cables.
+	 */
+	sdeb_data_lock(sip, false);
+	for (b = 0; b < nr_bit_errors; b++) {
+		unsigned int bit;
+
+		bit = get_random_u32_below(sdebug_sector_size * BITS_PER_BYTE);
+		pr_info("flipping bit %u in LBAs %llu:%llu\n",
+				bit, lba, lba + num - 1);
+
+		div_u64_rem(lba, sdebug_store_sectors, &block);
+		for (i = 0; i < num; i++) {
+			u8 *p = fsp + (block * sdebug_sector_size);
+
+			sdeb_data_sector_lock(sip, true);
+			p[bit / BITS_PER_BYTE] ^= (1 << (bit % BITS_PER_BYTE));
+			sdeb_data_sector_unlock(sip, true);
+			if (++block >= sdebug_store_sectors)
+				block = 0;
+		}
+	}
+
+	if (reftag_adjust) {
+		pr_info("adjusting reftag by %d sectors for LBAs %llu:%llu\n",
+			reftag_adjust, lba, lba + num - 1);
+		for (i = 0; i < num; i++) {
+			struct t10_pi_tuple *sdt = dif_store(sip, lba + i);
+
+			be32_add_cpu(&sdt->ref_tag, reftag_adjust);
+		}
+	}
+	sdeb_data_unlock(sip, false);
+
+out_unlock:
+	if (meta_data_locked)
+		sdeb_meta_write_unlock(sip);
+	return error;
+}
+
 static int resp_write_dt0(struct scsi_cmnd *scp, struct sdebug_dev_info *devip)
 {
 	bool check_prot;
@@ -6674,6 +6835,8 @@ static int scsi_debug_sdev_configure(struct scsi_device *sdp,
 	if (IS_ERR_OR_NULL(dentry))
 		pr_info("failed to create error file for device %s\n",
 			dev_name(&sdp->sdev_gendev));
+	debugfs_create_file("corrupt", 0600, devip->debugfs_entry, sdp,
+				&sdebug_corrupt_fops);
 
 	return 0;
 }
-- 
2.53.0

Re: [PATCH] scsi_debug: add support to corrupt data and/or reftag

[sashiko-bot]

Thank you for your contribution! Sashiko AI review found 5 potential issue(s) to consider:

New issues:
- [High] Unbounded nested loops in `corrupt_lbas()` executed with preemption disabled allow users to cause CPU lockups.
- [High] Unconditional dereference of `sip` in `corrupt_lbas()` when initialized with fake I/O (`fake_rw=1`).
- [Medium] Flawed map state validation in `corrupt_lbas()` rejects valid wrapping corruption requests.

Pre-existing issues:
- [High] Pre-existing out-of-bounds array access in `map_state()` when using large virtual LBAs.
- [High] Toggling `fake_rw` from 1 to 0 at runtime causes a pre-existing NULL pointer dereference in LBP operations.
--

--- Patch [1]: [PATCH] scsi_debug: add support to corrupt data and/or reftag ---
commit b674d4afbf6311bc89a7d69aaa16a3947e8d3546
Author: Christoph Hellwig <hch@lst.de>
scsi_debug: add support to corrupt data and/or reftag

Add a new debugfs file to inject corruptions of the data and/or reftag.
This will be used to detect that protection information and/or file
system checksumming can detect random bit errors or misplaced writes.

> diff --git a/drivers/scsi/scsi_debug.c b/drivers/scsi/scsi_debug.c
> index 3346562275f80..8750e07759306 100644
> --- a/drivers/scsi/scsi_debug.c
> +++ b/drivers/scsi/scsi_debug.c

[ ... ]

> @@ -4862,6 +4952,77 @@ static int resp_write_tape(struct scsi_cmnd *scp, struct sdebug_dev_info *devip)
>  	return 0;
>  }
>  
> +static int corrupt_lbas(struct sdebug_dev_info *devip, u64 lba, u32 num,
> +		u32 nr_bit_errors, s32 reftag_adjust)
> +{
> +	struct sdeb_store_info *sip = devip2sip(devip, false);
> +	bool meta_data_locked = false;
> +	u8 *fsp = sip->storep;

[Severity: High]
Does this unconditionally dereference sip if devip2sip() returns NULL?

If the scsi_debug module is loaded or toggled with fake_rw=1, devip2sip()
explicitly returns NULL. The sip->storep assignment then dereferences it
before any validation occurs, which will cause a kernel panic.

> +	u32 block, num_mapped, b, i;
> +	int error = 0;
> +
> +	if (sdebug_dev_is_zoned(devip) ||
> +	    sdebug_dix ||
> +	    scsi_debug_lbp())  {
> +		sdeb_meta_write_lock(sip);
> +		meta_data_locked = true;
> +	}
> +
> +	if (num > sdebug_capacity || lba > sdebug_capacity - num) {
> +		pr_err("logical blocks out of bounds: %llu:%u", lba, num);
> +		error = -EINVAL;
> +		goto out_unlock;
> +	}
> +
> +	if (scsi_debug_lbp() &&
> +	    (!map_state(sip, lba, &num_mapped) || num > num_mapped)) {

[Severity: High]
This is a pre-existing issue, but can map_state() perform an out-of-bounds
array access when called with large virtual LBAs?

If sdebug_virtual_gb is configured, sdebug_capacity exceeds the physical
sdebug_store_sectors. A command passing a large LBA generates an index
larger than map_size:

drivers/scsi/scsi_debug.c:map_state() {
    ...
    index = lba_to_map_index(lba);
    mapped = test_bit(index, sip->map_storep);
    ...
}

This causes test_bit and subsequent bit operations to access memory outside
the allocated sip->map_storep array.

[Severity: Medium]
Does this check incorrectly reject valid corruption requests that cross the
physical storage boundary?

If a user requests corruption across the physical store boundary (for example,
an lba near sdebug_store_sectors with a num crossing it), map_state() limits
the returned contiguous blocks to the physical boundary:

drivers/scsi/scsi_debug.c:map_state() {
    ...
    end = min_t(sector_t, sdebug_store_sectors, map_index_to_lba(next));
    *num = end - lba;
    ...
}

This causes the num > num_mapped condition to trigger and abort the request,
even though the underlying data loops correctly use div_u64_rem to wrap
around the physical bounds.

[Severity: High]
This is a pre-existing issue, but does toggling fake_rw from 1 to 0 at
runtime cause a NULL pointer dereference in LBP operations?

If fake_rw is toggled from 1 to 0 via sysfs, fake_rw_store() calls
sdebug_add_store() before updating sdebug_fake_rw to 0:

drivers/scsi/scsi_debug.c:fake_rw_store() {
    ...
    idx = sdebug_add_store();
    ...
    sdebug_fake_rw = n;
    ...
}

Inside sdebug_add_store(), scsi_debug_lbp() returns false because
sdebug_fake_rw is still 1, so sip->map_storep remains unallocated. Once
the transition completes, fake_rw is 0 and LBP becomes active. Any
subsequent command using LBP calls map_state(), which crashes when
dereferencing the NULL sip->map_storep.

> +		pr_err("can't modify unmapped logical blocks: %llu:%u",
> +			lba, num);
> +		error = -EINVAL;
> +		goto out_unlock;
> +	}
> +
> +	/*
> +	 * For each possible bit error, flip the same bit in each corrupted
> +	 * block to simulate repeated patterns like errors in cables.
> +	 */
> +	sdeb_data_lock(sip, false);
> +	for (b = 0; b < nr_bit_errors; b++) {
> +		unsigned int bit;
> +
> +		bit = get_random_u32_below(sdebug_sector_size * BITS_PER_BYTE);
> +		pr_info("flipping bit %u in LBAs %llu:%llu\n",
> +				bit, lba, lba + num - 1);
> +
> +		div_u64_rem(lba, sdebug_store_sectors, &block);
> +		for (i = 0; i < num; i++) {

[Severity: High]
Can these unbounded nested loops cause a CPU lockup since they run with
preemption disabled?

A user can write large values for num and nr_bit_errors into the corrupt
debugfs file. The sdeb_data_lock() function calls read_lock(), which
disables preemption. The function then enters these loops, potentially
stalling the CPU for an extended period of time if the inputs are large.

> +			u8 *p = fsp + (block * sdebug_sector_size);
> +
> +			sdeb_data_sector_lock(sip, true);
> +			p[bit / BITS_PER_BYTE] ^= (1 << (bit % BITS_PER_BYTE));
> +			sdeb_data_sector_unlock(sip, true);
> +			if (++block >= sdebug_store_sectors)
> +				block = 0;
> +		}
> +	}

[ ... ]

-- 
Sashiko AI review · https://sashiko.dev/#/patchset/20260819065850.1358861-2-hch@lst.de?part=1