Re: ALSA: iterator used after loop end in timer/seq port registration?

[Takashi Iwai <tiwai@suse.de>]

On Mon, 18 May 2026 18:00:29 +0200,
Maoyi Xie wrote:
> 
> Hi all,
> 
> While reading sound/core/ I noticed two places where the list
> iterator is used after the loop has walked past the end of the
> list. I would appreciate it if you could take a look and let me
> know whether these are real issues, and whether they are worth
> fixing.
> 
> The first is snd_timer_dev_register() in sound/core/timer.c
> (linux-7.1-rc1, around line 1018):
> 
>     list_for_each_entry(timer1, &snd_timer_list, device_list) {
>             if (timer1->tmr_class > timer->tmr_class)
>                     break;
>             ...
>             return -EBUSY;
>     }
>     list_add_tail(&timer->device_list, &timer1->device_list);
> 
> The second is snd_seq_create_port() in sound/core/seq/seq_ports.c
> (linux-7.1-rc1, around line 147):
> 
>     list_for_each_entry(p, &client->ports_list_head, list) {
>             ...
>             if (p->addr.port > num)
>                     break;
>             ...
>     }
>     list_add_tail(&new_port->list, &p->list);
> 
> In both cases, when the loop walks all entries without break, the
> iterator has gone one step past the last entry. &iter->member
> then aliases the list head via container_of offset cancellation,
> so the insert lands at the list tail. That is the intended
> behaviour, but the access is undefined per C11.
> 
> Jakob Koschel cleaned up many such sites in 2022, for example
> commits 99d8ae4ec8a (tracing: Remove usage of list iterator
> variable after the loop), 2966a9918df (clockevents: Use dedicated
> list iterator variable) and dc1acd5c946 (dlm: replace usage of
> found with dedicated list iterator variable). The two sites in
> sound/core/ were not covered.
> 
> A candidate fix would track an explicit insert_before pointer
> initialised to the list head and overwritten to &iter->member only
> when the loop breaks early. The observable behaviour is unchanged.
> 
> If this is intentional or already known, please disregard.
> Otherwise, I am happy to send a [PATCH] or to leave the fix to
> you. Thank you for your time, and sorry for the noise if this is
> not actually worth fixing or has already been spotted.

Thanks for the report.  I believe those two are actually bugs,
likely introduced by the early commit 9244b2c3079f
    [ALSA] alsa core: convert to list_for_each_entry*

The original code were with list_for_each(), and at the end of the
loop, it was supposed to go back to the list head.

If you have already fix patches ready for submission, it'd be
appreciated.  Otherwise I'll fix them up quickly, too.


Takashi