Re: [PATCH] ALSA: control: Validate buf_len before strnlen() in snd_ctl_elem_init_enum_names()

[Takashi Iwai <tiwai@suse.de>]

On Tue, 14 Apr 2026 12:10:12 +0200,
Ziqing Chen wrote:
> 
> On Tue, 15 Apr 2026, Takashi Iwai wrote:
> > Having a zero buf_len check is good, per se, but it doesn't have to be
> > at this late place.  It can be checked at the very beginning even
> > before the allocation (where we have already an upper bound check),
> > instead.
> 
> Thanks for the review.
> 
> The crash I hit is not caused by buf_len being zero at entry -- it
> occurs when buf_len is decremented to zero *during* the loop after
> successfully parsing earlier items. For example, a userspace caller
> can craft an ioctl payload where buf_len is just large enough for
> the first N-1 names but leaves exactly zero bytes for the Nth item.
> 
> An early check (buf_len == 0 before allocation) would catch the
> degenerate case where the caller passes a zero-length buffer, which
> is a good idea on its own, but it would not prevent the mid-loop
> exhaustion that triggers the FORTIFY_SOURCE panic.
> 
> The issue was originally caught during fuzz testing via ioctl on an
> arm64 (MT6993) device with CONFIG_FORTIFY_SOURCE and Clang. We have
> not been able to reproduce it since, which is consistent with it
> depending on Clang's __builtin_dynamic_object_size() heuristic for
> the loop-incremented pointer -- the evaluation can vary across
> compiler versions and optimization levels.
> 
> Here is the crash stack from the original hit:
> 
>   Unexpected kernel BRK exception at EL1
>   Internal error: BRK handler: 00000000f2000001 1 PREEMPT SMP
>   pc : snd_ctl_elem_init_enum_names+0x244/0x24c
>   lr : snd_ctl_elem_init_enum_names+0x244/0x24c
>   Call trace:
>    snd_ctl_elem_init_enum_names+0x244/0x24c
>    snd_ctl_elem_add+0x4bc/0x7b0
>    snd_ctl_elem_add_user+0x128/0x26c
>    snd_ctl_ioctl+0x9a4/0x1a68
>    __arm64_sys_ioctl+0x110/0x18c
>    invoke_syscall+0x9c/0x210
>    el0_svc_common+0xe4/0x1b0
>    do_el0_svc+0x34/0x44
>    el0_svc+0x38/0x84
>    el0t_64_sync_handler+0x70/0xbc
> 
> The BRK is from the fortified strnlen() -- when Clang loses track of
> the dynamic object size for the repeatedly incremented pointer p
> inside the loop, __builtin_dynamic_object_size() evaluates to 0,
> causing the FORTIFY check to fire before strnlen() even returns.
> 
> Would you prefer a v2 that adds both checks -- a buf_len == 0 guard
> at the function entry (next to the existing upper bound check) and
> the loop-level guard? Or do you think the loop check alone is
> sufficient?

OK, thanks for clarification, now it's clearer.

But your posted patch doesn't seem applicable because your mailer
broke tabs with spaces.  Could you try to fix your mailer setup and
resubmit?


thanks,

Takashi