netdev - Re: [PATCH net-next 4/8] udp: never accept GSO

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]

Message-ID: <CA+FuTSefLQ07od6Et6H2wO=p8+V2F28VYix4EgghHz6R0Bn9nw@mail.gmail.com>
Date:   Mon, 22 Mar 2021 09:42:04 -0400
From:   Willem de Bruijn <willemdebruijn.kernel@...il.com>
To:     Paolo Abeni <pabeni@...hat.com>
Cc:     Network Development <netdev@...r.kernel.org>,
        "David S. Miller" <davem@...emloft.net>,
        Jakub Kicinski <kuba@...nel.org>,
        Steffen Klassert <steffen.klassert@...unet.com>,
        Alexander Lobakin <alobakin@...me>
Subject: Re: [PATCH net-next 4/8] udp: never accept GSO_FRAGLIST packets

On Sun, Mar 21, 2021 at 1:01 PM Paolo Abeni <pabeni@...hat.com> wrote:
>
> Currently the UDP protocol delivers GSO_FRAGLIST packets to
> the sockets without the expected segmentation.
>
> This change addresses the issue introducing and maintaining
> a per socket bitmask of GSO types requiring segmentation.
> Enabling GSO removes SKB_GSO_UDP_L4 from such mask, while
> GSO_FRAGLIST packets are never accepted
>
> Note: this also updates the 'unused' field size to really
> fit the otherwise existing hole. It's size become incorrect
> after commit bec1f6f69736 ("udp: generate gso with UDP_SEGMENT").
>
> Fixes: 9fd1ff5d2ac7 ("udp: Support UDP fraglist GRO/GSO.")
> Signed-off-by: Paolo Abeni <pabeni@...hat.com>
> ---
>  include/linux/udp.h | 10 ++++++----
>  net/ipv4/udp.c      | 12 +++++++++++-
>  2 files changed, 17 insertions(+), 5 deletions(-)
>
> diff --git a/include/linux/udp.h b/include/linux/udp.h
> index aa84597bdc33c..6da342f15f351 100644
> --- a/include/linux/udp.h
> +++ b/include/linux/udp.h
> @@ -51,7 +51,7 @@ struct udp_sock {
>                                            * different encapsulation layer set
>                                            * this
>                                            */
> -                        gro_enabled:1; /* Can accept GRO packets */
> +                        gro_enabled:1; /* Request GRO aggregation */

unnecessary comment change?

>         /*
>          * Following member retains the information to create a UDP header
>          * when the socket is uncorked.
> @@ -68,7 +68,10 @@ struct udp_sock {
>  #define UDPLITE_SEND_CC  0x2           /* set via udplite setsockopt         */
>  #define UDPLITE_RECV_CC  0x4           /* set via udplite setsocktopt        */
>         __u8             pcflag;        /* marks socket as UDP-Lite if > 0    */
> -       __u8             unused[3];
> +       __u8             unused[1];
> +       unsigned int     unexpected_gso;/* GSO types this socket can't accept,
> +                                        * any of SKB_GSO_UDP_L4 or SKB_GSO_FRAGLIST
> +                                        */

An extra unsigned int for this seems overkill.

Current sockets that support SKB_GSO_UDP_L4 implicitly also support
SKB_GSO_FRAGLIST. This patch makes explicit that the second is not
supported..

>         /*
>          * For encapsulation sockets.
>          */
> @@ -131,8 +134,7 @@ static inline void udp_cmsg_recv(struct msghdr *msg, struct sock *sk,
>
>  static inline bool udp_unexpected_gso(struct sock *sk, struct sk_buff *skb)
>  {
> -       return !udp_sk(sk)->gro_enabled && skb_is_gso(skb) &&
> -              skb_shinfo(skb)->gso_type & SKB_GSO_UDP_L4;
> +       return skb_is_gso(skb) && skb_shinfo(skb)->gso_type & udp_sk(sk)->unexpected_gso;

.. just update this function as follows ?

 -       return !udp_sk(sk)->gro_enabled && skb_is_gso(skb) &&
 -              skb_shinfo(skb)->gso_type & SKB_GSO_UDP_L4;
+       return skb_is_gso(skb) &&
+                 (skb_shinfo(skb)->gso_type & SKB_GSO_FRAGLIST ||
!udp_sk(sk)->gro_enabled)

where the latter is shorthand for

  (skb_shinfo(skb)->gso_type & SKB_GSO_UDP_L4 && !udp_sk(sk)->gro_enabled)

but the are the only two GSO types that could arrive here.