netdev - Re: [RFC PATCH v2 03/10] udp: add support for UDP

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: <CAF=yD-+JOJJX+vOGuV3z3aYNEdjNQ=U0+OaZdeuVvOTrRa-oRA@mail.gmail.com>
Date:   Sun, 21 Oct 2018 16:07:55 -0400
From:   Willem de Bruijn <willemdebruijn.kernel@...il.com>
To:     Paolo Abeni <pabeni@...hat.com>
Cc:     Network Development <netdev@...r.kernel.org>,
        Willem de Bruijn <willemb@...gle.com>,
        steffen.klassert@...unet.com
Subject: Re: [RFC PATCH v2 03/10] udp: add support for UDP_GRO cmsg

On Fri, Oct 19, 2018 at 10:30 AM Paolo Abeni <pabeni@...hat.com> wrote:
>
> When UDP GRO is enabled, the UDP_GRO cmsg will carry the ingress
> datagram size. User-space can use such info to compute the original
> packets layout.
>
> Signed-off-by: Paolo Abeni <pabeni@...hat.com>
> ---
> CHECK: should we use a separate setsockopt to explicitly enable
> gso_size cmsg reception? So that user space can enable UDP_GRO and
> fetch cmsg without forcefully receiving GRO related info.

A user can avoid the message by not passing control data. Though in
most practical cases it seems unsafe to do so, anyway, as the path MTU
can be lower than the expected device MTU.

> diff --git a/net/ipv4/udp.c b/net/ipv4/udp.c
> index 3c277378814f..2331ac9de954 100644
> --- a/net/ipv4/udp.c
> +++ b/net/ipv4/udp.c
> @@ -1714,6 +1714,10 @@ int udp_recvmsg(struct sock *sk, struct msghdr *msg, size_t len, int noblock,
>                 memset(sin->sin_zero, 0, sizeof(sin->sin_zero));
>                 *addr_len = sizeof(*sin);
>         }
> +
> +       if (udp_sk(sk)->gro_enabled)
> +               udp_cmsg_recv(msg, sk, skb);
> +

Perhaps we can avoid adding a branch by setting a bit in
inet->cmsg_flags for gso_size to let the below branch handle the cmsg
processing.

I'd still set that as part of the UDP_GRO setsockopt. Though if you
insist it could be a value 2 instead of 1, effectively allowing the
above mentioned opt-out.

>         if (inet->cmsg_flags)
>                 ip_cmsg_recv_offset(msg, sk, skb, sizeof(struct udphdr), off);
>