[PATCH] modpost: prevent stack buffer overflow in do_input_entry()

[PATCH] modpost: prevent stack buffer overflow in do_input_entry()

[Hasan Basbunar]

Several functions in scripts/mod/file2alias.c build the module alias
string by repeatedly appending into a fixed-size on-stack buffer:

	char alias[256] = {};
	...
	sprintf(alias + strlen(alias), "%X,*", i);

In do_input_entry() this pattern is unbounded across nine bitmap
classes (evbit/keybit/relbit/absbit/mscbit/ledbit/sndbit/ffbit/swbit).
The keybit case alone scans bits from INPUT_DEVICE_ID_KEY_MIN_INTERESTING
(0x71) to INPUT_DEVICE_ID_KEY_MAX (0x2ff), which is 655 iterations; if
a MODULE_DEVICE_TABLE(input, ...) populates keybit[] densely, the
emission reaches ~3132 bytes — overflowing the 256-byte buffer by
about 12x. include/linux/mod_devicetable.h declares storage for the
full bit range ("keybit[INPUT_DEVICE_ID_KEY_MAX / BITS_PER_LONG + 1]"),
so the worst case is reachable per the ABI.

No driver in the current tree triggers this — every in-tree user of
INPUT_DEVICE_ID_MATCH_KEYBIT populates keybit[] very sparsely (1-3
bits). The concern is defense-in-depth: the unbounded sprintf is a
silent stack-corruption primitive in a host build tool, and the buffer
size has not been revisited since this code was added in commit
1d8f430c15b3 ("[PATCH] Input: add modalias support", 2005).

Reproduced under AddressSanitizer with a stand-alone harness mirroring
the do_input loop on a fully-populated keybit:

  ==18319==ERROR: AddressSanitizer: stack-buffer-overflow
  WRITE of size 2 at offset 288 in frame [32, 288) 'alias'
    #6 do_input poc.c:44

  Stack-canary build:
  Abort trap: 6  (strlen(alias)=3134, cap was 256-1)

Add a small alias_append() helper around vsnprintf with cumulative
bookkeeping. It calls fatal() on overflow, matching the modpost style
for unrecoverable build conditions. do_input() takes the buffer size
as a new parameter; do_input_entry() passes sizeof(alias) at every
call site. This bounds every write into the on-stack buffer and turns
the latent overflow into a clean build error if it is ever reached.

Reported-by: Hasan Basbunar <basbunarhasan@gmail.com>
Signed-off-by: Hasan Basbunar <basbunarhasan@gmail.com>
---
 scripts/mod/file2alias.c | 70 ++++++++++++++++++++++++++++------------
 1 file changed, 50 insertions(+), 20 deletions(-)

diff --git a/scripts/mod/file2alias.c b/scripts/mod/file2alias.c
index 4e99393a35f1..c377e9a3761c 100644
--- a/scripts/mod/file2alias.c
+++ b/scripts/mod/file2alias.c
@@ -651,7 +651,36 @@ static void do_vio_entry(struct module *mod, void *symval)
 	module_alias_printf(mod, true, "%s", alias);
 }
 
