Re: [PATCH net v3] ipv6: rpl: reserve mac_len headroom when recompressed SRH grows

Greg Kroah-Hartman
2026-05-02T08:43:40Z

On Sat, May 02, 2026 at 01:36:18AM -0700, Yuan Tan wrote:
> On Tue, Apr 21, 2026 at 6:16 AM Greg Kroah-Hartman
> <gregkh@linuxfoundation.org> wrote:
> >
> > ipv6_rpl_srh_rcv() decompresses an RFC 6554 Source Routing Header, swaps
> > the next segment into ipv6_hdr->daddr, recompresses, then pulls the old
> > header and pushes the new one plus the IPv6 header back. The
> > recompressed header can be larger than the received one when the swap
> > reduces the common-prefix length the segments share with daddr (CmprI=0,
> > CmprE>0, seg[0][0] != daddr[0] gives the maximum +8 bytes).
> >
> > pskb_expand_head() was gated on segments_left == 0, so on earlier
> > segments the push consumed unchecked headroom. Once skb_push() leaves
> > fewer than skb->mac_len bytes in front of data,
> > skb_mac_header_rebuild()'s call to:
> >
> > skb_set_mac_header(skb, -skb->mac_len);
> >
> > will store (data - head) - mac_len into the u16 mac_header field, which
> > wraps to ~65530, and the following memmove() writes mac_len bytes ~64KiB
> > past skb->head.
> >
> > A single AF_INET6/SOCK_RAW/IPV6_HDRINCL packet over lo with a two
> > segment type-3 SRH (CmprI=0, CmprE=15) reaches headroom 8 after one
> > pass; KASAN reports a 14-byte OOB write in ipv6_rthdr_rcv.
> >
> > Fix this by expanding the head whenever the remaining room is less than
> > the push size plus mac_len, and request that much extra so the rebuilt
> > MAC header fits afterwards.
> >
> > Fixes: 8610c7c6e3bd ("net: ipv6: add support for rpl sr exthdr")
> > Cc: "David S. Miller" <davem@davemloft.net>
> > Cc: David Ahern <dsahern@kernel.org>
> > Cc: Eric Dumazet <edumazet@google.com>
> > Cc: Jakub Kicinski <kuba@kernel.org>
> > Cc: Paolo Abeni <pabeni@redhat.com>
> > Cc: Simon Horman <horms@kernel.org>
> > Cc: stable <stable@kernel.org>
> > Reported-by: Anthropic
> > Assisted-by: gkh_clanker_t1000
> > Signed-off-by: Greg Kroah-Hartman <gregkh@linuxfoundation.org>
> > ---
> > v3: - skb_postpull_rcsum() should not be changed, it's chdr NOT hdr, ugh
> > Link to v2: https://lore.kernel.org/r/2026042158-sediment-elliptic-a954@gregkh
> >
> > v2: - fixed up if statement to actually work properly, and test it
> > against a working poc (poc will be sent separately)
> > Reworded the changelog and the subject to make more sense
> > Link to v1: https://lore.kernel.org/r/2026042024-cabbie-gills-9371@gregkh
> >
> > net/ipv6/exthdrs.c | 9 ++++++---
> > 1 file changed, 6 insertions(+), 3 deletions(-)
> >
> > diff --git a/net/ipv6/exthdrs.c b/net/ipv6/exthdrs.c
> > index 95558fd6f447..03cbce842c1a 100644
> > --- a/net/ipv6/exthdrs.c
> > +++ b/net/ipv6/exthdrs.c
> > @@ -491,6 +491,7 @@ static int ipv6_rpl_srh_rcv(struct sk_buff *skb)
> > struct net *net = dev_net(skb->dev);
> > struct inet6_dev *idev;
> > struct ipv6hdr *oldhdr;
> > + unsigned int chdr_len;
> > unsigned char *buf;
> > int accept_rpl_seg;
> > int i, err;
> > @@ -592,8 +593,10 @@ static int ipv6_rpl_srh_rcv(struct sk_buff *skb)
> > skb_pull(skb, ((hdr->hdrlen + 1) << 3));
> > skb_postpull_rcsum(skb, oldhdr,
> > sizeof(struct ipv6hdr) + ((hdr->hdrlen + 1) << 3));
> > - if (unlikely(!hdr->segments_left)) {
> > - if (pskb_expand_head(skb, sizeof(struct ipv6hdr) + ((chdr->hdrlen + 1) << 3), 0,
> > + chdr_len = sizeof(struct ipv6hdr) + ((chdr->hdrlen + 1) << 3);
> > + if (unlikely(!hdr->segments_left ||
> > + skb_headroom(skb) < chdr_len + skb->mac_len)) {
> > + if (pskb_expand_head(skb, chdr_len + skb->mac_len, 0,
> > GFP_ATOMIC)) {
> > __IP6_INC_STATS(net, ip6_dst_idev(skb_dst(skb)), IPSTATS_MIB_OUTDISCARDS);
> > kfree_skb(skb);
> > @@ -603,7 +606,7 @@ static int ipv6_rpl_srh_rcv(struct sk_buff *skb)
> >
> > oldhdr = ipv6_hdr(skb);
> > }
> > - skb_push(skb, ((chdr->hdrlen + 1) << 3) + sizeof(struct ipv6hdr));
> > + skb_push(skb, chdr_len);
> > skb_reset_network_header(skb);
> > skb_mac_header_rebuild(skb);
> > skb_set_transport_header(skb, sizeof(struct ipv6hdr));
> > --
> > 2.53.0
> >
>
> I am happy to see this bug is finally fixed.
>
> However, it seems the Reported-by tag does not include our credit.
>
> We first submitted the security report and patch to
> security@kernel.org on February 23, titled:
>
> [SECURITY] IPv6/RPL: 14-byte controllable OOB write via SRH len
> overflow + skb_mac_header_rebuild() u16 wraparound
>
> [PATCH] ipv6: rpl: rebuild MAC+metadata safely when rewriting SRH
>
> We have aslo followed up and resent the patch several times, CCing all
> relevant maintainers.
>
> Could our credit be added to the patch?
>
> Reported-by: Yuan Tan <yuantan098@gmail.com>
> Reported-by: Yifan Wu <yifanwucs@gmail.com>
> Reported-by: Juefei Pu <tomapufckgml@gmail.com>
> Reported-by: Xin Liu <dstsmallbird@foxmail.com>
>
> I noticed this patch has already been merged into the mainline. Maybe
> it's too late to amend this patch. Is there any other way to document
> our reporting credit in the official records? Maybe in the stable
> backport?

Sorry, I made this patch based on a different report to me as is
documented in this patch, and did not notice that it was the same thing
as this one as I am drowning in reports.

This happens at times, and we can't rewrite public git history as that's
impossible to do, sorry about that. You have a public confirmation here
that yes, you did report this same issue, and yes, you did submit a
patch to resolve this, but it ended up being different from the one that
was accepted. Which again, happens all the time, it's just the nature
of development with a huge body of contributors.

thanks,

greg k-h