Web lists-archives.com

Re: [PATCH v2 07/19] x86: introduce __uaccess_begin_nospec and ASM_IFENCE




On Fri, Jan 12, 2018 at 11:26 AM, Dan Williams <dan.j.williams@xxxxxxxxx> wrote:
>
> By the time we get to de-reference uptr we know it is not pointing at
> kernel memory, because access_ok would have failed and the cpu would
> have waited for that failure result before doing anything else.

I'm not actually convinced that's right in the original patches,
exactly because of the issue that Josh pointed out: even if there is a
comparison inside access_ok() that will be properly serialized, then
that comparison can (and sometimes does) just cause a truth value to
be generated, and then there  might be *another* comparison of that
return value after the lfence. And while the return value is table,
the conditional branch on that comparison isn't.

The new model of just doing it together with the STAC should be fine, though.

I do think that it would be a good idea to very expressly document the
fact that it's not that the user access itself is unsafe. I do agree
that things like "get_user()" want to be protected, but not because of
any direct bugs or problems with get_user() and friends, but simply
because get_user() is an excellent source of a pointer that is
obviously controlled from a potentially attacking user space. So it's
a prime candidate for then finding _subsequent_ accesses that can then
be used to perturb the cache.

                 Linus