From: Alejandro Colomar <alx@kernel.org>
To: Matthew House <mattlloydhouse@gmail.com>
Cc: linux-man@vger.kernel.org, linux-api@vger.kernel.org,
netdev@vger.kernel.org, Ulrich Drepper <drepper@redhat.com>,
"David S. Miller" <davem@davemloft.net>,
Andrew Morton <akpm@linux-foundation.org>
Subject: Re: [PATCH v3] recv.2: Document MSG_CMSG_CLOEXEC as returned in msg_flags
Date: Tue, 18 Jul 2023 23:39:09 +0200 [thread overview]
Message-ID: <96515710-f82f-cd2a-8366-7fff84f28d82@kernel.org> (raw)
In-Reply-To: <20230718172648.943269-1-mattlloydhouse@gmail.com>
[-- Attachment #1.1: Type: text/plain, Size: 5245 bytes --]
Hi Matthew, Ulrich,
On 2023-07-18 19:26, Matthew House wrote:
> Ever since commit 4a19542e5f69 ("O_CLOEXEC for SCM_RIGHTS") added the
> MSG_CMSG_CLOEXEC flag to recvmsg(2), the flag has also been copied into the
> returned msg->msg_flags when specified, regardless of whether any file
> descriptors were actually received, or whether the protocol supports
> receiving file descriptors at all. This behavior was primarily an
> implementation artifact: by copying MSG_CMSG_CLOEXEC into the msg_flags,
> scm_detach_fds() in net/core/scm.c (and its _compat() counterpart in
> net/compat.c) could determine whether it was set without having to receive
> a copy of the recvmsg(2) flags.
>
> This mechanism was closely modeled after the internal MSG_CMSG_COMPAT flag,
> which is passed by the compat versions of the send[m]msg(2) and
> recv[m]msg(2) syscalls to inform various functions that user space expects
> a compat layout. When the flag was first implemented by commits
> 3225fc8a85f4 ("[NET]: Simplify scm handling and sendmsg/recvmsg invocation,
> consolidate net compat syscalls.") and 7e8d06bc1d90 ("[COMPAT]: Fix
> MSG_CMSG_COMPAT flag passing, kill cmsg_compat_recvmsg_fixup.") (in
> history/history.git), the behavior was very similar: recvmsg(2) would add
> MSG_CMSG_COMPAT to the msg_flags, and put_cmsg() and scm_detach_fds() in
> net/core/scm.c would read the flag to determine whether to delegate to
> their _compat() counterparts.
>
> However, after the initial implementation, more work was done to hide
> MSG_CMSG_COMPAT from user space. First, commit 37f7f421cce1 ("[NET]: Do not
> leak MSG_CMSG_COMPAT into userspace.") started scrubbing the bit from
> msg_flags right before copying it back into user space. Then, since passing
> the MSG_CMSG_COMPAT flag into the syscalls from non-compat code could
> confuse the kernel, commits 1be374a0518a ("net: Block MSG_CMSG_COMPAT in
> send(m)msg and recv(m)msg") and a7526eb5d06b ("net: Unbreak
> compat_sys_{send,recv}msg") made them return -EINVAL if user space
> attempted to pass the flag. But to reduce breakage, commit d720d8cec563
> ("net: compat: Ignore MSG_CMSG_COMPAT in compat_sys_{send, recv}msg")
> rolled that back somewhat, making MSG_CMSG_COMPAT an error for the
> non-compat syscalls and a no-op for the compat syscalls, which is the
> current status quo.
>
> Even though MSG_CMSG_CLOEXEC was implemented after the kernel started
> scrubbing MSG_CMSG_COMPAT from the returned msg_flags, the newer flag never
> received the same treatment. At this point, this behavior has effectively
> become part of the user-space API, to the extent that io_uring has been
> careful in commit 9bb66906f23e ("io_uring: support multishot in recvmsg")
> to replicate the behavior in its multishot IORING_OP_RECVMSG operation.
>
> Therefore, document this behavior to avoid confusion when user space sees
> MSG_CMSG_CLOEXEC returned in msg->msg_flags.
>
> Cc: linux-api@vger.kernel.org
> Cc: netdev@vger.kernel.org
> Cc: Ulrich Drepper <drepper@redhat.com>
> Cc: "David S. Miller" <davem@davemloft.net>
> Cc: Andrew Morton <akpm@linux-foundation.org>
> Signed-off-by: Matthew House <mattlloydhouse@gmail.com>
> ---
> Alright, I've summarized the history in the commit message, and I've added
> the CCs you requested.
>
> Also, for future reference, Drepper gave a reply to the last email, which
> did not make it onto the list:
>
> On Tue, Jul 18, 2023 at 9:24 AM Ulrich Drepper <drepper@redhat.com> wrote:
>> On Tue, Jul 18, 2023 at 2:10 PM Alejandro Colomar <alx@kernel.org> wrote:
>>
>>>> As for the original
>>>> purpose of the behavior, it's not really clear, and it may well have been
>>>> an implementation artifact that got enshrined in the user space ABI.
>>> (Even
>>>> io_uring is careful to replicate this behavior!)
>>>
>>> This is what worries me. I've CCd a bunch of people to see if they can
>>> bring some light.
>>>
>>
>> It definitely was an artifact of the implementation. I haven't tested
>> getting the close-on-exec flag information for all interfaces. The
>> assumption was that the information about the close-on-exec flag is
>> received with the universal fcntl() call.
Patch applied. I've included Drepper's quote in the commit message too.
Thank you both!
Cheers,
Alex
>
> Thank you,
> Matthew House
>
> man2/recv.2 | 9 +++++++++
> 1 file changed, 9 insertions(+)
>
> diff --git a/man2/recv.2 b/man2/recv.2
> index 660c103fb..1cd9f3e1b 100644
> --- a/man2/recv.2
> +++ b/man2/recv.2
> @@ -412,6 +412,15 @@ is returned to indicate that expedited or out-of-band data was received.
> .B MSG_ERRQUEUE
> indicates that no data was received but an extended error from the socket
> error queue.
> +.TP
> +.BR MSG_CMSG_CLOEXEC " (since Linux 2.6.23)"
> +.\" commit 4a19542e5f694cd408a32c3d9dc593ba9366e2d7
> +indicates that
> +.B MSG_CMSG_CLOEXEC
> +was specified in the
> +.I flags
> +argument of
> +.BR recvmsg ().
> .SH RETURN VALUE
> These calls return the number of bytes received, or \-1
> if an error occurred.
--
<http://www.alejandro-colomar.es/>
GPG key fingerprint: A9348594CE31283A826FBDD8D57633D441E25BB5
[-- Attachment #2: OpenPGP digital signature --]
[-- Type: application/pgp-signature, Size: 833 bytes --]
prev parent reply other threads:[~2023-07-18 21:39 UTC|newest]
Thread overview: 4+ messages / expand[flat|nested] mbox.gz Atom feed top
[not found] <20230709213358.389871-1-mattlloydhouse@gmail.com>
[not found] ` <363c0f82-969d-1927-1bd5-b664cfc83a87@kernel.org>
[not found] ` <20230716234803.851580-1-mattlloydhouse@gmail.com>
2023-07-17 23:10 ` [PATCH v2] recv.2: Document MSG_CMSG_CLOEXEC as returned in msg_flags Alejandro Colomar
[not found] ` <20230718060121.934187-1-mattlloydhouse@gmail.com>
2023-07-18 12:03 ` Alejandro Colomar
2023-07-18 17:26 ` [PATCH v3] " Matthew House
2023-07-18 21:39 ` Alejandro Colomar [this message]
Reply instructions:
You may reply publicly to this message via plain-text email
using any one of the following methods:
* Save the following mbox file, import it into your mail client,
and reply-to-all from there: mbox
Avoid top-posting and favor interleaved quoting:
https://en.wikipedia.org/wiki/Posting_style#Interleaved_style
* Reply using the --to, --cc, and --in-reply-to
switches of git-send-email(1):
git send-email \
--in-reply-to=96515710-f82f-cd2a-8366-7fff84f28d82@kernel.org \
--to=alx@kernel.org \
--cc=akpm@linux-foundation.org \
--cc=davem@davemloft.net \
--cc=drepper@redhat.com \
--cc=linux-api@vger.kernel.org \
--cc=linux-man@vger.kernel.org \
--cc=mattlloydhouse@gmail.com \
--cc=netdev@vger.kernel.org \
/path/to/YOUR_REPLY
https://kernel.org/pub/software/scm/git/docs/git-send-email.html
* If your mail client supports setting the In-Reply-To header
via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line
before the message body.
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for read-only IMAP folder(s) and NNTP newsgroup(s).