RE: [PATCH v4 0/7] kernel: introduce uaccess logging

From: Peter Collingbourne

Sent: 13 December 2021 19:49

On Sat, Dec 11, 2021 at 9:23 AM David Laight [off-list ref] wrote:

quoted

From: Peter Collingbourne

quoted

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)
_______________________________________________
linux-arm-kernel mailing list
linux-arm-kernel@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-arm-kernel