drm/nouveau/tmr: iterator used after loop end in nvkm_timer_alarm?

[Maoyi Xie]

Hi all,

While reading drivers/gpu/drm/nouveau/nvkm/subdev/timer/base.c
I noticed something that looks like a past the end iterator
pattern. I would appreciate it if you could take a look and
let me know whether this is a real issue, and whether it is
worth fixing.

The site is nvkm_timer_alarm() (linux-7.1-rc1, around line 122):

    list_for_each_entry(list, &tmr->alarms, head) {
            if (list->timestamp > alarm->timestamp)
                    break;
    }
    list_add_tail(&alarm->head, &list->head);

When the loop walks all entries without break, list has gone
one step past the last entry. &list->head then aliases
&tmr->alarms (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). This site in
nvkm/subdev/timer was not covered.

A candidate fix would track an explicit insert_before pointer
initialised to the list head (&tmr->alarms) and overwritten to
&list->head 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,
Maoyi Xie
https://maoyixie.com/