On Tue, Jun 18, 2024 at 11:40 AM Ilya Leoshkevich [off-list ref] wrote:

On Tue, 2024-06-18 at 11:24 +0200, Alexander Potapenko wrote:

quoted

On Thu, Jun 13, 2024 at 5:39 PM Ilya Leoshkevich [off-list ref]
wrote:

quoted

put_user() uses inline assembly with precise constraints, so Clang
is
in principle capable of instrumenting it automatically.
Unfortunately,
one of the constraints contains a dereferenced user pointer, and
Clang
does not currently distinguish user and kernel pointers. Therefore
KMSAN attempts to access shadow for user pointers, which is not a
right
thing to do.

An obvious fix to add __no_sanitize_memory to __put_user_fn() does
not
work, since it's __always_inline. And __always_inline cannot be
removed
due to the __put_user_bad() trick.

A different obvious fix of using the "a" instead of the "+Q"
constraint
degrades the code quality, which is very important here, since it's
a
hot path.

Instead, repurpose the __put_user_asm() macro to define
__put_user_{char,short,int,long}_noinstr() functions and mark them
with
__no_sanitize_memory. For the non-KMSAN builds make them
__always_inline in order to keep the generated code quality. Also
define __put_user_{char,short,int,long}() functions, which call the
aforementioned ones and which *are* instrumented, because they call
KMSAN hooks, which may be implemented as macros.

I am not really familiar with s390 assembly, but I think you still
need to call kmsan_copy_to_user() and kmsan_copy_from_user() to
properly initialize the copied data and report infoleaks.
Would it be possible to insert calls to linux/instrumented.h hooks
into uaccess functions?

Aren't the existing instrument_get_user() / instrument_put_user() calls
sufficient?

Oh, sorry, I overlooked them. Yes, those should be sufficient.
But you don't include linux/instrumented.h, do you?