netdev - Re: [PATCH v1 net] icmp6: Fix null-ptr-deref of fib6_null_entry->rt6i_idev in icmp6

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] [day] [month] [year] [list]

Date: Fri, 7 Jul 2023 17:07:52 -0700
From: Kuniyuki Iwashima <kuniyu@...zon.com>
To: <dsahern@...nel.org>
CC: <davem@...emloft.net>, <edumazet@...gle.com>, <kuba@...nel.org>,
	<kuni1840@...il.com>, <kuniyu@...zon.com>, <netdev@...r.kernel.org>,
	<pabeni@...hat.com>, <wangyufen@...wei.com>
Subject: Re: [PATCH v1 net] icmp6: Fix null-ptr-deref of fib6_null_entry->rt6i_idev in icmp6_dev().

From: David Ahern <dsahern@...nel.org>
Date: Fri, 7 Jul 2023 15:42:58 -0600
> On 7/6/23 5:30 PM, Kuniyuki Iwashima wrote:
> > With some IPv6 Ext Hdr (RPL, SRv6, etc.), we can send a packet that
> > has the link-local address as src and dst IP and will be forwarded to
> > an external IP in the IPv6 Ext Hdr.
> > 
> > For example, the script below generates a packet whose src IP is the
> > link-local address and dst is updated to 11::.
> > 
> >   # for f in $(find /proc/sys/net/ -name *seg6_enabled*); do echo 1 > $f; done
> >   # python3
> >   >>> from socket import *
> >   >>> from scapy.all import *
> >   >>>
> >   >>> SRC_ADDR = DST_ADDR = "fe80::5054:ff:fe12:3456"
> >   >>>
> >   >>> pkt = IPv6(src=SRC_ADDR, dst=DST_ADDR)
> >   >>> pkt /= IPv6ExtHdrSegmentRouting(type=4, addresses=["11::", "22::"], segleft=1)
> >   >>>
> >   >>> sk = socket(AF_INET6, SOCK_RAW, IPPROTO_RAW)
> >   >>> sk.sendto(bytes(pkt), (DST_ADDR, 0))
> > 
> > For such a packet, we call ip6_route_input() to look up a route for the
> > next destination in these three functions depending on the header type.
> > 
> >   * ipv6_rthdr_rcv()
> >   * ipv6_rpl_srh_rcv()
> >   * ipv6_srh_rcv()
> > 
> > If no route is found, fib6_null_entry is set to skb, and the following
> 
> ip6_null_entry is the one set on the skb.

Oops, I'm lost while writing :)

I'll update subject, changelog, and comment in v2.

Thanks, David!


> 
> > dst_input(skb) calls ip6_pkt_drop().
> > 
> > Finally, in icmp6_dev(), we dereference skb_rt6_info(skb)->rt6i_idev->dev
> > as the input device is the loopback interface.  Then, we have to check if
> > skb_rt6_info(skb)->rt6i_idev is NULL or not to avoid NULL pointer deref
> > for fib6_null_entry.
> > 
> 
> ...
> 
> > 
> > Fixes: 4832c30d5458 ("net: ipv6: put host and anycast routes on device with address")
> > Reported-by: Wang Yufen <wangyufen@...wei.com>
> > Closes: https://lore.kernel.org/netdev/1ddf7fc8-bcb3-ab48-4894-24158e8a9d0f@huawei.com/
> > Signed-off-by: Kuniyuki Iwashima <kuniyu@...zon.com>
> > ---
> >  net/ipv6/icmp.c | 5 ++++-
> >  1 file changed, 4 insertions(+), 1 deletion(-)
> > 
> > diff --git a/net/ipv6/icmp.c b/net/ipv6/icmp.c
> > index 9edf1f45b1ed..bd6479e3c757 100644
> > --- a/net/ipv6/icmp.c
> > +++ b/net/ipv6/icmp.c
> > @@ -424,7 +424,10 @@ static struct net_device *icmp6_dev(const struct sk_buff *skb)
> >  	if (unlikely(dev->ifindex == LOOPBACK_IFINDEX || netif_is_l3_master(skb->dev))) {
> >  		const struct rt6_info *rt6 = skb_rt6_info(skb);
> >  
> > -		if (rt6)
> > +		/* The destination could be an external IP in Ext Hdr (SRv6, RPL, etc.),
> > +		 * and fib6_null_entry could be set to skb if no route is found.
> > +		 */
> > +		if (rt6 && rt6->rt6i_idev)
> >  			dev = rt6->rt6i_idev->dev;
> >  	}
> >  
> 
> Reviewed-by: David Ahern <dsahern@...nel.org>