Re: [PATCH] scsi: target: rd: Reject page counts larger than total RAM

[sashiko-bot@kernel.org]

Thank you for your contribution! Sashiko AI review found 2 potential issue(s) to consider:
- [High] mm: OOM killer storm from unbounded ramdisk allocation
- [Medium] target: incomplete fix for MAX_PAGE_ORDER warning on large RAM systems
--

commit 12a2dad36a98577f1d3e561b02b29fc0a3802314
Author: Sanan Hasanov <sanan.hasanov@ucf.edu>

scsi: target: rd: Reject page counts larger than total RAM

The rd_pages= device parameter is taken from configfs without an upper
bound. rd_build_device_space() uses it to size the sg table array, so a
large value such as INT_MAX makes it attempt a kzalloc() of roughly
64 MiB, which is above the page allocator's maximum order and triggers:
[ ... ]

> diff --git a/drivers/target/target_core_rd.c b/drivers/target/target_core_rd.c
> index 092d9fe0d4e31..3700cc1b5d67c 100644
> --- a/drivers/target/target_core_rd.c
> +++ b/drivers/target/target_core_rd.c
[ ... ]
> @@ -188,6 +189,12 @@ 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()) {

[Severity: High]
Does this limit still allow a privileged user to request up to 100% of
physical memory, potentially triggering an OOM killer storm?

In drivers/target/target_core_rd.c:rd_allocate_sgl_table(), this translates
to looping totalram_pages() times and calling:

    pg = alloc_pages(GFP_KERNEL, 0);

Because GFP_KERNEL lacks __GFP_RETRY_MAYFAIL or __GFP_NORETRY, the page
allocator will aggressively retry by invoking the OOM killer when free memory
is exhausted.

Furthermore, because these allocations are not accounted to the caller's
memcg (no __GFP_ACCOUNT), the calling process maintains a low badness score,
which might cause the OOM killer to systemically terminate innocent
user-space processes.

[Severity: Medium]
Will this still exceed the page allocator's maximum order on systems with a
large amount of RAM (e.g., >= 512 GB)?

The size of the sg_table array is calculated in rd_build_device_space() as:

    sg_tables = (total_sg_needed / max_sg_per_table) + 1;

On a system with 1 TB of RAM, totalram_pages() is ~268 million pages. The
required sg_table array allocation size would be
(268,435,456 / 2048) * 64 = ~8 MiB.

An 8 MiB allocation exceeds the typical 4 MiB KMALLOC_MAX_SIZE
(MAX_PAGE_ORDER) limit of the page allocator, which means the subsequent
contiguous allocation in rd_build_device_space():

    sg_table = kzalloc_objs(*sg_table, sg_tables);

will still trigger the exact same kernel WARNING and -ENOMEM failure on
systems with high memory capacity.

Could kvcalloc() (and kvfree()) be used here to safely fall back to vmalloc
for large array sizes?

> +		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;

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