-static void do_input(char *alias,
+/*
+ * alias_append() — bounded printf into a fixed-size alias buffer.
+ *
+ * Replaces the historical pattern  sprintf(alias + strlen(alias), ...)  used
+ * across this file. That pattern silently corrupts the stack when the
+ * cumulative formatted output exceeds the destination size; with a
+ * maliciously-crafted MODULE_DEVICE_TABLE() input the worst-case emission in
+ * do_input_entry() is about 12x the on-stack 256-byte buffer. Use snprintf
+ * with cumulative bookkeeping and abort the build on overflow.
+ */
+static void __attribute__((format(printf, 3, 4)))
+alias_append(char *alias, size_t size, const char *fmt, ...)
+{
+	size_t len = strlen(alias);
+	va_list args;
+	int n;
+
+	if (len >= size)
+		fatal("alias buffer (%zu) overflow before append\n", size);
+
+	va_start(args, fmt);
+	n = vsnprintf(alias + len, size - len, fmt, args);
+	va_end(args);
+
+	if (n < 0 || (size_t)n >= size - len)
+		fatal("alias buffer (%zu) overflow on append (need %d, have %zu)\n",
+		      size, n, size - len);
+}
+
+static void do_input(char *alias, size_t size,
 		     kernel_ulong_t *arr, unsigned int min, unsigned int max)
 {
 	unsigned int i;
@@ -659,13 +688,14 @@ static void do_input(char *alias,
 	for (i = min; i <= max; i++)
 		if (get_unaligned_native(arr + i / BITS_PER_LONG) &
 		    (1ULL << (i % BITS_PER_LONG)))
-			sprintf(alias + strlen(alias), "%X,*", i);
+			alias_append(alias, size, "%X,*", i);
 }
 
 /* input:b0v0p0e0-eXkXrXaXmXlXsXfXwX where X is comma-separated %02X. */
 static void do_input_entry(struct module *mod, void *symval)
 {
 	char alias[256] = {};
+	const size_t sizeof_alias = sizeof(alias);
 
 	DEF_FIELD(symval, input_device_id, flags);
 	DEF_FIELD(symval, input_device_id, bustype);
@@ -687,35 +717,35 @@ static void do_input_entry(struct module *mod, void *symval)
 	ADD(alias, "p", flags & INPUT_DEVICE_ID_MATCH_PRODUCT, product);
 	ADD(alias, "e", flags & INPUT_DEVICE_ID_MATCH_VERSION, version);
 
-	sprintf(alias + strlen(alias), "-e*");
+	alias_append(alias, sizeof_alias, "-e*");
 	if (flags & INPUT_DEVICE_ID_MATCH_EVBIT)
-		do_input(alias, *evbit, 0, INPUT_DEVICE_ID_EV_MAX);
-	sprintf(alias + strlen(alias), "k*");
+		do_input(alias, sizeof_alias, *evbit, 0, INPUT_DEVICE_ID_EV_MAX);
+	alias_append(alias, sizeof_alias, "k*");
 	if (flags & INPUT_DEVICE_ID_MATCH_KEYBIT)
-		do_input(alias, *keybit,
+		do_input(alias, sizeof_alias, *keybit,
 			 INPUT_DEVICE_ID_KEY_MIN_INTERESTING,
 			 INPUT_DEVICE_ID_KEY_MAX);
-	sprintf(alias + strlen(alias), "r*");
+	alias_append(alias, sizeof_alias, "r*");
 	if (flags & INPUT_DEVICE_ID_MATCH_RELBIT)
-		do_input(alias, *relbit, 0, INPUT_DEVICE_ID_REL_MAX);
-	sprintf(alias + strlen(alias), "a*");
+		do_input(alias, sizeof_alias, *relbit, 0, INPUT_DEVICE_ID_REL_MAX);
+	alias_append(alias, sizeof_alias, "a*");
 	if (flags & INPUT_DEVICE_ID_MATCH_ABSBIT)
-		do_input(alias, *absbit, 0, INPUT_DEVICE_ID_ABS_MAX);
-	sprintf(alias + strlen(alias), "m*");
+		do_input(alias, sizeof_alias, *absbit, 0, INPUT_DEVICE_ID_ABS_MAX);
+	alias_append(alias, sizeof_alias, "m*");
 	if (flags & INPUT_DEVICE_ID_MATCH_MSCIT)
-		do_input(alias, *mscbit, 0, INPUT_DEVICE_ID_MSC_MAX);
-	sprintf(alias + strlen(alias), "l*");
+		do_input(alias, sizeof_alias, *mscbit, 0, INPUT_DEVICE_ID_MSC_MAX);
+	alias_append(alias, sizeof_alias, "l*");
 	if (flags & INPUT_DEVICE_ID_MATCH_LEDBIT)
-		do_input(alias, *ledbit, 0, INPUT_DEVICE_ID_LED_MAX);
-	sprintf(alias + strlen(alias), "s*");
+		do_input(alias, sizeof_alias, *ledbit, 0, INPUT_DEVICE_ID_LED_MAX);
+	alias_append(alias, sizeof_alias, "s*");
 	if (flags & INPUT_DEVICE_ID_MATCH_SNDBIT)
-		do_input(alias, *sndbit, 0, INPUT_DEVICE_ID_SND_MAX);
-	sprintf(alias + strlen(alias), "f*");
+		do_input(alias, sizeof_alias, *sndbit, 0, INPUT_DEVICE_ID_SND_MAX);
+	alias_append(alias, sizeof_alias, "f*");
 	if (flags & INPUT_DEVICE_ID_MATCH_FFBIT)
-		do_input(alias, *ffbit, 0, INPUT_DEVICE_ID_FF_MAX);
-	sprintf(alias + strlen(alias), "w*");
+		do_input(alias, sizeof_alias, *ffbit, 0, INPUT_DEVICE_ID_FF_MAX);
+	alias_append(alias, sizeof_alias, "w*");
 	if (flags & INPUT_DEVICE_ID_MATCH_SWBIT)
-		do_input(alias, *swbit, 0, INPUT_DEVICE_ID_SW_MAX);
+		do_input(alias, sizeof_alias, *swbit, 0, INPUT_DEVICE_ID_SW_MAX);
 
 	module_alias_printf(mod, false, "input:%s", alias);
 }
-- 
2.53.0

Re: [PATCH] modpost: prevent stack buffer overflow in do_input_entry()

[Randy Dunlap]

Hi,

On 4/27/26 1:42 PM, Hasan Basbunar wrote:
> Several functions in scripts/mod/file2alias.c build the module alias
> string by repeatedly appending into a fixed-size on-stack buffer:
> 
> 	char alias[256] = {};
> 	...
> 	sprintf(alias + strlen(alias), "%X,*", i);
> 
> In do_input_entry() this pattern is unbounded across nine bitmap
> classes (evbit/keybit/relbit/absbit/mscbit/ledbit/sndbit/ffbit/swbit).
> The keybit case alone scans bits from INPUT_DEVICE_ID_KEY_MIN_INTERESTING
> (0x71) to INPUT_DEVICE_ID_KEY_MAX (0x2ff), which is 655 iterations; if
> a MODULE_DEVICE_TABLE(input, ...) populates keybit[] densely, the
> emission reaches ~3132 bytes — overflowing the 256-byte buffer by
> about 12x. include/linux/mod_devicetable.h declares storage for the
> full bit range ("keybit[INPUT_DEVICE_ID_KEY_MAX / BITS_PER_LONG + 1]"),
> so the worst case is reachable per the ABI.
> 

do_input() is the only do_function() that accepts such a large array "arr".
Did you look at the other do_functions() and conclude that there are no other
issues?
I wonder how large an alias do_dmi_entry() could produce?

> No driver in the current tree triggers this — every in-tree user of
> INPUT_DEVICE_ID_MATCH_KEYBIT populates keybit[] very sparsely (1-3
> bits). The concern is defense-in-depth: the unbounded sprintf is a
> silent stack-corruption primitive in a host build tool, and the buffer
> size has not been revisited since this code was added in commit
> 1d8f430c15b3 ("[PATCH] Input: add modalias support", 2005).

Maybe Fixes: that?

> Reproduced under AddressSanitizer with a stand-alone harness mirroring
> the do_input loop on a fully-populated keybit:
> 
>   ==18319==ERROR: AddressSanitizer: stack-buffer-overflow
>   WRITE of size 2 at offset 288 in frame [32, 288) 'alias'
>     #6 do_input poc.c:44
> 
>   Stack-canary build:
>   Abort trap: 6  (strlen(alias)=3134, cap was 256-1)
> 
> Add a small alias_append() helper around vsnprintf with cumulative
> bookkeeping. It calls fatal() on overflow, matching the modpost style

I probably wouldn't call that cumulative accounting.
It takes strlen(current alias) each time that it's called.
(but this is just a nit :)

> for unrecoverable build conditions. do_input() takes the buffer size
> as a new parameter; do_input_entry() passes sizeof(alias) at every
> call site. This bounds every write into the on-stack buffer and turns
> the latent overflow into a clean build error if it is ever reached.
> 
> Reported-by: Hasan Basbunar <basbunarhasan@gmail.com>
> Signed-off-by: Hasan Basbunar <basbunarhasan@gmail.com>
> ---
>  scripts/mod/file2alias.c | 70 ++++++++++++++++++++++++++++------------
>  1 file changed, 50 insertions(+), 20 deletions(-)

Reviewed-by: Randy Dunlap <rdunlap@infradead.org>
Tested-by: Randy Dunlap <rdunlap@infradead.org>

Tested means that the before & after versions of
modules.builtin.modinfo are the same.

-- 
~Randy

[PATCH v2] modpost: prevent stack buffer overflow in do_input_entry() and do_dmi_entry()

[Hasan Basbunar]

Several functions in scripts/mod/file2alias.c build the module alias
string by repeatedly appending into a fixed-size on-stack buffer:

	char alias[256] = {};
	...
	sprintf(alias + strlen(alias), "%X,*", i);

This pattern is unbounded and silently corrupts the stack when the
formatted output exceeds the destination size. Two functions in this
file are realistically reachable with input that overflows their
buffer:

1. do_input_entry() appends across nine bitmap classes
   (evbit/keybit/relbit/absbit/mscbit/ledbit/sndbit/ffbit/swbit). The
   keybit case alone scans bits from INPUT_DEVICE_ID_KEY_MIN_INTERESTING
   (0x71) to INPUT_DEVICE_ID_KEY_MAX (0x2ff), 655 iterations; if a
   MODULE_DEVICE_TABLE(input, ...) populates keybit[] densely, the
   emission reaches ~3132 bytes — overflowing the 256-byte buffer by
   about 12x. include/linux/mod_devicetable.h declares storage for the
   full bit range ("keybit[INPUT_DEVICE_ID_KEY_MAX / BITS_PER_LONG + 1]"),
   so the worst case is reachable per the ABI.

2. do_dmi_entry() emits one ":<prefix>*<filtered_substr>*" segment per
   matched DMI field, up to 4 matches per dmi_system_id. Each substr
   is sized as char[79] in struct dmi_strmatch (mod_devicetable.h:584),
   and dmi_ascii_filter() copies it verbatim into the alias buffer
   without bounds. Worst case: 4 × (1 + 3 + 1 + 79 + 1) = 336 bytes
   into alias[256], an 80-byte overflow.

No driver in the current tree triggers either case — every in-tree
INPUT_DEVICE_ID_MATCH_KEYBIT user populates keybit[] very sparsely
(1-3 bits), and no in-tree dmi_system_id has four maximally-long
matches. The concern is defense-in-depth: both unbounded sprintf
chains are silent stack-corruption primitives in a host build tool,
and the buffer sizes have not been revisited since the corresponding
code was first introduced.

The other do_*_entry() handlers in this file (do_usb_entry,
do_cpu_entry, do_typec_entry, ...) were audited and are bounded by
their input field sizes (uint16 IDs, fixed-length keys); their alias
buffers do not need this treatment.

Reproduced under AddressSanitizer with a stand-alone harness mirroring
do_input on a fully-populated keybit:

  ==18319==ERROR: AddressSanitizer: stack-buffer-overflow
  WRITE of size 2 at offset 288 in frame [32, 288) 'alias'
    #6 do_input poc.c:44

  Stack-canary build:
  Abort trap: 6  (strlen(alias)=3134, cap was 256-1)

Add a small alias_append() helper around vsnprintf with a remaining-
space check and call fatal() on overflow, matching the modpost style
for unrecoverable build conditions. do_input() takes the buffer size
as a new parameter; do_input_entry() and do_dmi_entry() pass
sizeof(alias) at every call site. dmi_ascii_filter() takes the
remaining buffer size as well and aborts on truncation. This bounds
every write into the on-stack buffers and turns the latent overflow
into a clean build error if it is ever reached.

Fixes: 1d8f430c15b3 ("[PATCH] Input: add modalias support")
Signed-off-by: Hasan Basbunar <basbunarhasan@gmail.com>
---
v1: https://lore.kernel.org/lkml/20260427204255.22117-1-basbunarhasan@gmail.com/

Changes since v1 (per Randy Dunlap's review):
- Audited the other do_*_entry() handlers; do_dmi_entry() has the same
  unbounded-sprintf shape with a realistic 80-byte worst-case overflow,
  and is fixed in v2 alongside do_input_entry(). The remaining
  do_*_entry() handlers were verified bounded by their input field
  types and do not need this treatment.
- Added a Fixes: tag pointing to the original do_input introduction
  (commit 1d8f430c15b3, 2005).
- Reworded the alias_append() comment: replaced "cumulative
  bookkeeping" with "remaining-space check", which is what the helper
  actually does.

Randy: I have not carried forward your Reviewed-by/Tested-by from v1
because v2 expands scope to do_dmi_entry() (new code you have not seen
yet); please re-affirm if v2 looks good to you.

---
 scripts/mod/file2alias.c | 91 ++++++++++++++++++++++++++++------------
 1 file changed, 65 insertions(+), 26 deletions(-)

diff --git a/scripts/mod/file2alias.c b/scripts/mod/file2alias.c
index 4e99393a35f1..9ec5c4e1f3ed 100644
--- a/scripts/mod/file2alias.c
+++ b/scripts/mod/file2alias.c
@@ -651,7 +651,38 @@ static void do_vio_entry(struct module *mod, void *symval)
 	module_alias_printf(mod, true, "%s", alias);
 }
 
-static void do_input(char *alias,
+/*
+ * alias_append() — bounded printf-append into a fixed-size alias buffer.
+ *
+ * Replaces the historical pattern  sprintf(alias + strlen(alias), ...)
+ * used across this file. That pattern silently corrupts the stack when
+ * the formatted output exceeds the destination size; the worst-case
+ * emission in do_input_entry() with a maximally-populated keybit[] is
+ * about 12x the on-stack 256-byte buffer, and do_dmi_entry() can also
+ * exceed its 256 bytes for four maximal-length DMI matches. Use
+ * snprintf with a remaining-space check and abort the build on
+ * overflow.
+ */
+static void __attribute__((format(printf, 3, 4)))
+alias_append(char *alias, size_t size, const char *fmt, ...)
+{
+	size_t len = strlen(alias);
+	va_list args;
+	int n;
+
+	if (len >= size)
+		fatal("alias buffer (%zu) overflow before append\n", size);
+
+	va_start(args, fmt);
+	n = vsnprintf(alias + len, size - len, fmt, args);
+	va_end(args);
+
+	if (n < 0 || (size_t)n >= size - len)
+		fatal("alias buffer (%zu) overflow on append (need %d, have %zu)\n",
+		      size, n, size - len);
+}
+
+static void do_input(char *alias, size_t size,
 		     kernel_ulong_t *arr, unsigned int min, unsigned int max)
 {
 	unsigned int i;
@@ -659,13 +690,14 @@ static void do_input(char *alias,
 	for (i = min; i <= max; i++)
 		if (get_unaligned_native(arr + i / BITS_PER_LONG) &
 		    (1ULL << (i % BITS_PER_LONG)))
-			sprintf(alias + strlen(alias), "%X,*", i);
+			alias_append(alias, size, "%X,*", i);
 }
 
 /* input:b0v0p0e0-eXkXrXaXmXlXsXfXwX where X is comma-separated %02X. */
 static void do_input_entry(struct module *mod, void *symval)
 {
 	char alias[256] = {};
+	const size_t sizeof_alias = sizeof(alias);
 
 	DEF_FIELD(symval, input_device_id, flags);
 	DEF_FIELD(symval, input_device_id, bustype);
@@ -687,35 +719,35 @@ static void do_input_entry(struct module *mod, void *symval)
 	ADD(alias, "p", flags & INPUT_DEVICE_ID_MATCH_PRODUCT, product);
 	ADD(alias, "e", flags & INPUT_DEVICE_ID_MATCH_VERSION, version);
 
-	sprintf(alias + strlen(alias), "-e*");
+	alias_append(alias, sizeof_alias, "-e*");
 	if (flags & INPUT_DEVICE_ID_MATCH_EVBIT)
-		do_input(alias, *evbit, 0, INPUT_DEVICE_ID_EV_MAX);
-	sprintf(alias + strlen(alias), "k*");
+		do_input(alias, sizeof_alias, *evbit, 0, INPUT_DEVICE_ID_EV_MAX);
+	alias_append(alias, sizeof_alias, "k*");
 	if (flags & INPUT_DEVICE_ID_MATCH_KEYBIT)
-		do_input(alias, *keybit,
+		do_input(alias, sizeof_alias, *keybit,
 			 INPUT_DEVICE_ID_KEY_MIN_INTERESTING,
 			 INPUT_DEVICE_ID_KEY_MAX);
-	sprintf(alias + strlen(alias), "r*");
+	alias_append(alias, sizeof_alias, "r*");
 	if (flags & INPUT_DEVICE_ID_MATCH_RELBIT)
-		do_input(alias, *relbit, 0, INPUT_DEVICE_ID_REL_MAX);
-	sprintf(alias + strlen(alias), "a*");
+		do_input(alias, sizeof_alias, *relbit, 0, INPUT_DEVICE_ID_REL_MAX);
+	alias_append(alias, sizeof_alias, "a*");
 	if (flags & INPUT_DEVICE_ID_MATCH_ABSBIT)
-		do_input(alias, *absbit, 0, INPUT_DEVICE_ID_ABS_MAX);
-	sprintf(alias + strlen(alias), "m*");
+		do_input(alias, sizeof_alias, *absbit, 0, INPUT_DEVICE_ID_ABS_MAX);
+	alias_append(alias, sizeof_alias, "m*");
 	if (flags & INPUT_DEVICE_ID_MATCH_MSCIT)
-		do_input(alias, *mscbit, 0, INPUT_DEVICE_ID_MSC_MAX);
-	sprintf(alias + strlen(alias), "l*");
+		do_input(alias, sizeof_alias, *mscbit, 0, INPUT_DEVICE_ID_MSC_MAX);
+	alias_append(alias, sizeof_alias, "l*");
 	if (flags & INPUT_DEVICE_ID_MATCH_LEDBIT)
-		do_input(alias, *ledbit, 0, INPUT_DEVICE_ID_LED_MAX);
-	sprintf(alias + strlen(alias), "s*");
+		do_input(alias, sizeof_alias, *ledbit, 0, INPUT_DEVICE_ID_LED_MAX);
+	alias_append(alias, sizeof_alias, "s*");
 	if (flags & INPUT_DEVICE_ID_MATCH_SNDBIT)
-		do_input(alias, *sndbit, 0, INPUT_DEVICE_ID_SND_MAX);
-	sprintf(alias + strlen(alias), "f*");
+		do_input(alias, sizeof_alias, *sndbit, 0, INPUT_DEVICE_ID_SND_MAX);
+	alias_append(alias, sizeof_alias, "f*");
 	if (flags & INPUT_DEVICE_ID_MATCH_FFBIT)
-		do_input(alias, *ffbit, 0, INPUT_DEVICE_ID_FF_MAX);
-	sprintf(alias + strlen(alias), "w*");
+		do_input(alias, sizeof_alias, *ffbit, 0, INPUT_DEVICE_ID_FF_MAX);
+	alias_append(alias, sizeof_alias, "w*");
 	if (flags & INPUT_DEVICE_ID_MATCH_SWBIT)
-		do_input(alias, *swbit, 0, INPUT_DEVICE_ID_SW_MAX);
+		do_input(alias, sizeof_alias, *swbit, 0, INPUT_DEVICE_ID_SW_MAX);
 
 	module_alias_printf(mod, false, "input:%s", alias);
 }
@@ -895,12 +927,16 @@ static const struct dmifield {
 	{ NULL,  DMI_NONE }
 };
 
-static void dmi_ascii_filter(char *d, const char *s)
+static void dmi_ascii_filter(char *d, size_t avail, const char *s)
 {
 	/* Filter out characters we don't want to see in the modalias string */
 	for (; *s; s++)
-		if (*s > ' ' && *s < 127 && *s != ':')
+		if (*s > ' ' && *s < 127 && *s != ':') {
+			if (avail <= 1)
+				fatal("%s: alias buffer overflow\n", __func__);
 			*(d++) = *s;
+			avail--;
+		}
 
 	*d = 0;
 }
@@ -909,6 +945,8 @@ static void dmi_ascii_filter(char *d, const char *s)
 static void do_dmi_entry(struct module *mod, void *symval)
 {
 	char alias[256] = {};
+	const size_t sizeof_alias = sizeof(alias);
+	size_t len;
 	int i, j;
 	DEF_FIELD_ADDR(symval, dmi_system_id, matches);
 
@@ -916,11 +954,12 @@ static void do_dmi_entry(struct module *mod, void *symval)
 		for (j = 0; j < 4; j++) {
 			if ((*matches)[j].slot &&
 			    (*matches)[j].slot == dmi_fields[i].field) {
-				sprintf(alias + strlen(alias), ":%s*",
-					dmi_fields[i].prefix);
-				dmi_ascii_filter(alias + strlen(alias),
+				alias_append(alias, sizeof_alias, ":%s*",
+					     dmi_fields[i].prefix);
+				len = strlen(alias);
+				dmi_ascii_filter(alias + len, sizeof_alias - len,
 						 (*matches)[j].substr);
-				strcat(alias, "*");
+				alias_append(alias, sizeof_alias, "*");
 			}
 		}
 	}
-- 
2.53.0

Re: [PATCH v2] modpost: prevent stack buffer overflow in do_input_entry() and do_dmi_entry()

[Randy Dunlap]

On 4/27/26 11:29 PM, Hasan Basbunar wrote:
> Several functions in scripts/mod/file2alias.c build the module alias
> string by repeatedly appending into a fixed-size on-stack buffer:
> 
> 	char alias[256] = {};
> 	...
> 	sprintf(alias + strlen(alias), "%X,*", i);
> 
> This pattern is unbounded and silently corrupts the stack when the
> formatted output exceeds the destination size. Two functions in this
> file are realistically reachable with input that overflows their
> buffer:
> 
[snip]
> 
> Fixes: 1d8f430c15b3 ("[PATCH] Input: add modalias support")
> Signed-off-by: Hasan Basbunar <basbunarhasan@gmail.com>

LGTM. Thanks.

Reviewed-by: Randy Dunlap <rdunlap@infradead.org>
Tested-by: Randy Dunlap <rdunlap@infradead.org>

where testing means that modules.builtin.modinfo files before & after
are the same.

> ---
> v1: https://lore.kernel.org/lkml/20260427204255.22117-1-basbunarhasan@gmail.com/
> 
> Changes since v1 (per Randy Dunlap's review):
> - Audited the other do_*_entry() handlers; do_dmi_entry() has the same
>   unbounded-sprintf shape with a realistic 80-byte worst-case overflow,
>   and is fixed in v2 alongside do_input_entry(). The remaining
>   do_*_entry() handlers were verified bounded by their input field
>   types and do not need this treatment.
> - Added a Fixes: tag pointing to the original do_input introduction
>   (commit 1d8f430c15b3, 2005).
> - Reworded the alias_append() comment: replaced "cumulative
>   bookkeeping" with "remaining-space check", which is what the helper
>   actually does.
> 
> Randy: I have not carried forward your Reviewed-by/Tested-by from v1
> because v2 expands scope to do_dmi_entry() (new code you have not seen
> yet); please re-affirm if v2 looks good to you.
> 
> ---
>  scripts/mod/file2alias.c | 91 ++++++++++++++++++++++++++++------------
>  1 file changed, 65 insertions(+), 26 deletions(-)
> 

-- 
~Randy

Re: [PATCH v2] modpost: prevent stack buffer overflow in do_input_entry() and do_dmi_entry()

[Nicolas Schier]

On Tue, Apr 28, 2026 at 08:29:12AM +0200, Hasan Basbunar wrote:
> Several functions in scripts/mod/file2alias.c build the module alias
> string by repeatedly appending into a fixed-size on-stack buffer:
> 
> 	char alias[256] = {};
> 	...
> 	sprintf(alias + strlen(alias), "%X,*", i);
> 
> This pattern is unbounded and silently corrupts the stack when the
> formatted output exceeds the destination size. Two functions in this
> file are realistically reachable with input that overflows their
> buffer:
> 
> 1. do_input_entry() appends across nine bitmap classes
>    (evbit/keybit/relbit/absbit/mscbit/ledbit/sndbit/ffbit/swbit). The
>    keybit case alone scans bits from INPUT_DEVICE_ID_KEY_MIN_INTERESTING
>    (0x71) to INPUT_DEVICE_ID_KEY_MAX (0x2ff), 655 iterations; if a
>    MODULE_DEVICE_TABLE(input, ...) populates keybit[] densely, the
>    emission reaches ~3132 bytes — overflowing the 256-byte buffer by
>    about 12x. include/linux/mod_devicetable.h declares storage for the
>    full bit range ("keybit[INPUT_DEVICE_ID_KEY_MAX / BITS_PER_LONG + 1]"),
>    so the worst case is reachable per the ABI.
> 
> 2. do_dmi_entry() emits one ":<prefix>*<filtered_substr>*" segment per
>    matched DMI field, up to 4 matches per dmi_system_id. Each substr
>    is sized as char[79] in struct dmi_strmatch (mod_devicetable.h:584),
>    and dmi_ascii_filter() copies it verbatim into the alias buffer
>    without bounds. Worst case: 4 × (1 + 3 + 1 + 79 + 1) = 336 bytes
>    into alias[256], an 80-byte overflow.
> 
> No driver in the current tree triggers either case — every in-tree
> INPUT_DEVICE_ID_MATCH_KEYBIT user populates keybit[] very sparsely
> (1-3 bits), and no in-tree dmi_system_id has four maximally-long
> matches. The concern is defense-in-depth: both unbounded sprintf
> chains are silent stack-corruption primitives in a host build tool,
> and the buffer sizes have not been revisited since the corresponding
> code was first introduced.
> 
> The other do_*_entry() handlers in this file (do_usb_entry,
> do_cpu_entry, do_typec_entry, ...) were audited and are bounded by
> their input field sizes (uint16 IDs, fixed-length keys); their alias
> buffers do not need this treatment.
> 
> Reproduced under AddressSanitizer with a stand-alone harness mirroring
> do_input on a fully-populated keybit:
> 
>   ==18319==ERROR: AddressSanitizer: stack-buffer-overflow
>   WRITE of size 2 at offset 288 in frame [32, 288) 'alias'
>     #6 do_input poc.c:44
> 
>   Stack-canary build:
>   Abort trap: 6  (strlen(alias)=3134, cap was 256-1)
> 
> Add a small alias_append() helper around vsnprintf with a remaining-
> space check and call fatal() on overflow, matching the modpost style
> for unrecoverable build conditions. do_input() takes the buffer size
> as a new parameter; do_input_entry() and do_dmi_entry() pass
> sizeof(alias) at every call site. dmi_ascii_filter() takes the
> remaining buffer size as well and aborts on truncation. This bounds
> every write into the on-stack buffers and turns the latent overflow
> into a clean build error if it is ever reached.
> 
> Fixes: 1d8f430c15b3 ("[PATCH] Input: add modalias support")
> Signed-off-by: Hasan Basbunar <basbunarhasan@gmail.com>
> ---
> v1: https://lore.kernel.org/lkml/20260427204255.22117-1-basbunarhasan@gmail.com/
> 
> Changes since v1 (per Randy Dunlap's review):
> - Audited the other do_*_entry() handlers; do_dmi_entry() has the same
>   unbounded-sprintf shape with a realistic 80-byte worst-case overflow,
>   and is fixed in v2 alongside do_input_entry(). The remaining
>   do_*_entry() handlers were verified bounded by their input field
>   types and do not need this treatment.
> - Added a Fixes: tag pointing to the original do_input introduction
>   (commit 1d8f430c15b3, 2005).
> - Reworded the alias_append() comment: replaced "cumulative
>   bookkeeping" with "remaining-space check", which is what the helper
>   actually does.
> 
> Randy: I have not carried forward your Reviewed-by/Tested-by from v1
> because v2 expands scope to do_dmi_entry() (new code you have not seen
> yet); please re-affirm if v2 looks good to you.
> 
> ---
>  scripts/mod/file2alias.c | 91 ++++++++++++++++++++++++++++------------
>  1 file changed, 65 insertions(+), 26 deletions(-)
> 
> diff --git a/scripts/mod/file2alias.c b/scripts/mod/file2alias.c
> index 4e99393a35f1..9ec5c4e1f3ed 100644
> --- a/scripts/mod/file2alias.c
> +++ b/scripts/mod/file2alias.c
> @@ -651,7 +651,38 @@ static void do_vio_entry(struct module *mod, void *symval)
>  	module_alias_printf(mod, true, "%s", alias);
>  }
>  
> -static void do_input(char *alias,
> +/*
> + * alias_append() — bounded printf-append into a fixed-size alias buffer.
> + *


> + * Replaces the historical pattern  sprintf(alias + strlen(alias), ...)
> + * used across this file. That pattern silently corrupts the stack when
> + * the formatted output exceeds the destination size; the worst-case
> + * emission in do_input_entry() with a maximally-populated keybit[] is
> + * about 12x the on-stack 256-byte buffer, and do_dmi_entry() can also
> + * exceed its 256 bytes for four maximal-length DMI matches. Use
> + * snprintf with a remaining-space check and abort the build on
> + * overflow.

This is well-documented in the commit message (thanks for that!), and I
expect this to not become updated in case we run into the fatal() case
and update the maximum buffer sizes.  Thus, I'd rather not add that
stanza into the code at all.


Nevertheless, thanks a lot!  LGTM.

I am going to apply that to kbuild-fixes-unstable for some linux-next testing.

Kind regards,
Nicolas

[PATCH v3] modpost: prevent stack buffer overflow in do_input_entry() and do_dmi_entry()

[Hasan Basbunar]

Several functions in scripts/mod/file2alias.c build the module alias
string by repeatedly appending into a fixed-size on-stack buffer:

	char alias[256] = {};
	...
	sprintf(alias + strlen(alias), "%X,*", i);

This pattern is unbounded and silently corrupts the stack when the
formatted output exceeds the destination size. Two functions in this
file are realistically reachable with input that overflows their
buffer:

1. do_input_entry() appends across nine bitmap classes
   (evbit/keybit/relbit/absbit/mscbit/ledbit/sndbit/ffbit/swbit). The
   keybit case alone scans bits from INPUT_DEVICE_ID_KEY_MIN_INTERESTING
   (0x71) to INPUT_DEVICE_ID_KEY_MAX (0x2ff), 655 iterations; if a
   MODULE_DEVICE_TABLE(input, ...) populates keybit[] densely, the
   emission reaches ~3132 bytes — overflowing the 256-byte buffer by
   about 12x. include/linux/mod_devicetable.h declares storage for the
   full bit range ("keybit[INPUT_DEVICE_ID_KEY_MAX / BITS_PER_LONG + 1]"),
   so the worst case is reachable per the ABI.

2. do_dmi_entry() emits one ":<prefix>*<filtered_substr>*" segment per
   matched DMI field, up to 4 matches per dmi_system_id. Each substr
   is sized as char[79] in struct dmi_strmatch (mod_devicetable.h:584),
   and dmi_ascii_filter() copies it verbatim into the alias buffer
   without bounds. Worst case: 4 × (1 + 3 + 1 + 79 + 1) = 336 bytes
   into alias[256], an 80-byte overflow.

No driver in the current tree triggers either case — every in-tree
INPUT_DEVICE_ID_MATCH_KEYBIT user populates keybit[] very sparsely
(1-3 bits), and no in-tree dmi_system_id has four maximally-long
matches. The concern is defense-in-depth: both unbounded sprintf
chains are silent stack-corruption primitives in a host build tool,
and the buffer sizes have not been revisited since the corresponding
code was first introduced.

The other do_*_entry() handlers in this file (do_usb_entry,
do_cpu_entry, do_typec_entry, ...) were audited and are bounded by
their input field sizes (uint16 IDs, fixed-length keys); their alias
buffers do not need this treatment.

Reproduced under AddressSanitizer with a stand-alone harness mirroring
do_input on a fully-populated keybit:

  ==18319==ERROR: AddressSanitizer: stack-buffer-overflow
  WRITE of size 2 at offset 288 in frame [32, 288) 'alias'
    #6 do_input poc.c:44

  Stack-canary build:
  Abort trap: 6  (strlen(alias)=3134, cap was 256-1)

Add a small alias_append() helper around vsnprintf with a remaining-
space check and call fatal() on overflow, matching the modpost style
for unrecoverable build conditions. do_input() takes the buffer size
as a new parameter; do_input_entry() and do_dmi_entry() pass
sizeof(alias) at every call site. dmi_ascii_filter() takes the
remaining buffer size as well and aborts on truncation. This bounds
every write into the on-stack buffers and turns the latent overflow
into a clean build error if it is ever reached.

Fixes: 1d8f430c15b3 ("[PATCH] Input: add modalias support")
Reviewed-by: Randy Dunlap <rdunlap@infradead.org>
Tested-by: Randy Dunlap <rdunlap@infradead.org>
Signed-off-by: Hasan Basbunar <basbunarhasan@gmail.com>
---
v1: https://lore.kernel.org/lkml/20260427204255.22117-1-basbunarhasan@gmail.com/
v2: https://lore.kernel.org/lkml/20260428062912.32918-1-basbunarhasan@gmail.com/

Changes since v2 (per Nicolas Schier's review):
- Dropped the alias_append() comment block. The rationale (buffer
  sizes, worst-case figures) lives in the commit message, where it
  cannot drift from the source if alias[] sizes are revisited later.
  The function name, signature, and fatal() messages already make
  the intent explicit at the failure site.

The change vs v2 is comment-only; the helper body, all call sites,
and the dmi_ascii_filter() bounds are unchanged from v2. Randy
Dunlap's Reviewed-by/Tested-by from v2 are carried forward
accordingly.

---
 scripts/mod/file2alias.c | 79 +++++++++++++++++++++++++++++++----------------
 1 file changed, 53 insertions(+), 26 deletions(-)

diff --git a/scripts/mod/file2alias.c b/scripts/mod/file2alias.c
index 4e99393a35f1..a7c2b4f8d6e9 100644
--- a/scripts/mod/file2alias.c
+++ b/scripts/mod/file2alias.c
@@ -651,7 +651,26 @@ static void do_vio_entry(struct module *mod, void *symval)
 	module_alias_printf(mod, true, "%s", alias);
 }
 
-static void do_input(char *alias,
+static void __attribute__((format(printf, 3, 4)))
+alias_append(char *alias, size_t size, const char *fmt, ...)
+{
+	size_t len = strlen(alias);
+	va_list args;
+	int n;
+
+	if (len >= size)
+		fatal("alias buffer (%zu) overflow before append\n", size);
+
+	va_start(args, fmt);
+	n = vsnprintf(alias + len, size - len, fmt, args);
+	va_end(args);
+
+	if (n < 0 || (size_t)n >= size - len)
+		fatal("alias buffer (%zu) overflow on append (need %d, have %zu)\n",
+		      size, n, size - len);
+}
+
+static void do_input(char *alias, size_t size,
 		     kernel_ulong_t *arr, unsigned int min, unsigned int max)
 {
 	unsigned int i;
@@ -659,13 +678,14 @@ static void do_input(char *alias,
 	for (i = min; i <= max; i++)
 		if (get_unaligned_native(arr + i / BITS_PER_LONG) &
 		    (1ULL << (i % BITS_PER_LONG)))
-			sprintf(alias + strlen(alias), "%X,*", i);
+			alias_append(alias, size, "%X,*", i);
 }
 
 /* input:b0v0p0e0-eXkXrXaXmXlXsXfXwX where X is comma-separated %02X. */
 static void do_input_entry(struct module *mod, void *symval)
 {
 	char alias[256] = {};
+	const size_t sizeof_alias = sizeof(alias);
 
 	DEF_FIELD(symval, input_device_id, flags);
 	DEF_FIELD(symval, input_device_id, bustype);
@@ -687,35 +707,35 @@ static void do_input_entry(struct module *mod, void *symval)
 	ADD(alias, "p", flags & INPUT_DEVICE_ID_MATCH_PRODUCT, product);
 	ADD(alias, "e", flags & INPUT_DEVICE_ID_MATCH_VERSION, version);
 
-	sprintf(alias + strlen(alias), "-e*");
+	alias_append(alias, sizeof_alias, "-e*");
 	if (flags & INPUT_DEVICE_ID_MATCH_EVBIT)
-		do_input(alias, *evbit, 0, INPUT_DEVICE_ID_EV_MAX);
-	sprintf(alias + strlen(alias), "k*");
+		do_input(alias, sizeof_alias, *evbit, 0, INPUT_DEVICE_ID_EV_MAX);
+	alias_append(alias, sizeof_alias, "k*");
 	if (flags & INPUT_DEVICE_ID_MATCH_KEYBIT)
-		do_input(alias, *keybit,
+		do_input(alias, sizeof_alias, *keybit,
 			 INPUT_DEVICE_ID_KEY_MIN_INTERESTING,
 			 INPUT_DEVICE_ID_KEY_MAX);
-	sprintf(alias + strlen(alias), "r*");
+	alias_append(alias, sizeof_alias, "r*");
 	if (flags & INPUT_DEVICE_ID_MATCH_RELBIT)
-		do_input(alias, *relbit, 0, INPUT_DEVICE_ID_REL_MAX);
-	sprintf(alias + strlen(alias), "a*");
+		do_input(alias, sizeof_alias, *relbit, 0, INPUT_DEVICE_ID_REL_MAX);
+	alias_append(alias, sizeof_alias, "a*");
 	if (flags & INPUT_DEVICE_ID_MATCH_ABSBIT)
-		do_input(alias, *absbit, 0, INPUT_DEVICE_ID_ABS_MAX);
-	sprintf(alias + strlen(alias), "m*");
+		do_input(alias, sizeof_alias, *absbit, 0, INPUT_DEVICE_ID_ABS_MAX);
+	alias_append(alias, sizeof_alias, "m*");
 	if (flags & INPUT_DEVICE_ID_MATCH_MSCIT)
-		do_input(alias, *mscbit, 0, INPUT_DEVICE_ID_MSC_MAX);
-	sprintf(alias + strlen(alias), "l*");
+		do_input(alias, sizeof_alias, *mscbit, 0, INPUT_DEVICE_ID_MSC_MAX);
+	alias_append(alias, sizeof_alias, "l*");
 	if (flags & INPUT_DEVICE_ID_MATCH_LEDBIT)
-		do_input(alias, *ledbit, 0, INPUT_DEVICE_ID_LED_MAX);
-	sprintf(alias + strlen(alias), "s*");
+		do_input(alias, sizeof_alias, *ledbit, 0, INPUT_DEVICE_ID_LED_MAX);
+	alias_append(alias, sizeof_alias, "s*");
 	if (flags & INPUT_DEVICE_ID_MATCH_SNDBIT)
-		do_input(alias, *sndbit, 0, INPUT_DEVICE_ID_SND_MAX);
-	sprintf(alias + strlen(alias), "f*");
+		do_input(alias, sizeof_alias, *sndbit, 0, INPUT_DEVICE_ID_SND_MAX);
+	alias_append(alias, sizeof_alias, "f*");
 	if (flags & INPUT_DEVICE_ID_MATCH_FFBIT)
-		do_input(alias, *ffbit, 0, INPUT_DEVICE_ID_FF_MAX);
-	sprintf(alias + strlen(alias), "w*");
+		do_input(alias, sizeof_alias, *ffbit, 0, INPUT_DEVICE_ID_FF_MAX);
+	alias_append(alias, sizeof_alias, "w*");
 	if (flags & INPUT_DEVICE_ID_MATCH_SWBIT)
-		do_input(alias, *swbit, 0, INPUT_DEVICE_ID_SW_MAX);
+		do_input(alias, sizeof_alias, *swbit, 0, INPUT_DEVICE_ID_SW_MAX);
 
 	module_alias_printf(mod, false, "input:%s", alias);
 }
@@ -895,12 +915,16 @@ static const struct dmifield {
 	{ NULL,  DMI_NONE }
 };
 
-static void dmi_ascii_filter(char *d, const char *s)
+static void dmi_ascii_filter(char *d, size_t avail, const char *s)
 {
 	/* Filter out characters we don't want to see in the modalias string */
 	for (; *s; s++)
-		if (*s > ' ' && *s < 127 && *s != ':')
+		if (*s > ' ' && *s < 127 && *s != ':') {
+			if (avail <= 1)
+				fatal("%s: alias buffer overflow\n", __func__);
 			*(d++) = *s;
+			avail--;
+		}
 
 	*d = 0;
 }
@@ -909,6 +933,8 @@ static void dmi_ascii_filter(char *d, const char *s)
 static void do_dmi_entry(struct module *mod, void *symval)
 {
 	char alias[256] = {};
+	const size_t sizeof_alias = sizeof(alias);
+	size_t len;
 	int i, j;
 	DEF_FIELD_ADDR(symval, dmi_system_id, matches);
 
@@ -916,11 +942,12 @@ static void do_dmi_entry(struct module *mod, void *symval)
 		for (j = 0; j < 4; j++) {
 			if ((*matches)[j].slot &&
 			    (*matches)[j].slot == dmi_fields[i].field) {
-				sprintf(alias + strlen(alias), ":%s*",
-					dmi_fields[i].prefix);
-				dmi_ascii_filter(alias + strlen(alias),
+				alias_append(alias, sizeof_alias, ":%s*",
+					     dmi_fields[i].prefix);
+				len = strlen(alias);
+				dmi_ascii_filter(alias + len, sizeof_alias - len,
 						 (*matches)[j].substr);
-				strcat(alias, "*");
+				alias_append(alias, sizeof_alias, "*");
 			}
 		}
 	}
-- 
2.53.0