Messages in this thread | | | From | Andy Lutomirski <> | Date | Sun, 29 Mar 2020 09:50:34 -0700 | Subject | Re: [RFC][PATCH 01/22] x86 user stack frame reads: switch to explicit __get_user() |
| |
On Sun, Mar 29, 2020 at 2:26 AM Ingo Molnar <mingo@kernel.org> wrote: > > > * Al Viro <viro@zeniv.linux.org.uk> wrote: > > > > but the __get_user() API doesn't carry the 'unsafe' tag yet. > > > > > > Should we add an __unsafe_get_user() alias to it perhaps, and use it > > > in all code that adds it, like the chunk above? Or rename it to > > > __unsafe_get_user() outright? No change to the logic, but it would be > > > more obvious what code has inherited old __get_user() uses and which > > > code uses __unsafe_get_user() intentionally. > > > > > > Even after your series there's 700 uses of __get_user(), so it would > > > make sense to make a distinction in name at least and tag all unsafe > > > APIs with an 'unsafe_' prefix. > > > > "unsafe" != "lacks access_ok", it's "done under user_access_begin". > > Well, I thought the principle was that we'd mark generic APIs that had > *either* a missing access_ok() check or a missing > user_access_begin()/end() wrapping marked unsafe_*(), right? > > __get_user() has __uaccess_begin()/end() on the inside, but doesn't have > the access_ok() check, so those calls are 'unsafe' with regard to not > being safe to untrusted (ptr,size) ranges. > > I agree that all of these topics need equal attention: > > - leaking of cleared SMAP state (CLAC), which results in a silent > failure. > > - running user accesses without STAC, which results in a crash. > > - not doing an access_ok() check on untrusted (pointer,size) ranges, > which results in a silent failure as well.
My incliniation is to just get rid of the __get_user()-style APIs. There shouldn't be any __get_user() calls that can't be directly replaced by get_user(), and a single integer comparison is not that expensive. On SMAP systems, the speedup of __get_user vs get_user is negligible.
(It's possible that some arch code somewhere uses __get_user as a way to say "access user or kernel memory -- I know what I'm doing". This is crap if it exists. It better not happen in generic code because of sane architectures like s390x.)
| |