All of lore.kernel.org
 help / color / mirror / Atom feed
From: Greg Kroah-Hartman <gregkh@linuxfoundation.org>
To: Yuan Tan <yuantan098@gmail.com>
Cc: netdev@vger.kernel.org, linux-kernel@vger.kernel.org,
	"David S. Miller" <davem@davemloft.net>,
	David Ahern <dsahern@kernel.org>,
	Eric Dumazet <edumazet@google.com>,
	Jakub Kicinski <kuba@kernel.org>, Paolo Abeni <pabeni@redhat.com>,
	Simon Horman <horms@kernel.org>, stable <stable@kernel.org>
Subject: Re: [PATCH net v3] ipv6: rpl: reserve mac_len headroom when recompressed SRH grows
Date: Sat, 2 May 2026 10:42:58 +0200	[thread overview]
Message-ID: <2026050237-subheader-footsie-9f72@gregkh> (raw)
In-Reply-To: <CAPuPA7JS-yG_dYaPVjv9JiGOEXSpExXSQhsZaexUkr-WMeQHrw@mail.gmail.com>

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

      reply	other threads:[~2026-05-02  8:43 UTC|newest]

Thread overview: 4+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2026-04-21 13:16 [PATCH net v3] ipv6: rpl: reserve mac_len headroom when recompressed SRH grows Greg Kroah-Hartman
2026-04-28  1:10 ` patchwork-bot+netdevbpf
2026-05-02  8:36 ` Yuan Tan
2026-05-02  8:42   ` Greg Kroah-Hartman [this message]

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=2026050237-subheader-footsie-9f72@gregkh \
    --to=gregkh@linuxfoundation.org \
    --cc=davem@davemloft.net \
    --cc=dsahern@kernel.org \
    --cc=edumazet@google.com \
    --cc=horms@kernel.org \
    --cc=kuba@kernel.org \
    --cc=linux-kernel@vger.kernel.org \
    --cc=netdev@vger.kernel.org \
    --cc=pabeni@redhat.com \
    --cc=stable@kernel.org \
    --cc=yuantan098@gmail.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 an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.