List iterator used after loop in sja1105_insert_gate_entry?

[Maoyi Xie]

Hi all,

I came across what looks like an iterator used after the loop
ends in drivers/net/dsa/sja1105/sja1105_vl.c (linux-7.1-rc1),
in sja1105_insert_gate_entry(). I would appreciate your input
on whether this is worth fixing or whether I am misreading the
pattern.

  list_for_each_entry(p, &gating_cfg->entries, list) {
      if (p->interval == e->interval) {
          NL_SET_ERR_MSG_MOD(extack, "Gate conflict");
          rc = -EBUSY;
          goto err;
      }
      if (e->interval < p->interval)
          break;
  }
  list_add(&e->list, p->list.prev);

When the loop walks the whole list without break (the new
interval is larger than every existing one), `p` walks past the
end of the list and `p->list` aliases the list head via
container_of() offset cancellation. `p->list.prev` then resolves
to head->prev, which is the real tail entry's list, and the
list_add() inserts the new entry at the tail. That is the
intended behaviour for the case where the loop falls through.
The dereference of the iterator after the loop ends is the part
I am unsure about. Same shape as the Koschel cleanups from 2022
(99d8ae4ec8a tracing, 2966a9918df clockevents, dc1acd5c946 dlm,
and others) and the "controlled container confusion" pattern
described in [1].

I drafted a candidate fix that initialises an explicit
`insert_after` pointer to gating_cfg->entries.prev (the list
tail) and overwrites it to p->list.prev only on early break,
then passes insert_after to list_add(). The iterator is no
longer dereferenced after the loop and the diff is 5+/2-.

I built sja1105_vl.o on x86_64 with NET_DSA_SJA1105 +
NET_DSA_SJA1105_TAS + NET_DSA_SJA1105_VL at W=1 and the object
compiles clean with no warnings. I also ran a small userspace
mock of the two versions across seven scenarios: single entry
above and below the new interval, single entry conflict,
multi-entry insert at head, middle, and tail (fall through),
and a multi-entry mid-list conflict. The final list ordering
and the return code match in every case.

Does this look like something worth a [PATCH]? Happy to send
one if so, or to drop it if the shape here is fine.

Thanks,
Maoyi
https://maoyixie.com/

[1] Jakob Koschel et al., "UNCONTAINED: Uncovering Container
    Confusion in the Linux Kernel", USENIX Security 2023.
    https://www.usenix.org/conference/usenixsecurity23/presentation/koschel