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: <eb8da70b-359c-410a-b029-8bad5ba04389@bytedance.com>
Date: Wed, 24 Jul 2024 21:18:45 -0700
From: Zijian Zhang <zijianzhang@...edance.com>
To: 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

On 7/24/24 8:08 PM, Willem de Bruijn wrote:
> 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.
> 

Agree, the ABI change should be the main concern. I think I will go with
the current method firstly, and the original one as backup.

Thanks for the quick reply and clarification!