From: Maoyi Xie <maoyixie.tju@gmail.com>
To: Nishanth Menon <nm@ti.com>
Cc: Siddharth Vadapalli <s-vadapalli@ti.com>,
Roger Quadros <rogerq@kernel.org>,
David Yang <mmyangfl@gmail.com>,
Andrew Lunn <andrew+netdev@lunn.ch>,
netdev@vger.kernel.org, linux-kernel@vger.kernel.org
Subject: List iterator used after loop in netcp_register_{tx,rx}hook?
Date: Fri, 22 May 2026 22:35:22 +0800 [thread overview]
Message-ID: <20260522143522.83473-1-maoyixie.tju@gmail.com> (raw)
Hi all,
I came across what looks like an iterator used after the loop
ends in drivers/net/ethernet/ti/netcp_core.c (linux-7.1-rc1),
in both netcp_register_txhook() and netcp_register_rxhook().
I would appreciate your input on whether this is worth fixing
or whether I am misreading the pattern.
list_for_each_entry(next, &netcp_priv->txhook_list_head, list) {
if (next->order > order)
break;
}
__list_add(&entry->list, next->list.prev, &next->list);
When the loop walks the whole list without break (every existing
hook has order <= the new one), `next` walks past the end of the
list and `&next->list` aliases the list head via container_of()
offset cancellation, so __list_add() resolves to inserting 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_before` pointer to the list head and overwrites it to
&next->list only on early break, then passes insert_before to
__list_add(). The iterator is no longer dereferenced after the
loop and the diff is 12+/4-, symmetric across tx and rx.
I built netcp_core.o for ARM with keystone_defconfig at W=1 and
the object compiles clean for our change. The W=1 warnings that
remain are in netcp_ndo_open() at lines 1601-1662 (snprintf
truncation) and are already present in the file, unrelated to
this. I also ran a small userspace mock of the two versions
across five scenarios: empty list, single entry above the new
order, single entry below the new order, multiple entries with
the new order in the middle, and multiple entries where the new
order falls through to the tail. The final list ordering matches
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
reply other threads:[~2026-05-22 14:35 UTC|newest]
Thread overview: [no followups] expand[flat|nested] mbox.gz Atom feed
Reply instructions:
You may reply publicly to this message via plain-text email
using any one of the following methods:
* Save the following mbox file, import it into your mail client,
and reply-to-all from there: mbox
Avoid top-posting and favor interleaved quoting:
https://en.wikipedia.org/wiki/Posting_style#Interleaved_style
* Reply using the --to, --cc, and --in-reply-to
switches of git-send-email(1):
git send-email \
--in-reply-to=20260522143522.83473-1-maoyixie.tju@gmail.com \
--to=maoyixie.tju@gmail.com \
--cc=andrew+netdev@lunn.ch \
--cc=linux-kernel@vger.kernel.org \
--cc=mmyangfl@gmail.com \
--cc=netdev@vger.kernel.org \
--cc=nm@ti.com \
--cc=rogerq@kernel.org \
--cc=s-vadapalli@ti.com \
/path/to/YOUR_REPLY
https://kernel.org/pub/software/scm/git/docs/git-send-email.html
* If your mail client supports setting the In-Reply-To header
via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line
before the message body.
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox
all inboxes | Powered by JetHome®