Skip to content

Commit 49f8fcd

Browse files
hasso5703Nicolas Schier
hasso5703
authored and
Nicolas Schier
committed
modpost: prevent stack buffer overflow in do_input_entry() and do_dmi_entry()
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: 1d8f430 ("[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> Link: https://patch.msgid.link/20260505161102.44087-1-basbunarhasan@gmail.com Signed-off-by: Nicolas Schier <nsc@kernel.org>
1 parent 5200f5f commit 49f8fcd

1 file changed

Lines changed: 53 additions & 26 deletions

File tree

‎scripts/mod/file2alias.c‎

Lines changed: 53 additions & 26 deletions
Original file line numberDiff line numberDiff line change
@@ -651,21 +651,41 @@ static void do_vio_entry(struct module *mod, void *symval)
651651
module_alias_printf(mod, true, "%s", alias);
652652
}
653653

654-
static void do_input(char *alias,
654+
static void __attribute__((format(printf, 3, 4)))
655+
alias_append(char *alias, size_t size, const char *fmt, ...)
656+
{
657+
size_t len = strlen(alias);
658+
va_list args;
659+
int n;
660+
661+
if (len >= size)
662+
fatal("alias buffer (%zu) overflow before append\n", size);
663+
664+
va_start(args, fmt);
665+
n = vsnprintf(alias + len, size - len, fmt, args);
666+
va_end(args);
667+
668+
if (n < 0 || (size_t)n >= size - len)
669+
fatal("alias buffer (%zu) overflow on append (need %d, have %zu)\n",
670+
size, n, size - len);
671+
}
672+
673+
static void do_input(char *alias, size_t size,
655674
kernel_ulong_t *arr, unsigned int min, unsigned int max)
656675
{
657676
unsigned int i;
658677

659678
for (i = min; i <= max; i++)
660679
if (get_unaligned_native(arr + i / BITS_PER_LONG) &
661680
(1ULL << (i % BITS_PER_LONG)))
662-
sprintf(alias + strlen(alias), "%X,*", i);
681+
alias_append(alias, size, "%X,*", i);
663682
}
664683

665684
/* input:b0v0p0e0-eXkXrXaXmXlXsXfXwX where X is comma-separated %02X. */
666685
static void do_input_entry(struct module *mod, void *symval)
667686
{
668687
char alias[256] = {};
688+
const size_t sizeof_alias = sizeof(alias);
669689

670690
DEF_FIELD(symval, input_device_id, flags);
671691
DEF_FIELD(symval, input_device_id, bustype);
@@ -687,35 +707,35 @@ static void do_input_entry(struct module *mod, void *symval)
687707
ADD(alias, "p", flags & INPUT_DEVICE_ID_MATCH_PRODUCT, product);
688708
ADD(alias, "e", flags & INPUT_DEVICE_ID_MATCH_VERSION, version);
689709

690-
sprintf(alias + strlen(alias), "-e*");
710+
alias_append(alias, sizeof_alias, "-e*");
691711
if (flags & INPUT_DEVICE_ID_MATCH_EVBIT)
692-
do_input(alias, *evbit, 0, INPUT_DEVICE_ID_EV_MAX);
693-
sprintf(alias + strlen(alias), "k*");
712+
do_input(alias, sizeof_alias, *evbit, 0, INPUT_DEVICE_ID_EV_MAX);
713+
alias_append(alias, sizeof_alias, "k*");
694714
if (flags & INPUT_DEVICE_ID_MATCH_KEYBIT)
695-
do_input(alias, *keybit,
715+
do_input(alias, sizeof_alias, *keybit,
696716
INPUT_DEVICE_ID_KEY_MIN_INTERESTING,
697717
INPUT_DEVICE_ID_KEY_MAX);
698-
sprintf(alias + strlen(alias), "r*");
718+
alias_append(alias, sizeof_alias, "r*");
699719
if (flags & INPUT_DEVICE_ID_MATCH_RELBIT)
700-
do_input(alias, *relbit, 0, INPUT_DEVICE_ID_REL_MAX);
701-
sprintf(alias + strlen(alias), "a*");
720+
do_input(alias, sizeof_alias, *relbit, 0, INPUT_DEVICE_ID_REL_MAX);
721+
alias_append(alias, sizeof_alias, "a*");
702722
if (flags & INPUT_DEVICE_ID_MATCH_ABSBIT)
703-
do_input(alias, *absbit, 0, INPUT_DEVICE_ID_ABS_MAX);
704-
sprintf(alias + strlen(alias), "m*");
723+
do_input(alias, sizeof_alias, *absbit, 0, INPUT_DEVICE_ID_ABS_MAX);
724+
alias_append(alias, sizeof_alias, "m*");
705725
if (flags & INPUT_DEVICE_ID_MATCH_MSCIT)
706-
do_input(alias, *mscbit, 0, INPUT_DEVICE_ID_MSC_MAX);
707-
sprintf(alias + strlen(alias), "l*");
726+
do_input(alias, sizeof_alias, *mscbit, 0, INPUT_DEVICE_ID_MSC_MAX);
727+
alias_append(alias, sizeof_alias, "l*");
708728
if (flags & INPUT_DEVICE_ID_MATCH_LEDBIT)
709-
do_input(alias, *ledbit, 0, INPUT_DEVICE_ID_LED_MAX);
710-
sprintf(alias + strlen(alias), "s*");
729+
do_input(alias, sizeof_alias, *ledbit, 0, INPUT_DEVICE_ID_LED_MAX);
730+
alias_append(alias, sizeof_alias, "s*");
711731
if (flags & INPUT_DEVICE_ID_MATCH_SNDBIT)
712-
do_input(alias, *sndbit, 0, INPUT_DEVICE_ID_SND_MAX);
713-
sprintf(alias + strlen(alias), "f*");
732+
do_input(alias, sizeof_alias, *sndbit, 0, INPUT_DEVICE_ID_SND_MAX);
733+
alias_append(alias, sizeof_alias, "f*");
714734
if (flags & INPUT_DEVICE_ID_MATCH_FFBIT)
715-
do_input(alias, *ffbit, 0, INPUT_DEVICE_ID_FF_MAX);
716-
sprintf(alias + strlen(alias), "w*");
735+
do_input(alias, sizeof_alias, *ffbit, 0, INPUT_DEVICE_ID_FF_MAX);
736+
alias_append(alias, sizeof_alias, "w*");
717737
if (flags & INPUT_DEVICE_ID_MATCH_SWBIT)
718-
do_input(alias, *swbit, 0, INPUT_DEVICE_ID_SW_MAX);
738+
do_input(alias, sizeof_alias, *swbit, 0, INPUT_DEVICE_ID_SW_MAX);
719739

720740
module_alias_printf(mod, false, "input:%s", alias);
721741
}
@@ -895,12 +915,16 @@ static const struct dmifield {
895915
{ NULL, DMI_NONE }
896916
};
897917

898-
static void dmi_ascii_filter(char *d, const char *s)
918+
static void dmi_ascii_filter(char *d, size_t avail, const char *s)
899919
{
900920
/* Filter out characters we don't want to see in the modalias string */
901921
for (; *s; s++)
902-
if (*s > ' ' && *s < 127 && *s != ':')
922+
if (*s > ' ' && *s < 127 && *s != ':') {
923+
if (avail <= 1)
924+
fatal("%s: alias buffer overflow\n", __func__);
903925
*(d++) = *s;
926+
avail--;
927+
}
904928

905929
*d = 0;
906930
}
@@ -909,18 +933,21 @@ static void dmi_ascii_filter(char *d, const char *s)
909933
static void do_dmi_entry(struct module *mod, void *symval)
910934
{
911935
char alias[256] = {};
936+
const size_t sizeof_alias = sizeof(alias);
937+
size_t len;
912938
int i, j;
913939
DEF_FIELD_ADDR(symval, dmi_system_id, matches);
914940

915941
for (i = 0; i < ARRAY_SIZE(dmi_fields); i++) {
916942
for (j = 0; j < 4; j++) {
917943
if ((*matches)[j].slot &&
918944
(*matches)[j].slot == dmi_fields[i].field) {
919-
sprintf(alias + strlen(alias), ":%s*",
920-
dmi_fields[i].prefix);
921-
dmi_ascii_filter(alias + strlen(alias),
945+
alias_append(alias, sizeof_alias, ":%s*",
946+
dmi_fields[i].prefix);
947+
len = strlen(alias);
948+
dmi_ascii_filter(alias + len, sizeof_alias - len,
922949
(*matches)[j].substr);
923-
strcat(alias, "*");
950+
alias_append(alias, sizeof_alias, "*");
924951
}
925952
}
926953
}

0 commit comments

Comments
 (0)