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] [thread-next>] [day] [month] [year] [list]
Date: Wed, 20 Mar 2024 11:12:23 +0100
From: Alexander Potapenko <glider@...gle.com>
To: Linus Torvalds <torvalds@...ux-foundation.org>
Cc: akpm@...ux-foundation.org, linux-kernel@...r.kernel.org, 
	linux-mm@...ck.org, kasan-dev@...glegroups.com, tglx@...utronix.de, 
	x86@...nel.org, Dmitry Vyukov <dvyukov@...gle.com>, Marco Elver <elver@...gle.com>, 
	Tetsuo Handa <penguin-kernel@...ove.sakura.ne.jp>
Subject: Re: [PATCH v1 3/3] x86: call instrumentation hooks from copy_mc.c

On Tue, Mar 19, 2024 at 6:58 PM Linus Torvalds
<torvalds@...ux-foundation.org> wrote:
>
> On Tue, 19 Mar 2024 at 09:37, Alexander Potapenko <glider@...gle.com> wrote:
> >
> >         if (copy_mc_fragile_enabled) {
> >                 __uaccess_begin();
> > +               instrument_copy_to_user(dst, src, len);
> >                 ret = copy_mc_fragile((__force void *)dst, src, len);
> >                 __uaccess_end();
>
> I'd actually prefer that instrument_copy_to_user() to be *outside* the
> __uaccess_begin.

Good point, this is doable.

>
> In fact, I'm a bit surprised that objtool didn't complain about it in that form.

This is because a bunch of KMSAN functions is ignored by objtool:
https://elixir.bootlin.com/linux/latest/source/tools/objtool/check.c#L1200

> __uaccess_begin() causes the CPU to accept kernel accesses to user
> mode, and I don't think instrument_copy_to_user() has any business
> actually touching user mode memory.

Ack.

> In fact it might be better to rename the function and change the prototype to
>
>    instrument_src(src, len);
>
> because you really can't sanely instrument the destination of a user
> copy, but "instrument_src()" might be useful in other situations than
> just user copies.

Right now at least for KMSAN it is important to distinguish between a
usercopy and e.g. a URB submission: both are checked using the same
function, but depending on what is happening the report title is
different.

The destination parameter is also used by KMSAN to print fancier error reports.
For an infoleak we show the target userspace address together with
other information, e.g.:

  BUG: KMSAN: kernel-infoleak in instrument_copy_to_user
include/linux/instrumented.h:114 [inline]
  ...
  Bytes 34-35 of 36 are uninitialized
  Memory access of size 36 starts at ffff8881152e5680
  Data copied to user address 00007ffc9a4a12a0

It comes in handy when debugging reproducers locally.

Future debugging tools may also need more insight into the semantics
of the instrumented accesses.

Powered by blists - more mailing lists

Powered by Openwall GNU/*/Linux Powered by OpenVZ