Re: [PATCH v2 1/2] 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: bypass of totalram_pages limit via configfs causes unbounded allocation and leak
--

commit 080421786d44f31a44c204db4deacf32fd54462e
Author: Sanan Hasanov <sanan.hasanov@ucf.edu>
scsi: target: rd: Fix oversized sg table array allocation

This commit adds a limit check to ensure the ramdisk page count does not
exceed totalram_pages() during device configuration, avoiding large
out-of-bounds allocations. It also changes kzalloc_objs to kvzalloc_objs to
fallback to vmalloc for large valid page counts.

Closes: 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

[ ... ]

> @@ -188,10 +189,16 @@ static int rd_build_device_space(struct rd_dev *rd_dev)
>  	if (rd_dev->rd_flags & RDF_NULLIO)
>  		return 0;
>  
> +	if (rd_dev->rd_page_count > totalram_pages()) {
> +		pr_err("Page count: %u exceeds total RAM pages: %lu for Ramdisk device\n",
> +		       rd_dev->rd_page_count, totalram_pages());
> +		return -EINVAL;
> +	}
> +
>  	total_sg_needed = rd_dev->rd_page_count;
>  
>  	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;
>  

[ ... ]

> @@ -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;
>  
>  	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);

[Severity: High]
Does this code still allow an unbounded memory allocation bypass and a
resource leak?

While a totalram_pages() limit check was added to rd_build_device_space()
above, the configfs control attribute can dynamically update
rd_dev->rd_page_count via rd_set_configfs_dev_params() after the device
is already configured:

drivers/target/target_core_rd.c:rd_set_configfs_dev_params() {
    ...
    rd_dev->rd_page_count = arg;
    ...
}

If a massive page count is set dynamically and then PI protection is enabled
by writing to the pi_prot_type attribute, rd_init_prot() calls
rd_build_prot_space(). The bypassed rd_page_count is used to calculate
total_sg_needed here, potentially re-triggering the memory exhaustion
without any limit checks.

Additionally, if the large allocation fails downstream in
rd_allocate_sgl_table(), rd_build_prot_space() returns the error but leaves
rd_dev->sg_prot_array pointing to the partially populated table without
freeing it. Since pi_prot_type transitions back to 0 on failure, can the user
repeatedly trigger this path to leak system memory?

>  	if (!sg_table)
>  		return -ENOMEM;
>

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