netdev - Re: [PATCH for-6.1 1/2] io_uring/net: fail zc send for unsupported protocols

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: <ed49aa87-5481-ae92-2488-e959121e8869@kernel.dk>
Date:   Thu, 20 Oct 2022 05:49:35 -0700
From:   Jens Axboe <axboe@...nel.dk>
To:     Stefan Metzmacher <metze@...ba.org>,
        Pavel Begunkov <asml.silence@...il.com>,
        io-uring@...r.kernel.org
Cc:     netdev@...r.kernel.org
Subject: Re: [PATCH for-6.1 1/2] io_uring/net: fail zc send for unsupported
 protocols

On 10/20/22 2:13 AM, Stefan Metzmacher wrote:
> Hi Pavel,
> 
>> If a protocol doesn't support zerocopy it will silently fall back to
>> copying. This type of behaviour has always been a source of troubles
>> so it's better to fail such requests instead. For now explicitly
>> whitelist supported protocols in io_uring, which should be turned later
>> into a socket flag.
>>
>> Cc: <stable@...r.kernel.org> # 6.0
>> Signed-off-by: Pavel Begunkov <asml.silence@...il.com>
>> ---
>>   io_uring/net.c | 9 +++++++++
>>   1 file changed, 9 insertions(+)
>>
>> diff --git a/io_uring/net.c b/io_uring/net.c
>> index 8c7226b5bf41..28127f1de1f0 100644
>> --- a/io_uring/net.c
>> +++ b/io_uring/net.c
>> @@ -120,6 +120,13 @@ static void io_netmsg_recycle(struct io_kiocb *req, unsigned int issue_flags)
>>       }
>>   }
>>   +static inline bool io_sock_support_zc(struct socket *sock)
>> +{
>> +    return likely(sock->sk && sk_fullsock(sock->sk) &&
>> +             (sock->sk->sk_protocol == IPPROTO_TCP ||
>> +              sock->sk->sk_protocol == IPPROTO_UDP));
>> +}
> 
> Can we please make this more generic (at least for 6.1, which is likely be an lts release)
> 
> It means my out of tree smbdirect driver would not be able to provide SENDMSG_ZC.
> 
> Currently sk_setsockopt has this logic:
> 
>         case SO_ZEROCOPY:
>                 if (sk->sk_family == PF_INET || sk->sk_family == PF_INET6) {
>                         if (!(sk_is_tcp(sk) ||
>                               (sk->sk_type == SOCK_DGRAM &&
>                                sk->sk_protocol == IPPROTO_UDP)))
>                                 ret = -EOPNOTSUPP;
>                 } else if (sk->sk_family != PF_RDS) {
>                         ret = -EOPNOTSUPP;
>                 }
>                 if (!ret) {
>                         if (val < 0 || val > 1)
>                                 ret = -EINVAL;
>                         else
>                                 sock_valbool_flag(sk, SOCK_ZEROCOPY, valbool);
>                 }
>                 break;
> 
> Maybe the socket creation code could set
> unsigned char skc_so_zerocopy_supported:1;
> and/or
> unsigned char skc_zerocopy_msg_ubuf_supported:1;
> 
> In order to avoid the manual complex tests.

I agree that would be cleaner, even for 6.1. Let's drop these two
for now.

-- 
Jens Axboe