netdev - Re: [patch RFC net-next 2/3] icmp: ICMPV6: Examine invoking packet for Segment Route Headers.

lists.openwall.net		lists / announce owl-users owl-dev john-users john-dev passwdqc-users yescrypt popa3d-users / oss-security kernel-hardening musl sabotage tlsify passwords / crypt-dev xvendor / Bugtraq Full-Disclosure linux-kernel linux-netdev linux-ext4 linux-hardening linux-cve-announce PHC
Open Source and information security mailing list archives

Hash Suite: Windows password security audit tool. GUI, reports in PDF.

[<prev] [next>] [<thread-prev] [thread-next>] [day] [month] [year] [list]

Message-ID: <YafG5hboD7itUddn@lunn.ch>
Date:   Wed, 1 Dec 2021 20:03:02 +0100
From:   Andrew Lunn <andrew@...n.ch>
To:     Willem de Bruijn <willemdebruijn.kernel@...il.com>
Cc:     David Miller <davem@...emloft.net>,
        Jakub Kicinski <kuba@...nel.org>,
        Hideaki YOSHIFUJI <yoshfuji@...ux-ipv6.org>,
        David Ahern <dsahern@...nel.org>,
        James Prestwood <prestwoj@...il.com>,
        Justin Iurman <justin.iurman@...ege.be>,
        Praveen Chaudhary <praveen5582@...il.com>,
        "Jason A . Donenfeld" <Jason@...c4.com>,
        Eric Dumazet <edumazet@...gle.com>,
        netdev <netdev@...r.kernel.org>
Subject: Re: [patch RFC net-next 2/3] icmp: ICMPV6: Examine invoking packet
 for Segment Route Headers.

On Wed, Dec 01, 2021 at 10:22:38AM -0800, Willem de Bruijn wrote:
> > > > +static void icmpv6_notify_srh(struct sk_buff *skb, struct inet6_skb_parm *opt)
> > > > +{
> > > > +       struct sk_buff *skb_orig;
> > > > +       struct ipv6_sr_hdr *srh;
> > > > +
> > > > +       skb_orig = skb_clone(skb, GFP_ATOMIC);
> > > > +       if (!skb_orig)
> > > > +               return;
> > >
> > > Is this to be allowed to write to skb->cb? Or because seg6_get_srh
> > > calls pskb_may_pull to parse the headers?
> >
> > This is an ICMP error message. So we have an IP packet, skb, which
> > contains in the message body the IP packet which invoked the error. If
> > we pass skb to seg6_get_srh() it will look in the received ICMP
> > packet. But we actually want to find the SRH in the packet which
> > invoked the error, the one which is in the message body. So the code
> > makes a clone of the skb, and then updates the pointers so that it
> > points to the invoking packet within the ICMP packet. Then we can use
> > seg6_get_srh() on this inner packet, since it just looks like an
> > ordinary IP packet.
> 
> Ah of course. I clearly did not appreciate the importance of that
> skb_reset_network_header.

So i should probably add a comment here. If we stick with this design.

> > Yes, i checked that. Because the skb has been cloned, if it needs to
> > rearrange the packet because it goes over a fragment boundary,
> > pskb_may_pull() will return false. And then we won't find the
> > SRH.
> 
> Great. So the feature only works if the SRH is in the linear header.

Yes, traceroute will remain broken if the invoking SRH header is not
in the linear header.

> Then if the packet is not shared, you can just temporarily reset the
> network header and revert it after?

Maybe. I was worried about any side affects of such an
operation. Working on a clone seemed a lot less risky.

Is it safe to due such games with the network header?

	Andrew