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
| ||
|
Message-ID: <ZGZavH7hxiq/pkF8@corigine.com> Date: Thu, 18 May 2023 19:05:00 +0200 From: Simon Horman <simon.horman@...igine.com> To: Eric Dumazet <edumazet@...gle.com> Cc: "David S . Miller" <davem@...emloft.net>, Jakub Kicinski <kuba@...nel.org>, Paolo Abeni <pabeni@...hat.com>, Alexander Aring <alex.aring@...il.com>, David Lebrun <david.lebrun@...ouvain.be>, netdev@...r.kernel.org, eric.dumazet@...il.com Subject: Re: [PATCH net 1/3] ipv6: exthdrs: fix potential use-after-free in ipv6_rpl_srh_rcv() On Wed, May 17, 2023 at 09:31:16PM +0000, Eric Dumazet wrote: > After calls to pskb_may_pull() we need to reload @hdr variable, > because skb->head might have changed. > > We need to move up first pskb_may_pull() call right after > looped_back label. > > Fixes: 8610c7c6e3bd ("net: ipv6: add support for rpl sr exthdr") > Signed-off-by: Eric Dumazet <edumazet@...gle.com> > Cc: Alexander Aring <alex.aring@...il.com> > Cc: David Lebrun <david.lebrun@...ouvain.be> > --- > net/ipv6/exthdrs.c | 11 +++++------ > 1 file changed, 5 insertions(+), 6 deletions(-) > > diff --git a/net/ipv6/exthdrs.c b/net/ipv6/exthdrs.c > index a8d961d3a477f6516f542025dfbcfc6f47407a70..b129e982205ee43cbf74f4900c3031827d962dc2 100644 > --- a/net/ipv6/exthdrs.c > +++ b/net/ipv6/exthdrs.c > @@ -511,6 +511,10 @@ static int ipv6_rpl_srh_rcv(struct sk_buff *skb) > } > > looped_back: > + if (!pskb_may_pull(skb, sizeof(*hdr))) { > + kfree_skb(skb); > + return -1; > + } > hdr = (struct ipv6_rpl_sr_hdr *)skb_transport_header(skb); > > if (hdr->segments_left == 0) { Hi Eric, Not far below this line there is a call to pskb_pull(): if (hdr->nexthdr == NEXTHDR_IPV6) { int offset = (hdr->hdrlen + 1) << 3; skb_postpull_rcsum(skb, skb_network_header(skb), skb_network_header_len(skb)); if (!pskb_pull(skb, offset)) { kfree_skb(skb); return -1; } skb_postpull_rcsum(skb, skb_transport_header(skb), offset); Should hdr be reloaded after the call to pskb_pull() too? > @@ -544,12 +548,6 @@ static int ipv6_rpl_srh_rcv(struct sk_buff *skb) > > return 1; > } > - > - if (!pskb_may_pull(skb, sizeof(*hdr))) { > - kfree_skb(skb); > - return -1; > - } > - > n = (hdr->hdrlen << 3) - hdr->pad - (16 - hdr->cmpre); > r = do_div(n, (16 - hdr->cmpri)); > /* checks if calculation was without remainder and n fits into > @@ -592,6 +590,7 @@ static int ipv6_rpl_srh_rcv(struct sk_buff *skb) > kfree_skb(skb); > return -1; > } > + hdr = (struct ipv6_rpl_sr_hdr *)skb_transport_header(skb); > > hdr->segments_left--; > i = n - hdr->segments_left; > -- > 2.40.1.606.ga4b1b128d6-goog > >
Powered by blists - more mailing lists