[PATCH v2 0/2] scsi: target: rd: Fix oversized ramdisk allocations

[PATCH v2 0/2] scsi: target: rd: Fix oversized ramdisk allocations

[Sanan Hasanov]

From: Sanan Hasanov <sanan.hasanov@ucf.edu>

A ramdisk page count set through configfs (rd_pages=) has no upper bound.
syzbot found that a large value makes rd_build_device_space() attempt a
single sg table array allocation above the page allocator's maximum
order, which triggers a WARNING. Patch 1 rejects page counts that
exceed system RAM and allocates the array with kvzalloc so that valid
large ramdisks also work.

A page count close to system RAM still passes that check, and the
backing pages are allocated with GFP_KERNEL, which invokes the OOM
killer instead of failing. On a 1 GiB VM, rd_pages=250000 panics the
system with "System is deadlocked on memory". Patch 2 allocates the
backing pages with __GFP_RETRY_MAYFAIL so that enabling such a device
fails with -ENOMEM instead.

Tested on an arm64 QEMU VM with 1 GiB RAM, with the syzbot reproducer,
normal and NULLIO ramdisks, DIF protection space, device removal, and
rd_pages=250000.

Changes in v2:
- Allocate the sg table arrays with kvzalloc_objs() so that ramdisks
  larger than 512 GiB do not hit the same WARNING (Sashiko AI review).
- New patch 2 to avoid the OOM killer when the backing pages cannot be
  allocated (Sashiko AI review).

v1: https://lore.kernel.org/all/20261008215709.46014-1-sanan.hasanou@gmail.com/

Sanan Hasanov (2):
  scsi: target: rd: Fix oversized sg table array allocation
  scsi: target: rd: Don't invoke the OOM killer for ramdisk pages

 drivers/target/target_core_rd.c | 16 ++++++++++++----
 1 file changed, 12 insertions(+), 4 deletions(-)


base-commit: 6c377d19d4a5116d9bec5203aa3c6c11523e7898
-- 
2.48.1

[PATCH v2 1/2] scsi: target: rd: Fix oversized sg table array allocation

[Sanan Hasanov]

From: Sanan Hasanov <sanan.hasanov@ucf.edu>

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:

  WARNING: mm/page_alloc.c:5340 at __alloc_frozen_pages_noprof+0x294/0x874
  Call trace:
   __alloc_frozen_pages_noprof+0x294/0x874 (P)
   ___kmalloc_large_node+0x64/0xd0
   __kmalloc_noprof+0x24/0x54
   rd_configure_device+0x74/0xdc
   target_configure_device+0xa0/0x1c4
   target_dev_enable_store+0x4c/0x5c
   configfs_write_iter+0xec/0x124

A ramdisk is backed page-by-page by system memory, so a page count
larger than totalram_pages() can never be satisfied. Reject it with
-EINVAL before allocating anything. NULLIO devices allocate no backing
pages and are not affected.

The array needs 64 bytes per 2048 pages, so even a valid page count
exceeds the 4 MiB maximum order once the ramdisk is larger than 512 GiB.
Allocate the sg table arrays with kvzalloc_objs() so that large arrays
fall back to vmalloc.

Fixes: c66ac9db8d4a ("[SCSI] target: Add LIO target core v4.0.0-rc6")
Reported-by: syzbot+fa495e1497c48a6ed885@syzkaller.appspotmail.com
Closes: https://syzkaller.appspot.com/bug?extid=fa495e1497c48a6ed885
Signed-off-by: Sanan Hasanov <sanan.hasanov@ucf.edu>
---
 drivers/target/target_core_rd.c | 13 ++++++++++---
 1 file changed, 10 insertions(+), 3 deletions(-)

diff --git a/drivers/target/target_core_rd.c b/drivers/target/target_core_rd.c
index 092d9fe0d..97c63b122 100644
--- a/drivers/target/target_core_rd.c
+++ b/drivers/target/target_core_rd.c
@@ -14,6 +14,7 @@
 #include <linux/string.h>
 #include <linux/parser.h>
 #include <linux/highmem.h>
+#include <linux/mm.h>
 #include <linux/timer.h>
 #include <linux/scatterlist.h>
 #include <linux/slab.h>
@@ -81,7 +82,7 @@ static u32 rd_release_sgl_table(struct rd_dev *rd_dev, struct rd_dev_sg_table *s
 		kfree(sg);
 	}
 
-	kfree(sg_table);
+	kvfree(sg_table);
 	return page_count;
 }
 
@@ -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);
 	if (!sg_table)
 		return -ENOMEM;
 
-- 
2.48.1

[PATCH v2 2/2] scsi: target: rd: Don't invoke the OOM killer for ramdisk pages

[Sanan Hasanov]

