From: Hangbin Liu <redacted>
ioam6_do_inline() and ioam6_do_encap() stored the next header into
the shared ilwt->tuninfo template and then memcpy()ed it into the
skb. The lwtstate is used from multiple CPUs without locking, so
another packet can overwrite eh.nexthdr between the store and the
memcpy.
The packet can then leave with an HBH nexthdr that does not match its
payload (e.g. TCP labelled as UDP, or a local packet labelled
IPPROTO_IPV6). Set nexthdr on the per-packet Hop-by-Hop copy after
memcpy instead, leave the shared template read-only.
Mark the tuninfo argument const now that the shared template is no
longer modified, and drop the unnecessary (u8 *) cast in memcpy.
Fixes: 3edede08ff37 ("ipv6: ioam: Support for IOAM injection with lwtunnels")
Fixes: 8cb3bf8bff3c ("ipv6: ioam: Add support for the ip6ip6 encapsulation")
Reported-by: Sashiko <netdev-bot+sashiko@kernel.org>
Closes: https://lore.kernel.org/netdev/179336082844.3134.1265197633714491521@kernel.org (local)
Reviewed-by: Justin Iurman <justin.iurman@gmail.com>
Signed-off-by: Hangbin Liu <redacted>
---
The issue is reported by Sashiko. It was confirmed with concurrent TCP
and UDP traffic through a shared ioam6 inline route on a multi-CPU host.
Without the fix, a packet can leave with an HBH nexthdr that does not
match its payload (e.g. hbh->nexthdr=17 with a TCP segment after the HBH,
or hbh->nexthdr=6 with a UDP segment). With the fix, the same load reports
no HBH/L4 mismatch: nexthdr is written only on the per-packet Hop-by-Hop
header after memcpy, never on the shared tuninfo template.
---
Changes in v4:
- Use a temp variable for the ipv6_opt_hdr setting (Jakub Kicinski)
- Link to v3: https://patch.msgid.link/20261109-ipv6_ioam_lock-v3-1-a86195859d1e@kylinos.cn (local)
Changes in v3:
- make tuninfo as const (Justin Iurman)
- Link to v2: https://patch.msgid.link/20261105-ipv6_ioam_lock-v2-1-8f0ea30c4ffa@kylinos.cn (local)
Changes in v2:
- Add one missed Fixes tag. No code change.
- Link to v1: https://patch.msgid.link/20261104-ipv6_ioam_lock-v1-1-4f24c41b261b@kylinos.cn (local)
---
net/ipv6/ioam6_iptunnel.c | 16 ++++++++++------
1 file changed, 10 insertions(+), 6 deletions(-)
diff --git a/net/ipv6/ioam6_iptunnel.c b/net/ipv6/ioam6_iptunnel.c
index cfb2c41634a0..e8be0d922f74 100644
--- a/net/ipv6/ioam6_iptunnel.c
+++ b/net/ipv6/ioam6_iptunnel.c
@@ -254,10 +254,11 @@ static int ioam6_do_fill(struct net *net, struct sk_buff *skb)
}
static int ioam6_do_inline(struct net *net, struct sk_buff *skb,
- struct ioam6_lwt_encap *tuninfo,
+ const struct ioam6_lwt_encap *tuninfo,
struct dst_entry *cache_dst)
{
struct ipv6hdr *oldhdr, *hdr;
+ struct ipv6_opt_hdr *opt_hdr;
int hdrlen, err;
hdrlen = (tuninfo->eh.hdrlen + 1) << 3;@@ -276,12 +277,13 @@ static int ioam6_do_inline(struct net *net, struct sk_buff *skb,
hdr = ipv6_hdr(skb);
memmove(hdr, oldhdr, sizeof(*oldhdr));
- tuninfo->eh.nexthdr = hdr->nexthdr;
skb_set_transport_header(skb, sizeof(*hdr));
skb_postpush_rcsum(skb, hdr, sizeof(*hdr) + hdrlen);
- memcpy(skb_transport_header(skb), (u8 *)tuninfo, hdrlen);
+ opt_hdr = (struct ipv6_opt_hdr *)skb_transport_header(skb);
+ memcpy(opt_hdr, tuninfo, hdrlen);
+ opt_hdr->nexthdr = hdr->nexthdr;
hdr->nexthdr = NEXTHDR_HOP;
hdr->payload_len = cpu_to_be16(skb->len - sizeof(*hdr));
@@ -290,7 +292,7 @@ static int ioam6_do_inline(struct net *net, struct sk_buff *skb,
}
static int ioam6_do_encap(struct net *net, struct sk_buff *skb,
- struct ioam6_lwt_encap *tuninfo,
+ const struct ioam6_lwt_encap *tuninfo,
bool has_tunsrc,
struct in6_addr *tunsrc,
struct in6_addr *tundst,
@@ -298,6 +300,7 @@ static int ioam6_do_encap(struct net *net, struct sk_buff *skb,
{
struct dst_entry *dst = skb_dst(skb);
struct ipv6hdr *hdr, *inner_hdr;
+ struct ipv6_opt_hdr *opt_hdr;
int hdrlen, len, err;
hdrlen = (tuninfo->eh.hdrlen + 1) << 3;@@ -314,8 +317,9 @@ static int ioam6_do_encap(struct net *net, struct sk_buff *skb,
skb_mac_header_rebuild(skb);
skb_set_transport_header(skb, sizeof(*hdr));
- tuninfo->eh.nexthdr = NEXTHDR_IPV6;
- memcpy(skb_transport_header(skb), (u8 *)tuninfo, hdrlen);
+ opt_hdr = (struct ipv6_opt_hdr *)skb_transport_header(skb);
+ memcpy(opt_hdr, tuninfo, hdrlen);
+ opt_hdr->nexthdr = NEXTHDR_IPV6;
hdr = ipv6_hdr(skb);
memcpy(hdr, inner_hdr, sizeof(*hdr));
---
base-commit: 145480b28ca82e9fae789427cc4569e596515a32
change-id: 20261103-ipv6_ioam_lock-65102ca54908
Best regards,
--
Hangbin Liu [off-list ref]