acpi/riscv: iterator after loop end in riscv_acpi_register_ext_intc?

[Maoyi Xie]

Hi all,

While reading drivers/acpi/riscv/irq.c I noticed something
that looks like a past the end iterator pattern, used in two
places in the same function. 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 riscv_acpi_register_ext_intc() (linux-7.1-rc1,
around line 155):

    list_for_each_entry(node, &ext_intc_list, list) {
            if (node->gsi_base < ext_intc_element->gsi_base)
                    break;
    }

    /* Adjust the previous node's GSI range if that has
       pending registration */
    prev = list_prev_entry(node, list);
    if (!list_entry_is_head(prev, &ext_intc_list, list)) {
            if (prev->flag & RISCV_ACPI_INTC_FLAG_PENDING)
                    prev->nr_irqs = ext_intc_element->gsi_base
                                    - prev->gsi_base;
    }

    list_add_tail(&ext_intc_element->list, &node->list);

When the loop walks all entries without break (no entry has
gsi_base smaller than the new one), node is past the end.
The function then uses node in two ways:

  1. list_prev_entry(node, list) relies on &node->list
     aliasing &ext_intc_list (the head) via container_of
     offset cancellation, so the result becomes the last
     real entry of the list (head->prev). The PENDING flag
     adjustment then runs on that last entry.

  2. list_add_tail(..., &node->list) relies on the same
     alias so the insert lands at the list tail.

Both behaviours are intended (sorted insertion plus a
fall-through path that still wants the last entry's PENDING
flag adjusted), but both dereferences of the past the end
node are 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 was not covered.

A candidate fix would track an explicit insertion target
(struct list_head *pos = &ext_intc_list) which is overwritten
to &node->list only when the loop breaks early. The PENDING
flag adjustment then reads pos->prev directly: if pos->prev
is &ext_intc_list (empty list, or break on the first entry),
the adjustment is skipped; otherwise pos->prev points to the
real entry that becomes the predecessor in list order. In the
fall-through path pos stays at the list head, and head->prev
is the last real entry, so that branch keeps adjusting the
last entry as before. 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/