[PATCH] scsi: target: rd: Check the return value of match_int()

[PATCH] scsi: target: rd: Check the return value of match_int()

[Shashank Mohan Jain]

rd_set_configfs_dev_params() ignores the return value of match_int()
for the rd_pages=, rd_nullio= and rd_dummy= options and then uses 'arg'
anyway.

match_token() only matches these options if the value is a number with
nothing after it, but match_int() can still fail with -ERANGE: when the
value is 24 characters or longer, on any architecture, and on 64-bit for
most values outside the range of an int.  'arg' is then not written.  It
still holds the value parsed for an earlier option in the same write, or
is uninitialized if no number was parsed yet.  The write succeeds, and
for rd_pages= that value becomes the page count and RDF_HAS_PAGE_COUNT
is set.  On 64-bit, for example, "rd_nullio=1,rd_pages=4294967296"
configures a one page device, and "rd_pages=4294967296" alone uses an
uninitialized page count (0 with CONFIG_INIT_STACK_ALL_ZERO, which is
only rejected when the device is enabled).  A stale 1 also turns on
RDF_NULLIO or RDF_DUMMY for an invalid rd_nullio= or rd_dummy= value.

Return the error from match_int(), as the fileio and pscsi backends do
for their match_int() options.  As there, options that come before the
invalid one in the same write have already been applied.

Fixes: c66ac9db8d4a ("[SCSI] target: Add LIO target core v4.0.0-rc6")
Assisted-by: LLM
Signed-off-by: Shashank Mohan Jain <jain.sm@gmail.com>
---
This patch was prepared with Claude Code (Anthropic), model Claude Opus 5.5
(claude-opus-5-5): the analysis, the fix, the changelog and the throwaway
test described below. The trailer only says "Assisted-by: LLM", as
Documentation/process/coding-assistants.rst describes.

Earlier attempts at this fix were not merged:
https://lore.kernel.org/r/1528779148-42485-1-git-send-email-jiazhouyang09@gmail.com
https://lore.kernel.org/r/20190112053159.99406-1-kjlu@umn.edu
This version follows the review comments there: it returns the error
from match_int() instead of -EINVAL, does not leak 'orig', covers
rd_nullio= (and rd_dummy=, added later), and goes through a single exit
path as the fileio backend does.  James Bottomley suggested
ignoring an invalid option instead of failing the write; I went with
returning the error so that a mistyped size is not silently replaced by
a stale or uninitialized one, but ignoring it (a 'break' instead of
'goto out') would also fix the bug.

Dependencies: none. The patch is correct on its own on current mainline.
It is related to "lib: parser: reject out-of-range values in
match_number()", sent to Andrew Morton:
https://lore.kernel.org/r/20260926012718.15675-1-jain.sm@gmail.com
With that patch, out-of-range values also fail on 32-bit (today they
wrap there, for example rd_pages=4294967296 gives 0 pages), and this
patch then turns that failure into an error instead of a stale value.

Testing done:
- W=1 build of drivers/target/target_core_rd.o with allmodconfig for
  x86_64 and i386: no warnings.
- A throwaway KUnit test (not part of this patch) called
  rd_set_configfs_dev_params() under UML (x86_64 and i386 subarch).
  Without the patch, on x86_64:
  "rd_nullio=1,rd_pages=4294967296" returns success with 1 page and
  RDF_HAS_PAGE_COUNT set; "rd_pages=4294967296" gives 0 pages with
  CONFIG_INIT_STACK_ALL_ZERO and 0xfefefefe pages with
  CONFIG_INIT_STACK_ALL_PATTERN; a 24-character rd_nullio= or rd_dummy=
  value of 0 after "rd_pages=1" sets RDF_NULLIO or RDF_DUMMY (also on
  i386).  With the patch all of these writes fail with -ERANGE, and valid
  values (rd_pages=8, rd_pages=0x10, "rd_pages=8,rd_nullio=1,rd_dummy=1",
  rd_pages=2147483647) are unchanged, on x86_64 and i386.

Not tested: writing to the configfs control file of a real rd_mcp device
with targetcli, and enabling such a device.

Not changed: a negative rd_pages= value still parses and becomes a huge
u32 page count, as before.

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

diff --git a/drivers/target/target_core_rd.c b/drivers/target/target_core_rd.c
index 092d9fe0d4e3..32762199e742 100644
--- a/drivers/target/target_core_rd.c
+++ b/drivers/target/target_core_rd.c
@@ -545,7 +545,7 @@ static ssize_t rd_set_configfs_dev_params(struct se_device *dev,
 	struct rd_dev *rd_dev = RD_DEV(dev);
 	char *orig, *ptr, *opts;
 	substring_t args[MAX_OPT_ARGS];
-	int arg, token;
+	int ret = 0, arg, token;
 
 	opts = kstrdup(page, GFP_KERNEL);
 	if (!opts)
@@ -560,14 +560,18 @@ static ssize_t rd_set_configfs_dev_params(struct se_device *dev,
 		token = match_token(ptr, tokens, args);
 		switch (token) {
 		case Opt_rd_pages:
-			match_int(args, &arg);
+			ret = match_int(args, &arg);
+			if (ret)
+				goto out;
 			rd_dev->rd_page_count = arg;
 			pr_debug("RAMDISK: Referencing Page"
 				" Count: %u\n", rd_dev->rd_page_count);
 			rd_dev->rd_flags |= RDF_HAS_PAGE_COUNT;
 			break;
 		case Opt_rd_nullio:
-			match_int(args, &arg);
+			ret = match_int(args, &arg);
+			if (ret)
+				goto out;
 			if (arg != 1)
 				break;
 
@@ -575,7 +579,9 @@ static ssize_t rd_set_configfs_dev_params(struct se_device *dev,
 			rd_dev->rd_flags |= RDF_NULLIO;
 			break;
 		case Opt_rd_dummy:
-			match_int(args, &arg);
+			ret = match_int(args, &arg);
+			if (ret)
+				goto out;
 			if (arg != 1)
 				break;
 
@@ -587,8 +593,9 @@ static ssize_t rd_set_configfs_dev_params(struct se_device *dev,
 		}
 	}
 
+out:
 	kfree(orig);
-	return count;
+	return (!ret) ? count : ret;
 }
 
 static ssize_t rd_show_configfs_dev_params(struct se_device *dev, char *b)
-- 
2.43.0