Re: [PATCH v10 05/12] scsi: scsi_debug: Avoid 32-bit overflow in WRITE SCATTERED offsets

[sashiko-bot@kernel.org]

Thank you for your contribution! Sashiko AI review found 1 potential issue(s) to consider:
- [High] scsi: scsi_debug: out-of-bounds read in sg_copy_buffer via off_t underflow
--

commit 84b5a3ba9d6d206750a298cdc52cd60cfbdeb0a6
Author: Niklas Cassel <cassel@kernel.org>

scsi: scsi_debug: Avoid 32-bit overflow in WRITE SCATTERED offsets

This commit changes num_by and sg_off to 64-bit to avoid 32-bit overflow
issues in resp_write_scat() when handling large LBA ranges. When calling
do_device_access(), it caps sg_off at U32_MAX.

> diff --git a/drivers/scsi/scsi_debug.c b/drivers/scsi/scsi_debug.c
> index b64ae3ad300d8..8f9d54269dcec 100644
> --- a/drivers/scsi/scsi_debug.c
> +++ b/drivers/scsi/scsi_debug.c
[ ... ]
> @@ -5311,7 +5312,8 @@ static int resp_write_scat(struct scsi_cmnd *scp,
>  		 * Write ranges atomically to keep as close to pre-atomic
>  		 * writes behaviour as possible.
>  		 */
> -		ret = do_device_access(sip, scp, sg_off, lba, num, group, true, true);
> +		ret = do_device_access(sip, scp, min_t(u64, sg_off, U32_MAX), lba,
> +				       num, group, true, true);

[Severity: High]
Does capping the offset to U32_MAX here lead to an out-of-bounds memory read
on 32-bit systems when processing descriptors beyond 4 GiB?

do_device_access() takes the capped value as a u32 (sg_skip), and then
passes it down to sg_copy_buffer() which expects an off_t:

lib/scatterlist.c:sg_copy_buffer() {
    size_t sg_copy_buffer(struct scatterlist *sgl, unsigned int nents,
                          void *buf, size_t buflen, off_t skip,
                          bool to_buffer)
}

On 32-bit platforms, off_t is a signed 32-bit integer, so the unsigned
U32_MAX (0xFFFFFFFF) is implicitly converted into -1.

When sg_copy_buffer() calls sg_miter_skip(), it uses this negative skip
value:

lib/scatterlist.c:sg_miter_skip() {
    ...
    consumed = min_t(off_t, offset, miter->__remaining);
    ...
}

Because offset is -1 and off_t is signed, min_t() selects -1 as the
consumed amount. This bypasses the skip loop and offsets the iterator
backwards, setting its internal byte offset to 0xFFFFFFFF.

Could this cause subsequent pointer arithmetic to point one byte before the
mapped page, resulting in an out-of-bounds memory copy?

-- 
Sashiko AI review · https://sashiko.dev/#/patchset/20260928072102.725566-14-cassel@kernel.org?part=5