netdev - Re: [PATCH net-next] packet: allow MSG

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: <6a4ebf7b-63c9-34b8-cff3-5b2312762972@kernel.dk>
Date:   Fri, 24 Feb 2023 07:32:30 -0700
From:   Jens Axboe <axboe@...nel.dk>
To:     Eric Dumazet <edumazet@...gle.com>,
        David Lamparter <equinox@...c24.net>
Cc:     netdev@...r.kernel.org, "David S. Miller" <davem@...emloft.net>,
        Willem de Bruijn <willemdebruijn.kernel@...il.com>
Subject: Re: [PATCH net-next] packet: allow MSG_NOSIGNAL in recvmsg

On 2/24/23 3:26 AM, Eric Dumazet wrote:
> On Fri, Feb 24, 2023 at 8:18 AM David Lamparter <equinox@...c24.net> wrote:
>>
>> packet_recvmsg() whitelists a bunch of MSG_* flags, which notably does
>> not include MSG_NOSIGNAL.  Unfortunately, io_uring always sets
>> MSG_NOSIGNAL, meaning AF_PACKET sockets can't be used in io_uring
>> recvmsg().
>>
>> As AF_PACKET sockets never generate SIGPIPE to begin with, MSG_NOSIGNAL
>> is a no-op and can simply be ignored.
>>
>> Signed-off-by: David Lamparter <equinox@...c24.net>
>> Cc: Willem de Bruijn <willemdebruijn.kernel@...il.com>
>> ---
>>  net/packet/af_packet.c | 2 +-
>>  1 file changed, 1 insertion(+), 1 deletion(-)
>>
> 
> This is odd... I think MSG_NOSIGNAL flag has a meaning for sendmsg()
> (or write sides in general)
> 
> EPIPE is not supposed to be generated at the receiving side ?
> 
> So I would rather make io_uring slightly faster :
> 
> 
> diff --git a/io_uring/net.c b/io_uring/net.c
> index cbd4b725f58c98e5bc5bf88d5707db5c8302e071..b7f190ca528e6e259eb2b072d7a16aaba98848cb
> 100644
> --- a/io_uring/net.c
> +++ b/io_uring/net.c
> @@ -567,7 +567,7 @@ int io_recvmsg_prep(struct io_kiocb *req, const
> struct io_uring_sqe *sqe)
>         sr->flags = READ_ONCE(sqe->ioprio);
>         if (sr->flags & ~(RECVMSG_FLAGS))
>                 return -EINVAL;
> -       sr->msg_flags = READ_ONCE(sqe->msg_flags) | MSG_NOSIGNAL;
> +       sr->msg_flags = READ_ONCE(sqe->msg_flags);
>         if (sr->msg_flags & MSG_DONTWAIT)
>                 req->flags |= REQ_F_NOWAIT;
>         if (sr->msg_flags & MSG_ERRQUEUE)

This looks fine to me. Do you want to send a "proper" patch for
this?

-- 
Jens Axboe