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
| ||
|
Date: Thu, 20 Oct 2022 05:59:29 -0700 From: Jens Axboe <axboe@...nel.dk> To: Pavel Begunkov <asml.silence@...il.com>, Stefan Metzmacher <metze@...ba.org>, 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 5:53 AM, Pavel Begunkov wrote: > On 10/20/22 13:49, Jens Axboe wrote: >> 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. > > As I mentioned let's drop, but if not for smb I do think it's > better as doesn't require changes in multiple /net files. I do think it's cleaner to do as a socket flag rather than hardcode it in the caller (and potentially making bad assumptions, even if the out-of-tree code is a bit of a reach for sure). -- Jens Axboe
Powered by blists - more mailing lists