From: Sanan Hasanov <sanan.hasanov@ucf.edu>

rd_allocate_sgl_table() allocates the ramdisk's backing memory one page
at a time with GFP_KERNEL. When the requested ramdisk is larger than the
memory that can be freed, these allocations do not fail but invoke the
OOM killer instead. The pages are not charged to any task, so the OOM
killer keeps killing unrelated processes, and may panic the system once
nothing is left to kill, while the configfs write continues allocating.

Use __GFP_RETRY_MAYFAIL so that the allocation fails once reclaim can
make no more progress. rd_allocate_sgl_table() already handles failure
by returning -ENOMEM, and the caller then releases the pages allocated
so far. Add __GFP_NOWARN since the driver logs its own error.

Signed-off-by: Sanan Hasanov <sanan.hasanov@ucf.edu>
---
 drivers/target/target_core_rd.c | 3 ++-
 1 file changed, 2 insertions(+), 1 deletion(-)

diff --git a/drivers/target/target_core_rd.c b/drivers/target/target_core_rd.c
index 97c63b122..46698300e 100644
--- a/drivers/target/target_core_rd.c
+++ b/drivers/target/target_core_rd.c
@@ -150,7 +150,8 @@ static int rd_allocate_sgl_table(struct rd_dev *rd_dev, struct rd_dev_sg_table *
 						- 1;
 
 		for (j = 0; j < sg_per_table; j++) {
-			pg = alloc_pages(GFP_KERNEL, 0);
+			pg = alloc_pages(GFP_KERNEL | __GFP_RETRY_MAYFAIL |
+					 __GFP_NOWARN, 0);
 			if (!pg) {
 				pr_err("Unable to allocate scatterlist"
 					" pages for struct rd_dev_sg_table\n");
-- 
2.48.1

Re: [PATCH v2 2/2] scsi: target: rd: Don't invoke the OOM killer for ramdisk pages

[sashiko-bot]

Thank you for your contribution! Sashiko AI review found 1 potential issue(s) to consider:
- [Medium] target: rd: memory leak in rd_build_prot_space() on allocation failure
--

commit 08e2848928f98cb3a6b575947cf8d271014c318f
Author: Sanan Hasanov <sanan.hasanov@ucf.edu>
Subject: scsi: target: rd: Don't invoke the OOM killer for ramdisk pages

This commit replaces the default memory allocation flags for ramdisk backing
pages with GFP_KERNEL | __GFP_RETRY_MAYFAIL | __GFP_NOWARN to avoid invoking
the OOM killer. The pages are requested via alloc_pages() in
rd_allocate_sgl_table().

> diff --git a/drivers/target/target_core_rd.c b/drivers/target/target_core_rd.c
> index 97c63b12266fb..46698300e2063 100644
> --- a/drivers/target/target_core_rd.c
> +++ b/drivers/target/target_core_rd.c
> @@ -150,7 +150,8 @@ static int rd_allocate_sgl_table(struct rd_dev *rd_dev, struct rd_dev_sg_table *
>  						- 1;
>  
>  		for (j = 0; j < sg_per_table; j++) {
> -			pg = alloc_pages(GFP_KERNEL, 0);
> +			pg = alloc_pages(GFP_KERNEL | __GFP_RETRY_MAYFAIL |
> +					 __GFP_NOWARN, 0);
>  			if (!pg) {

[Severity: Medium]
Can this new failure path cause a regression by leaking memory in
rd_build_prot_space()?

Since this allocation uses __GFP_RETRY_MAYFAIL, it can now fail and return
-ENOMEM. When this occurs, rd_build_prot_space() propagates the error
without cleaning up the partially populated array:

drivers/target/target_core_rd.c:rd_build_prot_space() {
    ...
	rd_dev->sg_prot_array = sg_table;
	rd_dev->sg_prot_count = sg_tables;

	rc = rd_allocate_sgl_table(rd_dev, sg_table, total_sg_needed, 0xff);
	if (rc)
		return rc;
    ...
}

This propagates the error up to the configfs handler:

drivers/target/target_core_configfs.c:pi_prot_type_store() {
    ...
		ret = dev->transport->init_prot(dev);
		if (ret) {
			da->pi_prot_type = old_prot;
			da->pi_prot_verify = (bool) da->pi_prot_type;
			return ret;
		}
    ...
}

The configfs handler restores state and returns without explicitly calling
dev->transport->free_prot(dev) to clean up the backend.

This leaves the partially populated scatterlists and pages permanently
dangling in rd_dev->sg_prot_array. If the user retries enabling the
protection type, rd_build_prot_space() unconditionally overwrites the
rd_dev->sg_prot_array pointer with a new allocation, permanently leaking
the previously allocated memory resources.

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

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

[sashiko-bot]

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