netdev - Re: [RFC PATCH v2 06/10] udp: cope with UDP GRO packet misdirection

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

Date:   Mon, 22 Oct 2018 13:43:17 +0200
From:   Steffen Klassert <steffen.klassert@...unet.com>
To:     Paolo Abeni <pabeni@...hat.com>
CC:     <netdev@...r.kernel.org>, Willem de Bruijn <willemb@...gle.com>
Subject: Re: [RFC PATCH v2 06/10] udp: cope with UDP GRO packet misdirection

On Fri, Oct 19, 2018 at 04:25:16PM +0200, Paolo Abeni wrote:
> +
> +static inline struct sk_buff *udp_rcv_segment(struct sock *sk,
> +					      struct sk_buff *skb)
> +{
> +	struct sk_buff *segs;
> +
> +	/* the GSO CB lays after the UDP one, no need to save and restore any
> +	 * CB fragment, just initialize it
> +	 */
> +	segs = __skb_gso_segment(skb, NETIF_F_SG, false);
> +	if (unlikely(IS_ERR(segs)))
> +		kfree_skb(skb);
> +	else if (segs)
> +		consume_skb(skb);
> +	return segs;
> +}
> +
> +

One empty line too much.

>  #define udp_portaddr_for_each_entry(__sk, list) \
>  	hlist_for_each_entry(__sk, list, __sk_common.skc_portaddr_node)
>  
> diff --git a/net/ipv4/udp.c b/net/ipv4/udp.c
> index 2331ac9de954..0d55145ce9f5 100644
> --- a/net/ipv4/udp.c
> +++ b/net/ipv4/udp.c
> @@ -1909,7 +1909,7 @@ EXPORT_SYMBOL(udp_encap_enable);
>   * Note that in the success and error cases, the skb is assumed to
>   * have either been requeued or freed.
>   */
> -static int udp_queue_rcv_skb(struct sock *sk, struct sk_buff *skb)
> +static int udp_queue_rcv_one_skb(struct sock *sk, struct sk_buff *skb)
>  {
>  	struct udp_sock *up = udp_sk(sk);
>  	int is_udplite = IS_UDPLITE(sk);
> @@ -2012,6 +2012,29 @@ static int udp_queue_rcv_skb(struct sock *sk, struct sk_buff *skb)
>  	return -1;
>  }
>  
> +void ip_protocol_deliver_rcu(struct net *net, struct sk_buff *skb, int proto);
> +
> +static int udp_queue_rcv_skb(struct sock *sk, struct sk_buff *skb)
> +{
> +	struct sk_buff *next, *segs;
> +	int ret;
> +
> +	if (likely(!udp_unexpected_gso(sk, skb)))
> +		return udp_queue_rcv_one_skb(sk, skb);
> +
> +	BUILD_BUG_ON(sizeof(struct udp_skb_cb) > SKB_SGO_CB_OFFSET);
> +	__skb_push(skb, -skb_mac_offset(skb));
> +	segs = udp_rcv_segment(sk, skb);
> +	for (skb = segs; skb; skb = next) {
> +		next = skb->next;
> +		__skb_pull(skb, skb_transport_offset(skb));
> +		ret = udp_queue_rcv_one_skb(sk, skb);

udp_queue_rcv_one_skb() starts with doing a xfrm4_policy_check().
Maybe we can do this on the GSO packet instead of the segments.
So far this code is just for handling a corner case, but this might
change.