netdev - RE: [PATCH 0/5] add initial io_uring

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] [day] [month] [year] [list]
Message-ID: <9233101ff5794556a0873832ebad4445@AcuMS.aculab.com>
Date: Wed, 3 May 2023 13:27:44 +0000
From: David Laight <David.Laight@...LAB.COM>
To: 'Adrien Delorme' <delorme.ade@...look.com>, Pavel Begunkov
	<asml.silence@...il.com>
CC: "axboe@...nel.dk" <axboe@...nel.dk>, "davem@...emloft.net"
	<davem@...emloft.net>, "dccp@...r.kernel.org" <dccp@...r.kernel.org>,
	"dsahern@...nel.org" <dsahern@...nel.org>, "edumazet@...gle.com"
	<edumazet@...gle.com>, "io-uring@...r.kernel.org" <io-uring@...r.kernel.org>,
	"kuba@...nel.org" <kuba@...nel.org>, "leit@...com" <leit@...com>,
	"leitao@...ian.org" <leitao@...ian.org>, "linux-kernel@...r.kernel.org"
	<linux-kernel@...r.kernel.org>, "marcelo.leitner@...il.com"
	<marcelo.leitner@...il.com>, "matthieu.baerts@...sares.net"
	<matthieu.baerts@...sares.net>, "mptcp@...ts.linux.dev"
	<mptcp@...ts.linux.dev>, "netdev@...r.kernel.org" <netdev@...r.kernel.org>,
	"pabeni@...hat.com" <pabeni@...hat.com>, "willemb@...gle.com"
	<willemb@...gle.com>, "willemdebruijn.kernel@...il.com"
	<willemdebruijn.kernel@...il.com>
Subject: RE: [PATCH 0/5] add initial io_uring_cmd support for sockets

From: Adrien Delorme
> Sent: 03 May 2023 14:11
> 
> From Adrien Delorme
> > From : Pavel Begunkov
> > Sent : 2 May 2023 15:04
> > On 5/2/23 10:21, Adrien Delorme wrote:
> > >  From Adrien Delorme
> > >
> > >> From: David Ahern
> > >> Sent: 12 April 2023 7:39
> > >>> Sent: 11 April 2023 16:28
> > >> ....
> > >> One problem is that not all sockopt calls pass the correct length.
> > >> And some of them can have very long buffers.
> > >> Not to mention the ones that are read-modify-write.
> > >>
> > >> A plausible solution is to pass a 'fat pointer' that contains some,
> > >> or all, of:
> > >>        - A userspace buffer pointer.
> > >>        - A kernel buffer pointer.
> > >>        - The length supplied by the user.
> > >>        - The length of the kernel buffer.
> > >>        = The number of bytes to copy on completion.
> > >> For simple user requests the syscall entry/exit code would copy the
> > >> data to a short on-stack buffer.
> > >> Kernel users just pass the kernel address.
> > >> Odd requests can just use the user pointer.
> > >>
> > >> Probably needs accessors that add in an offset.
> > >>
> > >> It might also be that some of the problematic sockopt were in decnet
> > >> - now removed.
> > >
> > > Hello everyone,
> > >
> > > I'm currently working on an implementation of {get,set} sockopt.
> > > Since this thread is already talking about it, I hope that I replying at the
> > correct place.
> >
> > Hi Adrien, I believe Breno is working on set/getsockopt as well and had
> > similar patches for awhile, but that would need for some problems to be
> > solved first, e.g. try and decide whether it copies to a ptr as the syscall
> > versions or would get/return optval directly in sqe/cqe. And also where to
> > store bits that you pass in struct args_setsockopt_uring, and whether to rely
> > on SQE128 or not.
> >
> 
> Hello Pavel,
> That is good to hear. If possible I would like to provide some help.
> I looked at the getsockopt implementation. From what I'm seeing, I believe that it would be easier to
> copies to a ptr as the syscall.
> The length of the output is usually 4 bytes (sometimes less) but in a lot of cases, this length is
> variable. Sometime it can even be bigger that the SQE128 ring.
> 
> Here is a non-exhaustive list of those cases :
> /net/ipv4/tcp.c : int do_tcp_getsockopt(...)
>   - TCP_INFO : up to 240 bytes
>   - TCP_CC_INFO and TCP_REPAIR_WINDOW : up to 20 bytes
>   - TCP_CONGESTION and TCP_ULP : up to 16 bytes
>   - TCP_ZEROCPOY_RECEIVE : up to 64 bytes
> /net/atm/commun.c : int vcc_getsockopt(...)
>   - SO_ATMQOS : up to 88 bytes
>   - SO_ATMPVC : up to 16 bytes
> /net/ipv4/io_sockglue.c : int do_ip_getsockopt(...)
>   - MCAST_MSFILTER : up to 144 bytes
>   - IP_MSFILTER : 16 bytes minimum
> 
> I will look into setsockopt but I believe it might be the same.
> If needed I can also complete this list.
> However there are some cases where it is hard to determinate a maximum amount of bytes in advance.

Also look at SCTP - it has some very long buffers.
Almost any code that uses SCTP needs to use the SCTP_STATUS
request to get the negotiated number of data streams
(that one is relatively short).
IIRC there are also getsockopt() that are read/modify/write!

There will also be user code that supplies a very long buffer
(too long to allocate in kernel) for some variable length requests.

So the generic system call code can allocate a short (eg on-stack)
buffer for short requests and then pass both the user and kernel
addresses (and lengths) through to the protocol functions.
Anything that needs a big buffer can directly copy to/from
and user buffers, kernel callers would need to pass a big enough
buffer.

But the code for small buffers would be much simplified for
both kernel and user access.

	David

-
Registered Address Lakeside, Bramley Road, Mount Farm, Milton Keynes, MK1 1PT, UK
Registration No: 1397386 (Wales)