Re: [PATCH net-next 4/9] sit: annotate data-races around tunnel->fwmark
From: netdev-bot+sashiko@kernel.org
Date: 2026-09-08 11:00:56
Thank you for your contribution! Sashiko AI review found 1 potential issue(s) to consider: Critical: 0 · High: 0 · Medium: 0 · Low: 1 - [Low] Incomplete fwmark annotation for sit devices: ipip6_tunnel_update() now publishes tunnel->fwmark with WRITE_ONCE()… -- sit: annotate data-races around tunnel->fwmark This adds READ_ONCE()/WRITE_ONCE() annotations around tunnel->fwmark in the sit driver, since ipip6_tunnel_xmit() runs locklessly with dev->lltx == true. It is preparation for running ipip6_fill_info() without RTNL.
quoted hunk ↗ jump to hunk
diff --git a/net/ipv6/sit.c b/net/ipv6/sit.c index 9a2bbd76d3794..e85fa80e80d1a 100644 --- a/net/ipv6/sit.c +++ b/net/ipv6/sit.c@@ -929,7 +929,7 @@ static netdev_tx_t ipip6_tunnel_xmit(struct sk_buff *skb, if (!dst && !ipip6_tunnel_dst_find(skb, &dst, false)) goto tx_error; - flowi4_init_output(&fl4, tunnel->parms.link, tunnel->fwmark, + flowi4_init_output(&fl4, tunnel->parms.link, READ_ONCE(tunnel->fwmark), tos & INET_DSCP_MASK, RT_SCOPE_UNIVERSE, IPPROTO_IPV6, 0, dst, tiph->saddr, 0, 0, sock_net_uid(tunnel->net, NULL));
[ ... ]
quoted hunk ↗ jump to hunk
@@ -1153,7 +1153,7 @@ static void ipip6_tunnel_update(struct ip_tunnel *t, t->parms.iph.frag_off = p->iph.frag_off; if (t->parms.link != p->link || t->fwmark != fwmark) { t->parms.link = p->link; - t->fwmark = fwmark; + WRITE_ONCE(t->fwmark, fwmark);
[Severity: Low]
Should the other lockless transmit path of the same sit device also be
annotated?
ipip6_tunnel_xmit() is only one of the two xmit paths. For IPv4 and MPLS
payloads, sit_tunnel_xmit() dispatches elsewhere:
net/ipv6/sit.c:sit_tunnel_xmit() {
switch (skb->protocol) {
case htons(ETH_P_IP):
sit_tunnel_xmit__(skb, dev, IPPROTO_IPIP);
break;
case htons(ETH_P_IPV6):
ipip6_tunnel_xmit(skb, dev);
break;
#if IS_ENABLED(CONFIG_MPLS)
case htons(ETH_P_MPLS_UC):
sit_tunnel_xmit__(skb, dev, IPPROTO_MPLS);
break;
#endif
...
}
and sit_tunnel_xmit__() -> ip_tunnel_xmit() reads the same field with a
plain load, right next to an already annotated read:
net/ipv4/ip_tunnel.c:ip_tunnel_xmit() {
...
ip_tunnel_init_flow(&fl4, protocol, dst, tnl_params->saddr,
tunnel->parms.o_key, tos & INET_DSCP_MASK,
tunnel->net, READ_ONCE(tunnel->parms.link),
tunnel->fwmark, skb_get_hash(skb), 0);
...
}
So for a sit tunnel carrying IPv4 or MPLS frames, an RTNL writer running
ipip6_changelink()/ioctl -> ipip6_tunnel_update() -> WRITE_ONCE(t->fwmark,
fwmark) still races with an unmarked read, and KCSAN can still report it.
The field is a naturally aligned u32, so the read cannot tear; the visible
effects are the KCSAN report plus one route lookup using the pre-update
mark.
Note that the generic writer in net/ipv4/ip_tunnel.c:ip_tunnel_update() is
also still a plain store to t->fwmark, so closing this would mean touching
the generic ip_tunnel code as well, which may be outside the intended scope
of this series. It was checked at the end of the series and the plain read
in ip_tunnel_xmit() is still there.
[ ... ]
--
Sashiko AI review · https://netdev-ai.bots.linux.dev/sashiko/#/patchset/20260907075846.2913645-1-edumazet%40google.com