Messages in this thread | | | From | David Laight <> | Subject | RE: [PATCH v4 0/7] kernel: introduce uaccess logging | Date | Mon, 13 Dec 2021 23:07:42 +0000 |
| |
From: Peter Collingbourne > Sent: 13 December 2021 19:49 > > On Sat, Dec 11, 2021 at 9:23 AM David Laight <David.Laight@aculab.com> wrote: > > > > From: Peter Collingbourne > > > Sent: 09 December 2021 22:16 > > > > > > This patch series introduces a kernel feature known as uaccess > > > logging, which allows userspace programs to be made aware of the > > > address and size of uaccesses performed by the kernel during > > > the servicing of a syscall. More details on the motivation > > > for and interface to this feature are available in the file > > > Documentation/admin-guide/uaccess-logging.rst added by the final > > > patch in the series. > > > > How does this work when get_user() and put_user() are used to > > do optimised copies? > > > > While adding checks to copy_to/from_user() is going to have > > a measurable performance impact - even if nothing is done, > > adding them to get/put_user() (and friends) is going to > > make some hot paths really slow. > > > > So maybe you could add it so KASAN test kernels, but you can't > > sensibly enable it on a production kernel. > > > > Now, it might be that you could semi-sensibly log 'data' transfers. > > But have you actually looked at all the transfers that happen > > for something like sendmsg(). > > The 'user copy hardening' code already has a significant impact > > on that code (in many places). > > Hi David, > > Yes, I realised after I sent out my patch (and while writing test > cases for it) that it didn't cover get_user()/put_user(). I have a > patch under development that will add this coverage. I used it to run > my invalid syscall and uname benchmarks and the results were basically > the same as without the coverage. > > Are you aware of any benchmarks that cover sendmsg()? I can try to > look at writing my own if not. I was also planning to write a > benchmark that uses getresuid() as this was the simplest syscall that > I could find that does multiple put_user() calls.
Also look at sys_poll() I think that uses __put/get_user().
I think you'll find some of the socket option code also uses get_user().
There is also the compat code for import_iovec(). IIRC that is actually faster than the non-compat version at the moment.
I did some benchmarking of writev("/dev/null", iov, 10); The cost of reading in the iovec is significant in that case. Maybe I ought to find time to sort out my patches.
For sendmsg() using __copy_from_user() to avoid the user-copy hardening checks also makes a measurable difference when sending UDP through raw sockets - which we do a lot of.
I think you'd need to instrument user_access_begin() and also be able to merge trace entries (for multiple get_user() calls).
You really don't have to look far to find places where copy_to/from_user() is optimised to multiple get/put_user() or __get/put_user() (or are they the 'nofault' variants?) Those are all hot paths - at least for some workloads. So adding anything there isn't likely to be accepted for production kernels.
David
- Registered Address Lakeside, Bramley Road, Mount Farm, Milton Keynes, MK1 1PT, UK Registration No: 1397386 (Wales)
| |