Re: [PATCH net] udp: Fix out-of-bounds read with unset mac header in unexpected GSO path

[Willem de Bruijn <willemdebruijn.kernel@gmail.com>]

Boyan Liu wrote:
> udp_queue_rcv_skb() and udpv6_queue_rcv_skb() re-segment an
> unexpected GSO packet after restoring skb->data to the mac
> header with __skb_push(skb, -skb_mac_offset(skb)), assuming
> the mac header is valid.
> 
> SRv6 End.DT4/End.DT6 VRF decapsulation calls
> skb_unset_mac_header(). iptunnel_pull_offloads() can leave an
> inner UDP GSO type (SKB_GSO_UDP_L4 or SKB_GSO_FRAGLIST)
> behind, so a socket that does not accept that GRO type runs
> the unexpected GSO path with mac_header == 0xffff.
> skb_mac_offset() then returns a large positive value, which
> the unsigned __skb_push() argument turns into a huge length:
> skb->data moves far outside the buffer and skb->len is
> corrupted, so segmentation reads out of bounds:
> 
>     BUG: KASAN: slab-out-of-bounds in inet_gso_segment (net/ipv4/af_inet.c:1383)
>     Read of size 1 at addr ffff88800da7ffff by task exploit/149
>     Call Trace:
>      inet_gso_segment (net/ipv4/af_inet.c:1383)
>      skb_mac_gso_segment (net/core/gso.c:53)
>      __skb_gso_segment (net/core/gso.c:124)
>      udp_queue_rcv_skb (net/ipv4/udp.c:2476 net/ipv4/udp.c:2466)
>      udp_unicast_rcv_skb (net/ipv4/udp.c:2620)
>      ip_local_deliver (net/ipv4/ip_input.c:262)
>      seg6_local_input_core (net/ipv6/seg6_local.c:1640)
>      lwtunnel_input (net/core/lwtunnel.c:465)
>      ipv6_rcv (net/ipv6/ip6_input.c:351)
> 
> __skb_gso_segment() resets the mac header to wherever
> skb->data points, so the push target only needs to be a valid
> header start. Fall back to the network header when the mac
> header was never set.

I wonder if the right fix here is to not call skb_unset_mac_header in
the first place.

There are plenty of L3 device type with no L2 header. They still have
a valid skb_mac_header.

This fix may be papering over a tunnel doing something non-standard.

> Fixes: cf329aa42b66 ("udp: cope with UDP GRO packet misdirection")
> Cc: stable@vger.kernel.org
> Reported-by: co+98d7de09cd94fec5@bugs.sh
> Signed-off-by: Boyan Liu <yymhvert@gmail.com>
> ---
>  net/ipv4/udp.c | 5 ++++-
>  net/ipv6/udp.c | 5 ++++-
>  2 files changed, 8 insertions(+), 2 deletions(-)
> 
> diff --git a/net/ipv4/udp.c b/net/ipv4/udp.c
> index bb8cfc62c..fe5a23f1c 100644
> --- a/net/ipv4/udp.c
> +++ b/net/ipv4/udp.c
> @@ -2441,7 +2441,10 @@ static int udp_queue_rcv_skb(struct sock *sk, struct sk_buff *skb)
>  		return udp_queue_rcv_one_skb(sk, skb);
>  
>  	BUILD_BUG_ON(sizeof(struct udp_skb_cb) > SKB_GSO_CB_OFFSET);
> -	__skb_push(skb, -skb_mac_offset(skb));
> +	if (skb_mac_header_was_set(skb))
> +		__skb_push(skb, -skb_mac_offset(skb));
> +	else
> +		__skb_push(skb, -skb_network_offset(skb));
>  	segs = udp_rcv_segment(sk, skb, true);
>  	skb_list_walk_safe(segs, skb, next) {
>  		__skb_pull(skb, skb_transport_offset(skb));
> diff --git a/net/ipv6/udp.c b/net/ipv6/udp.c
> index 93478d1ad..8db4e2869 100644
> --- a/net/ipv6/udp.c
> +++ b/net/ipv6/udp.c
> @@ -895,7 +895,10 @@ static int udpv6_queue_rcv_skb(struct sock *sk, struct sk_buff *skb)
>  	if (likely(!udp_unexpected_gso(sk, skb)))
>  		return udpv6_queue_rcv_one_skb(sk, skb);
>  
> -	__skb_push(skb, -skb_mac_offset(skb));
> +	if (skb_mac_header_was_set(skb))
> +		__skb_push(skb, -skb_mac_offset(skb));
> +	else
> +		__skb_push(skb, -skb_network_offset(skb));
>  	segs = udp_rcv_segment(sk, skb, false);
>  	skb_list_walk_safe(segs, skb, next) {
>  		__skb_pull(skb, skb_transport_offset(skb));
> -- 
> 2.43.0
>