Re: [PATCH v3 1/3] scsi: target: rd: Fix oversized sg table array allocation

[sashiko-bot@kernel.org]

Thank you for your contribution! Sashiko AI review found 1 potential issue(s) to consider:
- [High] target: 32-bit overflow in rd_build_prot_space() truncates protection array causing NULL dereference
--

commit 4c2aa7a4051244bd60d2a5f1cbee54dc66220984
Author: Sanan Hasanov <sanan.hasanov@ucf.edu>

scsi: target: rd: Fix oversized sg table array allocation

This commit fixes an issue where an unbounded rd_pages parameter could trigger
an oversized allocation attempt. It restricts the maximum page count to
totalram_pages() and transitions sg table arrays to use kvzalloc_objs() to
allow large allocations to fall back to vmalloc.

Link: https://syzkaller.appspot.com/bug?extid=fa495e1497c48a6ed885

> diff --git a/drivers/target/target_core_rd.c b/drivers/target/target_core_rd.c
> index 092d9fe0d4e31..97c63b12266fb 100644
> --- a/drivers/target/target_core_rd.c
> +++ b/drivers/target/target_core_rd.c

[ ... ]

> @@ -248,7 +255,7 @@ static int rd_build_prot_space(struct rd_dev *rd_dev, int prot_length, int block
>  	total_sg_needed = (rd_dev->rd_page_count * prot_length / block_size) + 1;

[Severity: High]
Does this calculation silently overflow for large page counts?

Both rd_page_count (u32) and prot_length (int) undergo 32-bit arithmetic. Since
the new upper bound allows rd_page_count to reach totalram_pages(), a system
with over 2TB of RAM can configure a ramdisk with more than 536 million pages.

Multiplying 536,870,912 by 8 (a common prot_length) overflows a 32-bit integer,
truncating total_sg_needed to a small value and resulting in an undersized
protection array.

Later, during I/O processing, if a request spans past the end of this truncated
array, sbc_dif_copy_prot() can exhaust the scatterlist. When sg_next() returns
NULL, it causes a kernel panic by dereferencing sg->length:

drivers/target/target_core_sbc.c:sbc_dif_copy_prot() {
        ...
        while (psg_len) {
                ...
                len = min(psg_len, sg->length - offset);
                ...
        }
        ...
}

Could this calculation be cast to a 64-bit type to prevent the overflow?

>  	sg_tables = (total_sg_needed / max_sg_per_table) + 1;
> -	sg_table = kzalloc_objs(*sg_table, sg_tables);
> +	sg_table = kvzalloc_objs(*sg_table, sg_tables);
>  	if (!sg_table)
>  		return -ENOMEM;
>

-- 
Sashiko AI review · https://sashiko.dev/#/patchset/20261008235411.56641-1-sanan.hasanou@gmail.com?part=1