Re: [PATCH v2 6/6] powerpc: Implement user_access_begin and friends

[Christophe Leroy]

Le 23/01/2020 Ã 13:31, Michael Ellerman a ÃcritÂ:
> Michael Ellerman <mpe@xxxxxxxxxxxxxx> writes:
> > Christophe Leroy <christophe.leroy@xxxxxx> writes:
> > > Today, when a function like strncpy_from_user() is called,
> > > the userspace access protection is de-activated and re-activated
> > > for every word read.
> > >
> > > By implementing user_access_begin and friends, the protection
> > > is de-activated at the beginning of the copy and re-activated at the
> > > end.
> > >
> > > Implement user_access_begin(), user_access_end() and
> > > unsafe_get_user(), unsafe_put_user() and unsafe_copy_to_user()
> > >
> > > For the time being, we keep user_access_save() and
> > > user_access_restore() as nops.
> >
> > That means we will run with user access enabled in a few more places, but
> > it's only used sparingly AFAICS:
> >
> >    kernel/trace/trace_branch.c:    unsigned long flags = user_access_save();
> >    lib/ubsan.c:    unsigned long flags = user_access_save();
> >    lib/ubsan.c:    unsigned long ua_flags = user_access_save();
> >    mm/kasan/common.c:      unsigned long flags = user_access_save();
> >
> > And we don't have objtool checking that user access enablement isn't
> > leaking in the first place, so I guess it's OK for us not to implement
> > these to begin with?
>
> It looks like we can implement them on on all three KUAP
> implementations.
>
> For radix and 8xx we just return/set the relevant SPR.
>
> For book3s/32/kup.h I think we'd just need to add a KUAP_CURRENT case to
> allow_user_access()?

Can't do that, we don't want to keep the info in current->thread.kuap 
after user_access_save(), otherwise we might unexpectedly re-open access 
through an interrupt.

And if we use KUAP_CURRENT case of prevent_user_access(), it means we'll 
read current->thread.kuap twice.

So, just regenerate addr and end from the flags, and use 
allow_user_access() and prevent_user_access() as usual.

I'll have it in v4

Christophe