Am 28.11.20 um 20:03 schrieb Victor Stewart: > On Thu, Nov 26, 2020 at 7:36 AM Stefan Metzmacher wrote: >> >> Am 23.11.20 um 17:29 schrieb Victor Stewart: >>> On Mon, Nov 23, 2020 at 4:13 PM Stefan Metzmacher wrote: >>>> >>>> Hi Victor, >>>> >>>> wouldn't it be enough to port the PROTO_CMSG_DATA_ONLY check to the sendmsg path? >>>> >>>> UDP sockets should have PROTO_CMSG_DATA_ONLY set. >>>> >>>> I guess that would fix your current problem. >>> >>> that would definitely solve the problem and is the easiest solution. >>> >>> but PROTO_CMSG_DATA_ONLY is only set on inet_stream_ops and >>> inet6_stream_ops but dgram? >> >> I guess PROTO_CMSG_DATA_ONLY should be added also for dgram sockets. >> >> Did you intend to remove the cc for the mailing list? >> >> I think in addition to the io-uring list, cc'ing netdev@vger.kernel.org >> would also be good. > > whoops forgot to reply all. > > before I CC netdev, what does PROTO_CMSG_DATA_ONLY actually mean? I don't really know, but I guess it means that, any supported CMSG type on that socket won't do any magic depending on the process state, like fd passing with SOL_SOCKET/SCM_RIGHTS or SCM_CREDENTIALS. The CMSG buffer would just be a plain byte array, which may only reference state attached to the specific socket or packet. I'd guess that the author and/or reviewers can clarify that, let's see what they'll answer. > I didn't find a clear explanation anywhere by searching the kernel, only > that it was defined as 1 and flagged on inet_stream_ops and > inet6_stream_ops. > > there must be a reason it was not initially included for dgrams? I can't think of any difference I guess the author just tried to get add support for the specific usecase that didn't work (MSG_ZEROCOPY in this case, most likely only tested with a tcp workload): commit 583bbf0624dfd8fc45f1049be1d4980be59451ff Author: Luke Hsiao Date: Fri Aug 21 21:41:04 2020 -0700 io_uring: allow tcp ancillary data for __sys_recvmsg_sock() For TCP tx zero-copy, the kernel notifies the process of completions by queuing completion notifications on the socket error queue. This patch allows reading these notifications via recvmsg to support TCP tx zero-copy. Ancillary data was originally disallowed due to privilege escalation via io_uring's offloading of sendmsg() onto a kernel thread with kernel credentials (https://crbug.com/project-zero/1975). So, we must ensure that the socket type is one where the ancillary data types that are delivered on recvmsg are plain data (no file descriptors or values that are translated based on the identity of the calling process). This was tested by using io_uring to call recvmsg on the MSG_ERRQUEUE with tx zero-copy enabled. Before this patch, we received -EINVALID from this specific code path. After this patch, we could read tcp tx zero-copy completion notifications from the MSG_ERRQUEUE. Signed-off-by: Soheil Hassas Yeganeh Signed-off-by: Arjun Roy Acked-by: Eric Dumazet Reviewed-by: Jann Horn Reviewed-by: Jens Axboe Signed-off-by: Luke Hsiao Signed-off-by: David S. Miller > but yes if there's nothing standing in the way of adding it for > dgrams, and it covers UDP_SEGMENT and UDP_GRO then that's of course > the least friction solution here. Yes, it would avoid whitelisting new specific usecases. metze