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@xxxxxxxxx>

LGTM. Thanks.

Reviewed-by: Randy Dunlap <rdunlap@xxxxxxxxxxxxx>
Tested-by: Randy Dunlap <rdunlap@xxxxxxxxxxxxx>

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

> ---
> v1: https://lore.kernel.org/lkml/20260427204255.22117-1-basbunarhasan@xxxxxxxxx/
> 
> 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