[PATCH v3 0/3] scsi: target: rd: Fix oversized ramdisk allocations

[PATCH v3 0/3] 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 3 allocates the
backing pages with __GFP_RETRY_MAYFAIL so that enabling such a device
fails with -ENOMEM instead.

That makes the failure path in rd_build_prot_space() reachable, and it
leaks the partially allocated protection space when pi_prot_type is
written again. Patch 2 fixes that first, so that patch 3 does not
introduce a leak.

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. The leak was checked with kmemleak by making
pi_prot_type=1 fail under memory pressure and then writing it again:
two unreferenced objects from rd_init_prot() are reported without
patch 2, none with it.

Changes in v3:
- New patch 2 to release the protection space when its allocation
  fails (Sashiko AI review).

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 to avoid the OOM killer when the backing pages cannot be
  allocated (Sashiko AI review).

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


Sanan Hasanov (3):
  scsi: target: rd: Fix oversized sg table array allocation
  scsi: target: rd: Release protection space on allocation failure
  scsi: target: rd: Don't invoke the OOM killer for ramdisk pages

 drivers/target/target_core_rd.c | 20 +++++++++++++++-----
 1 file changed, 15 insertions(+), 5 deletions(-)


base-commit: 6c377d19d4a5116d9bec5203aa3c6c11523e7898
-- 
2.48.1

[PATCH v3 1/3] 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

Re: [PATCH v3 1/3] 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: 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

[PATCH v3 2/3] scsi: target: rd: Release protection space on allocation failure

[Sanan Hasanov]

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

When rd_allocate_sgl_table() fails, rd_build_prot_space() returns the
error but leaves the partially populated sg_prot_array in place.
pi_prot_type_store() then restores the previous protection type without
calling ->free_prot(), so the pages and scatterlists allocated so far
stay attached to the device. If the user writes pi_prot_type again,
rd_build_prot_space() overwrites sg_prot_array and the earlier
allocation is leaked.

Release the protection space before returning the error, as
rd_configure_device() already does for the data space.

Fixes: d7e8eb5d9216 ("target/rd: Add support for protection SGL setup + release")
Signed-off-by: Sanan Hasanov <sanan.hasanov@ucf.edu>
---
 drivers/target/target_core_rd.c | 4 +++-
 1 file changed, 3 insertions(+), 1 deletion(-)

diff --git a/drivers/target/target_core_rd.c b/drivers/target/target_core_rd.c
index 97c63b122..b73bdd733 100644
--- a/drivers/target/target_core_rd.c
+++ b/drivers/target/target_core_rd.c
@@ -263,8 +263,10 @@ static int rd_build_prot_space(struct rd_dev *rd_dev, int prot_length, int block
 	rd_dev->sg_prot_count = sg_tables;
 
 	rc = rd_allocate_sgl_table(rd_dev, sg_table, total_sg_needed, 0xff);
-	if (rc)
+	if (rc) {
+		rd_release_prot_space(rd_dev);
 		return rc;
+	}
 
 	pr_debug("CORE_RD[%u] - Built Ramdisk Device ID: %u prot space of"
 		 " %u pages in %u tables\n", rd_dev->rd_host->rd_host_id,
-- 
2.48.1

Re: [PATCH v3 2/3] scsi: target: rd: Release protection space on allocation failure

[sashiko-bot]

Thank you for your contribution! Sashiko AI review found 1 potential issue(s) to consider:
- [High] target: UAF in rd_build_prot_space() error path during concurrent configfs writes
--

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

scsi: target: rd: Release protection space on allocation failure

When rd_allocate_sgl_table() fails, rd_build_prot_space() returns the error
but leaves the partially populated sg_prot_array in place. This commit adds
a call to rd_release_prot_space() before returning the error to prevent
leaking the previous allocation.

> diff --git a/drivers/target/target_core_rd.c b/drivers/target/target_core_rd.c
> index 97c63b12266fb..b73bdd733928e 100644
> --- a/drivers/target/target_core_rd.c
> +++ b/drivers/target/target_core_rd.c
> @@ -263,8 +263,10 @@ static int rd_build_prot_space(struct rd_dev *rd_dev, int prot_length, int block
>  	rd_dev->sg_prot_count = sg_tables;
>  
>  	rc = rd_allocate_sgl_table(rd_dev, sg_table, total_sg_needed, 0xff);
> -	if (rc)
> +	if (rc) {
> +		rd_release_prot_space(rd_dev);

[Severity: High]
Could this introduce a use-after-free if triggered by concurrent configfs
writes?

Looking at pi_prot_type_store() in drivers/target/target_core_configfs.c,
there appears to be no locking before checking old_prot and calling
dev->transport->init_prot(dev):

	int old_prot = da->pi_prot_type, ret;
	struct se_device *dev = da->da_dev;
	...
	da->pi_prot_type = flag;

	if (flag && !old_prot) {
		ret = dev->transport->init_prot(dev);

If multiple threads write to pi_prot_type concurrently, they can both enter
rd_build_prot_space() and concurrently overwrite the global
rd_dev->sg_prot_array pointer:

	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 one thread's allocation fails, it executes this new error path which
reads rd_dev->sg_prot_array and frees it. This could free the active
scatterlist array that is currently being populated by the other racing
thread, causing a use-after-free of the scatterlist table and pages.

>  		return rc;
> +	}
>  
>  	pr_debug("CORE_RD[%u] - Built Ramdisk Device ID: %u prot space of"
>  		 " %u pages in %u tables\n", rd_dev->rd_host->rd_host_id,

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

[PATCH v3 3/3] 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 b73bdd733..7c53d8a72 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