[PATCH] scsi: scsi_debug: negate wrapped memcmp() result

[PATCH] scsi: scsi_debug: negate wrapped memcmp() result

[raoxu]

From: Xu Rao <raoxu@uniontech.com>

comp_write_worker() returns true when the compared data matches.
memcmp() returns zero for equal data and non-zero for different data, so
its result must be negated before it is stored in a bool.

The first segment already uses !memcmp(), but the wrapped segment uses
memcmp() directly, reversing the match result. Use !memcmp() there as
well.

Fixes: 38d5c8336e60 ("scsi_debug: add Report supported opcodes+tmfs; Compare and write")
Cc: stable@vger.kernel.org
Signed-off-by: Xu Rao <raoxu@uniontech.com>
---
 drivers/scsi/scsi_debug.c | 4 ++--
 1 file changed, 2 insertions(+), 2 deletions(-)

diff --git a/drivers/scsi/scsi_debug.c b/drivers/scsi/scsi_debug.c
index 9d1c9c41d0f9..a5911482fc44 100644
--- a/drivers/scsi/scsi_debug.c
+++ b/drivers/scsi/scsi_debug.c
@@ -4310,8 +4310,8 @@ static bool comp_write_worker(struct sdeb_store_info *sip, u64 lba, u32 num,
 	if (!res)
 		return res;
 	if (rest)
-		res = memcmp(fsp, arr + ((num - rest) * lb_size),
-			     rest * lb_size);
+		res = !memcmp(fsp, arr + ((num - rest) * lb_size),
+			      rest * lb_size);
 	if (!res)
 		return res;
 	if (compare_only)
--
2.50.1

Re: [PATCH] scsi: scsi_debug: negate wrapped memcmp() result

[sashiko-bot]

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

Pre-existing issues:
- [High] VERIFY command with BYTCHK=3 bypasses vnum boundary checks, leading to an out-of-bounds read in comp_write_worker().
--

--- Patch [1]: [PATCH] scsi: scsi_debug: negate wrapped memcmp() result ---
commit d60dbbd2f90d919b4ace8e2a0080c2edd89e096f
Author: Xu Rao <raoxu@uniontech.com>

scsi: scsi_debug: negate wrapped memcmp() result
    
This commit fixes a logic error in comp_write_worker() where the result of
the wrapped segment memcmp() was not being negated. It ensures that equal
data correctly results in a true value, matching the behavior of the first
segment comparison.

> 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
> @@ -4264,8 +4264,8 @@ static bool comp_write_worker(struct sdeb_store_info *sip, u64 lba, u32 num,
>  	if (!res)
>  		return res;
>  	if (rest)
> -		res = memcmp(fsp, arr + ((num - rest) * lb_size),
> -			     rest * lb_size);
> +		res = !memcmp(fsp, arr + ((num - rest) * lb_size),
> +			      rest * lb_size);

[Severity: High]
This isn't a bug introduced by this patch, but can this second memcmp()
perform an out-of-bounds read into unmapped vmalloc memory when triggered
from resp_verify()?

Looking at resp_verify(), when a verify command is issued and is_bytchk3
is true, a_num is hardcoded to 1 before checking the device access
boundaries:

drivers/scsi/scsi_debug.c:resp_verify() {
    ...
    a_num = is_bytchk3 ? 1 : vnum;
    /* Treat following check like one for read (i.e. no write) access */
    ret = check_device_access_params(scp, lba, a_num, false);
    ...
}

Because check_device_access_params() validates a_num rather than vnum, a
large vnum can completely bypass the sdebug_store_sectors limit. If
scsi_debug is loaded with a small dev_size_mb (e.g. 1), a vnum up to 8192
can still pass the kcalloc() allocation.

When comp_write_worker() later calculates the remainder:

    if (block + num > store_blks)
        rest = block + num - store_blks;

If num (which comes from vnum) exceeds store_blks, rest will also exceed
store_blks. This would cause the memcmp() operation here to read
rest * lb_size bytes from fsp, crossing into unmapped vmalloc guard pages
and potentially causing an oops.

>  	if (!res)
>  		return res;
>  	if (compare_only)

-- 
Sashiko AI review · https://sashiko.dev/#/patchset/E5AD42E9C0E18633+20260803095328.3445311-1-raoxu@uniontech.com?part=1

Re: [PATCH] scsi: scsi_debug: negate wrapped memcmp() result

[John Garry]

On 03/08/2026 10:53, raoxu wrote:
> From: Xu Rao <raoxu@uniontech.com>
> 
> comp_write_worker() returns true when the compared data matches.
> memcmp() returns zero for equal data and non-zero for different data, so
> its result must be negated before it is stored in a bool.
> 
> The first segment already uses !memcmp(), but the wrapped segment uses
> memcmp() directly, reversing the match result. Use !memcmp() there as
> well.
> 
> Fixes: 38d5c8336e60 ("scsi_debug: add Report supported opcodes+tmfs; Compare and write")
> Cc: stable@vger.kernel.org
> Signed-off-by: Xu Rao <raoxu@uniontech.com>

Reviewed-by: John Garry <john.g.garry@oracle.com>

> ---
>   drivers/scsi/scsi_debug.c | 4 ++--
>   1 file changed, 2 insertions(+), 2 deletions(-)
> 
> diff --git a/drivers/scsi/scsi_debug.c b/drivers/scsi/scsi_debug.c
> index 9d1c9c41d0f9..a5911482fc44 100644
> --- a/drivers/scsi/scsi_debug.c
> +++ b/drivers/scsi/scsi_debug.c
> @@ -4310,8 +4310,8 @@ static bool comp_write_worker(struct sdeb_store_info *sip, u64 lba, u32 num,
>   	if (!res)
>   		return res;
>   	if (rest)
> -		res = memcmp(fsp, arr + ((num - rest) * lb_size),
> -			     rest * lb_size);
> +		res = !memcmp(fsp, arr + ((num - rest) * lb_size),
> +			      rest * lb_size);
>   	if (!res)
>   		return res;
>   	if (compare_only)
> --
> 2.50.1
> 
>