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

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

[Boyan Liu]

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.

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

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

[Willem de Bruijn]

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
>