netdev - Re: [PATCH net-next v7 1/3] sock: support copying cmsgs to the user space in sendmsg

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: <66a1c1bee3fc4_85f9929439@willemb.c.googlers.com.notmuch>
Date: Wed, 24 Jul 2024 23:08:46 -0400
From: Willem de Bruijn <willemdebruijn.kernel@...il.com>
To: Zijian Zhang <zijianzhang@...edance.com>, 
 Willem de Bruijn <willemdebruijn.kernel@...il.com>, 
 netdev@...r.kernel.org
Cc: edumazet@...gle.com, 
 cong.wang@...edance.com, 
 xiaochun.lu@...edance.com
Subject: Re: [PATCH net-next v7 1/3] sock: support copying cmsgs to the user
 space in sendmsg

Zijian Zhang wrote:
> On 7/9/24 9:40 AM, Willem de Bruijn wrote:
> > zijianzhang@ wrote:
> >> From: Zijian Zhang <zijianzhang@...edance.com>
> >>
> >> Users can pass msg_control as a placeholder to recvmsg, and get some info
> >> from the kernel upon returning of it, but it's not available for sendmsg.
> >> Recvmsg uses put_cmsg to copy info back to the user, while ____sys_sendmsg
> >> creates a kernel copy of msg_control and passes that to the callees,
> >> put_cmsg in sendmsg path will write into this kernel buffer.
> >>
> >> If users want to get info after returning of sendmsg, they typically have
> >> to call recvmsg on the ERRMSG_QUEUE of the socket, incurring extra system
> > 
> > nit: error queue or MSG_ERRQUEUE
> > 
> >> call overhead. This commit supports copying cmsg from the kernel space to
> >> the user space upon returning of sendmsg to mitigate this overhead.
> >>
> >> Signed-off-by: Zijian Zhang <zijianzhang@...edance.com>
> >> Signed-off-by: Xiaochun Lu <xiaochun.lu@...edance.com>
> > 
> > Overall this approach follows what I had in mind, thanks.
> > 
> > Looking forward to the discussion with a wider audience at netdevconf
> > next week.
> 
> 
> After wider exposure to netdev, besides the comments in this email
> series, I want to align the next step with you :)
> 
> Shall I also make this a config and add conditional compilation in the
> hot path？

At netdev there appeared to be some support for your original approach
of the application passing a user address as CMSG_DATA and the kernel
writing directly there.

That has the benefit of no modifications to net/socket.c and lower
overhead.

But there evidently hasn't been much other feedback on either approach.
Since this is an ABI change, SubmittingPatches suggests "User-space
API changes should also be copied to linux-api@...r.kernel.org." That
might give you more opinions, and is probably a good idea for
something this invasive.

If you choose to go with the current approach, a static_branch in
____sys_sendmsg would make the branch a noop in the common case.
Could be enabled on first setsockopt SO_ZEROCOPY. And never
disabled: no need for refcounting it.

Either way, no need for a CONFIG. Distros ship with defaults, so that
is what matters. And you would not want this default off.