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: <d2eb10cb-3cce-429c-7a72-7153b5442377@gmail.com>
Date:   Thu, 20 Oct 2022 13:48:37 +0100
From:   Pavel Begunkov <asml.silence@...il.com>
To:     Stefan Metzmacher <metze@...ba.org>, io-uring@...r.kernel.org
Cc:     Jens Axboe <axboe@...nel.dk>, 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 10:13, 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.
> 
> What do you think?

Ok, wanted to do it rather later but let me to try fiddle with it.

btw, what's happening with smbdirect? Do you plan upstream it one day
and it's just maturing out of tree?

-- 
Pavel Begunkov