Messages in this thread | | | Date | Thu, 3 Feb 2022 20:20:05 +0100 | From | Heiko Carstens <> | Subject | Re: [RFC PATCH 1/2] uaccess: Add mechanism for arch specific user access with argument |
| |
On Thu, Feb 03, 2022 at 07:11:40PM +0100, Janis Schoetterl-Glausch wrote: > KVM on s390 needs a mechanism to do accesses to guest memory > that honor storage key protection. > > On s390 each physical page is associated with 4 access control bits. > On access these are compared with an access key, which is either > provided by the instruction or taken from the CPU state. > Based on that comparison, the access either succeeds or is prevented. > > KVM on s390 needs to be able emulate this behavior, for example during > instruction emulation. KVM usually accesses the guest via > __copy_from/to_user, but in this case we need to also pass the access key. > Introduce __copy_from/to_user_opaque functions KVM can use to achieve > this by forwarding an architecture specific argument. > These functions are the same as their non _opaque counterparts, except > for the additional argument and also reside in include/linux/uaccess.h > so that they will not go out of sync should their counterparts change. > > Signed-off-by: Janis Schoetterl-Glausch <scgl@linux.ibm.com> > --- > include/linux/uaccess.h | 28 ++++++++++++++++++++++++++++ > 1 file changed, 28 insertions(+) > > diff --git a/include/linux/uaccess.h b/include/linux/uaccess.h > index ac0394087f7d..cc2c7c6e2b92 100644 > --- a/include/linux/uaccess.h > +++ b/include/linux/uaccess.h > @@ -114,6 +114,20 @@ __copy_from_user(void *to, const void __user *from, unsigned long n) > return raw_copy_from_user(to, from, n); > } > > +#ifdef uaccess_opaque > +static __always_inline __must_check unsigned long > +__copy_from_user_opaque(void *to, const void __user *from, unsigned long n, > + struct uaccess_opaque opaque) > +{ > + might_fault(); > + if (should_fail_usercopy()) > + return n; > + instrument_copy_from_user(to, from, n); > + check_object_size(to, n, false); > + return raw_copy_from_user_opaque(to, from, n, opaque); > +} > +#endif /* uaccess_opaque */ > + > /** > * __copy_to_user_inatomic: - Copy a block of data into user space, with less checking. > * @to: Destination address, in user space. > @@ -148,6 +162,20 @@ __copy_to_user(void __user *to, const void *from, unsigned long n) > return raw_copy_to_user(to, from, n); > } > > +#ifdef uaccess_opaque > +static __always_inline __must_check unsigned long > +__copy_to_user_opaque(void __user *to, const void *from, unsigned long n, > + struct uaccess_opaque opaque) > +{ > + might_fault(); > + if (should_fail_usercopy()) > + return n; > + instrument_copy_to_user(to, from, n); > + check_object_size(from, n, true); > + return raw_copy_to_user_opaque(to, from, n, opaque); > +} > +#endif /* uaccess_opaque */
I don't think this is acceptable for several reasons:
- we really don't want an "opaque" copy_to_user variant with completely different semantics for each architecture
- even if this would be only for s390 it is anything but obvious for the reader what the semantics of "opaque" are
- making a double underscore variant of something the regular api is really not nice
So I guess we have three options:
- add a "key" variant to common code, where the semantics are clearly that "key" is a matching access key required to access a user space page
- have this completely in s390 arch code and accept the burden (and risk) of keeping instrumentation, etc. in sync
- add some macros similar to the SYSCALL_DEFINE macros, which allow to create architecture specific copy_to/from_user variants with additional parameters.
| |