Re: [PATCH 4.4] ipv6: check skb->protocol before lookup for nexthop

[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]

 



On Wed, Aug 19, 2020 at 09:11:17PM +0100, Alessio Balsini wrote:
> From: WANG Cong <xiyou.wangcong@xxxxxxxxx>
> 
> [ Upstream commit 199ab00f3cdb6f154ea93fa76fd80192861a821d ]
> 
> Andrey reported a out-of-bound access in ip6_tnl_xmit(), this
> is because we use an ipv4 dst in ip6_tnl_xmit() and cast an IPv4
> neigh key as an IPv6 address:
> 
>         neigh = dst_neigh_lookup(skb_dst(skb),
>                                  &ipv6_hdr(skb)->daddr);
>         if (!neigh)
>                 goto tx_err_link_failure;
> 
>         addr6 = (struct in6_addr *)&neigh->primary_key; // <=== HERE
>         addr_type = ipv6_addr_type(addr6);
> 
>         if (addr_type == IPV6_ADDR_ANY)
>                 addr6 = &ipv6_hdr(skb)->daddr;
> 
>         memcpy(&fl6->daddr, addr6, sizeof(fl6->daddr));
> 
> Also the network header of the skb at this point should be still IPv4
> for 4in6 tunnels, we shold not just use it as IPv6 header.
> 
> This patch fixes it by checking if skb->protocol is ETH_P_IPV6: if it
> is, we are safe to do the nexthop lookup using skb_dst() and
> ipv6_hdr(skb)->daddr; if not (aka IPv4), we have no clue about which
> dest address we can pick here, we have to rely on callers to fill it
> from tunnel config, so just fall to ip6_route_output() to make the
> decision.
> 
> Fixes: ea3dc9601bda ("ip6_tunnel: Add support for wildcard tunnel endpoints.")
> Reported-by: Andrey Konovalov <andreyknvl@xxxxxxxxxx>
> Reported-by: syzbot+01400f5fc51cf4747bec@xxxxxxxxxxxxxxxxxxxxxxxxx
> Tested-by: Andrey Konovalov <andreyknvl@xxxxxxxxxx>
> Cc: Steffen Klassert <steffen.klassert@xxxxxxxxxxx>
> Signed-off-by: Cong Wang <xiyou.wangcong@xxxxxxxxx>
> Signed-off-by: David S. Miller <davem@xxxxxxxxxxxxx>
> Signed-off-by: Greg Kroah-Hartman <gregkh@xxxxxxxxxxxxxxxxxxx>
> Signed-off-by: Alessio Balsini <balsini@xxxxxxxxxxx>
> ---
>  net/ipv6/ip6_tunnel.c | 32 +++++++++++++++++---------------
>  1 file changed, 17 insertions(+), 15 deletions(-)

This was already applied to the 4.4.66 kernel release

But this patch applies to the 4.4.y tree.  Which is really really odd,
what is going on here?

confused,

greg k-h



[Index of Archives]     [Linux Kernel]     [Kernel Development Newbies]     [Linux USB Devel]     [Video for Linux]     [Linux Audio Users]     [Yosemite Hiking]     [Linux Kernel]     [Linux SCSI]

  Powered by Linux