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

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

[Ziqing Chen]

snd_ctl_elem_init_enum_names() advances pointer p through the names
buffer while decrementing buf_len. If buf_len reaches zero but items
remain, the next iteration calls strnlen(p, 0).

While strnlen(p, 0) returns 0 and would hit the existing name_len == 0
error path, CONFIG_FORTIFY_SOURCE's fortified strnlen() first checks
maxlen against __builtin_dynamic_object_size(). When Clang loses track
of p's object size inside the loop, this triggers a BRK exception panic
before the return value is examined.

Add a buf_len == 0 guard at the loop entry to prevent calling fortified
strnlen() on an exhausted buffer.

Found by kernel fuzz testing through Xiaomi Smartphone.

Fixes: 8d448162bda5 ("ALSA: control: add support for ENUMERATED user space controls")
Cc: stable@vger.kernel.org
Signed-off-by: Ziqing Chen <chenziqing@xiaomi.com>
---
 sound/core/control.c | 4 ++++
 1 file changed, 4 insertions(+)

diff --git a/sound/core/control.c b/sound/core/control.c
index 0ddade871b52..6ceb5f977fcd 100644
--- a/sound/core/control.c
+++ b/sound/core/control.c
@@ -1574,6 +1574,10 @@ static int snd_ctl_elem_init_enum_names(struct user_element *ue)
        /* check that there are enough valid names */
        p = names;
        for (i = 0; i < ue->info.value.enumerated.items; ++i) {
+               if (buf_len == 0) {
+                       kvfree(names);
+                       return -EINVAL;
+               }
                name_len = strnlen(p, buf_len);
                if (name_len == 0 || name_len >= 64 || name_len == buf_len) {
                        kvfree(names);
--
2.52.0

#/******±¾Óʼþ¼°Æ丽¼þº¬ÓÐСÃ×¹«Ë¾µÄ±£ÃÜÐÅÏ¢£¬½öÏÞÓÚ·¢Ë͸øÉÏÃæµØÖ·ÖÐÁгöµÄ¸öÈË»òȺ×é¡£½ûÖ¹ÈκÎÆäËûÈËÒÔÈκÎÐÎʽʹÓ㨰üÀ¨µ«²»ÏÞÓÚÈ«²¿»ò²¿·ÖµØй¶¡¢¸´ÖÆ¡¢»òÉ¢·¢£©±¾ÓʼþÖеÄÐÅÏ¢¡£Èç¹ûÄú´íÊÕÁ˱¾Óʼþ£¬ÇëÄúÁ¢¼´µç»°»òÓʼþ֪ͨ·¢¼þÈ˲¢É¾³ý±¾Óʼþ£¡ This e-mail and its attachments contain confidential information from XIAOMI, which is intended only for the person or entity whose address is listed above. Any use of the information contained herein in any way (including, but not limited to, total or partial disclosure, reproduction, or dissemination) by persons other than the intended recipient(s) is prohibited. If you receive this e-mail in error, please notify the sender by phone or email immediately and delete it!******/#

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

[Takashi Iwai]

On Tue, 14 Apr 2026 11:05:42 +0200,
Ziqing Chen wrote:
> 
> snd_ctl_elem_init_enum_names() advances pointer p through the names
> buffer while decrementing buf_len. If buf_len reaches zero but items
> remain, the next iteration calls strnlen(p, 0).
> 
> While strnlen(p, 0) returns 0 and would hit the existing name_len == 0
> error path, CONFIG_FORTIFY_SOURCE's fortified strnlen() first checks
> maxlen against __builtin_dynamic_object_size(). When Clang loses track
> of p's object size inside the loop, this triggers a BRK exception panic
> before the return value is examined.
> 
> Add a buf_len == 0 guard at the loop entry to prevent calling fortified
> strnlen() on an exhausted buffer.
> 
> Found by kernel fuzz testing through Xiaomi Smartphone.
> 
> Fixes: 8d448162bda5 ("ALSA: control: add support for ENUMERATED user space controls")
> Cc: stable@vger.kernel.org
> Signed-off-by: Ziqing Chen <chenziqing@xiaomi.com>
> ---
>  sound/core/control.c | 4 ++++
>  1 file changed, 4 insertions(+)
> 
> diff --git a/sound/core/control.c b/sound/core/control.c
> index 0ddade871b52..6ceb5f977fcd 100644
> --- a/sound/core/control.c
> +++ b/sound/core/control.c
> @@ -1574,6 +1574,10 @@ static int snd_ctl_elem_init_enum_names(struct user_element *ue)
>         /* check that there are enough valid names */
>         p = names;
>         for (i = 0; i < ue->info.value.enumerated.items; ++i) {
> +               if (buf_len == 0) {
> +                       kvfree(names);
> +                       return -EINVAL;
> +               }
>                 name_len = strnlen(p, buf_len);
>                 if (name_len == 0 || name_len >= 64 || name_len == buf_len) {
>                         kvfree(names);

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.

Could you test it and resubmit if that works?


thanks,

Takashi

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

[Ziqing Chen]

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?

Thanks,
Ziqing
#/******本邮件及其附件含有小米公司的保密信息，仅限于发送给上面地址中列出的个人或群组。禁止任何其他人以任何形式使用（包括但不限于全部或部分地泄露、复制、或散发）本邮件中的信息。如果您错收了本邮件，请您立即电话或邮件通知发件人并删除本邮件！ This e-mail and its attachments contain confidential information from XIAOMI, which is intended only for the person or entity whose address is listed above. Any use of the information contained herein in any way (including, but not limited to, total or partial disclosure, reproduction, or dissemination) by persons other than the intended recipient(s) is prohibited. If you receive this e-mail in error, please notify the sender by phone or email immediately and delete it!******/#

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

[Takashi Iwai]

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

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

[Ziqing Chen]

snd_ctl_elem_init_enum_names() advances pointer p through the names
buffer while decrementing buf_len. If buf_len reaches zero but items
remain, the next iteration calls strnlen(p, 0).

While strnlen(p, 0) returns 0 and would hit the existing name_len == 0
error path, CONFIG_FORTIFY_SOURCE's fortified strnlen() first checks
maxlen against __builtin_dynamic_object_size(). When Clang loses track
of p's object size inside the loop, this triggers a BRK exception panic
before the return value is examined.

Add a buf_len == 0 guard at the loop entry to prevent calling fortified
strnlen() on an exhausted buffer.

Found by kernel fuzz testing through Xiaomi Smartphone.

Fixes: 8d448162bda5 ("ALSA: control: add support for ENUMERATED user space controls")
Cc: stable@vger.kernel.org
Signed-off-by: Ziqing Chen <chenziqing@xiaomi.com>
---
 sound/core/control.c | 4 ++++
 1 file changed, 4 insertions(+)

diff --git a/sound/core/control.c b/sound/core/control.c
index 0ddade871b52..6ceb5f977fcd 100644
--- a/sound/core/control.c
+++ b/sound/core/control.c
@@ -1574,6 +1574,10 @@ static int snd_ctl_elem_init_enum_names(struct user_element *ue)
        /* check that there are enough valid names */
        p = names;
        for (i = 0; i < ue->info.value.enumerated.items; ++i) {
+               if (buf_len == 0) {
+                       kvfree(names);
+                       return -EINVAL;
+               }
                name_len = strnlen(p, buf_len);
                if (name_len == 0 || name_len >= 64 || name_len == buf_len) {
                        kvfree(names);
--
2.52.0

#/******±¾Óʼþ¼°Æ丽¼þº¬ÓÐСÃ×¹«Ë¾µÄ±£ÃÜÐÅÏ¢£¬½öÏÞÓÚ·¢Ë͸øÉÏÃæµØÖ·ÖÐÁгöµÄ¸öÈË»òȺ×é¡£½ûÖ¹ÈκÎÆäËûÈËÒÔÈκÎÐÎʽʹÓ㨰üÀ¨µ«²»ÏÞÓÚÈ«²¿»ò²¿·ÖµØй¶¡¢¸´ÖÆ¡¢»òÉ¢·¢£©±¾ÓʼþÖеÄÐÅÏ¢¡£Èç¹ûÄú´íÊÕÁ˱¾Óʼþ£¬ÇëÄúÁ¢¼´µç»°»òÓʼþ֪ͨ·¢¼þÈ˲¢É¾³ý±¾Óʼþ£¡ This e-mail and its attachments contain confidential information from XIAOMI, which is intended only for the person or entity whose address is listed above. Any use of the information contained herein in any way (including, but not limited to, total or partial disclosure, reproduction, or dissemination) by persons other than the intended recipient(s) is prohibited. If you receive this e-mail in error, please notify the sender by phone or email immediately and delete it!******/#

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

[Ziqing Chen]

snd_ctl_elem_init_enum_names() advances pointer p through the names
buffer while decrementing buf_len. If buf_len reaches zero but items
remain, the next iteration calls strnlen(p, 0).

While strnlen(p, 0) returns 0 and would hit the existing name_len == 0
error path, CONFIG_FORTIFY_SOURCE's fortified strnlen() first checks
maxlen against __builtin_dynamic_object_size(). When Clang loses track
of p's object size inside the loop, this triggers a BRK exception panic
before the return value is examined.

Add a buf_len == 0 guard at the loop entry to prevent calling fortified
strnlen() on an exhausted buffer.

Found by kernel fuzz testing through Xiaomi Smartphone.

Fixes: 8d448162bda5 ("ALSA: control: add support for ENUMERATED user space controls")
Cc: stable@vger.kernel.org
Signed-off-by: Ziqing Chen <chenziqing@xiaomi.com>
---
 sound/core/control.c | 4 ++++
 1 file changed, 4 insertions(+)

diff --git a/sound/core/control.c b/sound/core/control.c
index 0ddade871b52..6ceb5f977fcd 100644
--- a/sound/core/control.c
+++ b/sound/core/control.c
@@ -1574,6 +1574,10 @@ static int snd_ctl_elem_init_enum_names(struct user_element *ue)
 	/* check that there are enough valid names */
 	p = names;
 	for (i = 0; i < ue->info.value.enumerated.items; ++i) {
+		if (buf_len == 0) {
+			kvfree(names);
+			return -EINVAL;
+		}
 		name_len = strnlen(p, buf_len);
 		if (name_len == 0 || name_len >= 64 || name_len == buf_len) {
 			kvfree(names);
-- 
2.52.0