2022-02-08 15:43:12

by Janis Schoetterl-Glausch

[permalink] [raw]
Subject: [PATCH v2 01/11] s390/uaccess: Add copy_from/to_user_key functions

Add copy_from/to_user_key functions, which perform storage key checking.
These functions can be used by KVM for emulating instructions that need
to be key checked.
These functions differ from their non _key counterparts in
include/linux/uaccess.h only in the additional key argument and must be
kept in sync with those.

Since the existing uaccess implementation on s390 makes use of move
instructions that support having an additional access key supplied,
we can implement raw_copy_from/to_user_key by enhancing the
existing implementation.

Signed-off-by: Janis Schoetterl-Glausch <[email protected]>
---
arch/s390/include/asm/uaccess.h | 22 +++++++++
arch/s390/lib/uaccess.c | 81 +++++++++++++++++++++++++--------
2 files changed, 85 insertions(+), 18 deletions(-)

diff --git a/arch/s390/include/asm/uaccess.h b/arch/s390/include/asm/uaccess.h
index d74e26b48604..ba1bcb91af95 100644
--- a/arch/s390/include/asm/uaccess.h
+++ b/arch/s390/include/asm/uaccess.h
@@ -44,6 +44,28 @@ raw_copy_to_user(void __user *to, const void *from, unsigned long n);
#define INLINE_COPY_TO_USER
#endif

+unsigned long __must_check
+_copy_from_user_key(void *to, const void __user *from, unsigned long n, unsigned long key);
+
+static __always_inline unsigned long __must_check
+copy_from_user_key(void *to, const void __user *from, unsigned long n, unsigned long key)
+{
+ if (likely(check_copy_size(to, n, false)))
+ n = _copy_from_user_key(to, from, n, key);
+ return n;
+}
+
+unsigned long __must_check
+_copy_to_user_key(void __user *to, const void *from, unsigned long n, unsigned long key);
+
+static __always_inline unsigned long __must_check
+copy_to_user_key(void __user *to, const void *from, unsigned long n, unsigned long key)
+{
+ if (likely(check_copy_size(from, n, true)))
+ n = _copy_to_user_key(to, from, n, key);
+ return n;
+}
+
int __put_user_bad(void) __attribute__((noreturn));
int __get_user_bad(void) __attribute__((noreturn));

diff --git a/arch/s390/lib/uaccess.c b/arch/s390/lib/uaccess.c
index 8a5d21461889..b709239feb5d 100644
--- a/arch/s390/lib/uaccess.c
+++ b/arch/s390/lib/uaccess.c
@@ -59,11 +59,13 @@ static inline int copy_with_mvcos(void)
#endif

static inline unsigned long copy_from_user_mvcos(void *x, const void __user *ptr,
- unsigned long size)
+ unsigned long size, unsigned long key)
{
unsigned long tmp1, tmp2;
union oac spec = {
+ .oac2.key = key,
.oac2.as = PSW_BITS_AS_SECONDARY,
+ .oac2.k = 1,
.oac2.a = 1,
};

@@ -94,19 +96,19 @@ static inline unsigned long copy_from_user_mvcos(void *x, const void __user *ptr
}

static inline unsigned long copy_from_user_mvcp(void *x, const void __user *ptr,
- unsigned long size)
+ unsigned long size, unsigned long key)
{
unsigned long tmp1, tmp2;

tmp1 = -256UL;
asm volatile(
" sacf 0\n"
- "0: mvcp 0(%0,%2),0(%1),%3\n"
+ "0: mvcp 0(%0,%2),0(%1),%[key]\n"
"7: jz 5f\n"
"1: algr %0,%3\n"
" la %1,256(%1)\n"
" la %2,256(%2)\n"
- "2: mvcp 0(%0,%2),0(%1),%3\n"
+ "2: mvcp 0(%0,%2),0(%1),%[key]\n"
"8: jnz 1b\n"
" j 5f\n"
"3: la %4,255(%1)\n" /* %4 = ptr + 255 */
@@ -115,7 +117,7 @@ static inline unsigned long copy_from_user_mvcp(void *x, const void __user *ptr,
" slgr %4,%1\n"
" clgr %0,%4\n" /* copy crosses next page boundary? */
" jnh 6f\n"
- "4: mvcp 0(%4,%2),0(%1),%3\n"
+ "4: mvcp 0(%4,%2),0(%1),%[key]\n"
"9: slgr %0,%4\n"
" j 6f\n"
"5: slgr %0,%0\n"
@@ -123,24 +125,49 @@ static inline unsigned long copy_from_user_mvcp(void *x, const void __user *ptr,
EX_TABLE(0b,3b) EX_TABLE(2b,3b) EX_TABLE(4b,6b)
EX_TABLE(7b,3b) EX_TABLE(8b,3b) EX_TABLE(9b,6b)
: "+a" (size), "+a" (ptr), "+a" (x), "+a" (tmp1), "=a" (tmp2)
- : : "cc", "memory");
+ : [key] "d" (key << 4)
+ : "cc", "memory");
return size;
}

-unsigned long raw_copy_from_user(void *to, const void __user *from, unsigned long n)
+static unsigned long raw_copy_from_user_key(void *to, const void __user *from,
+ unsigned long n, unsigned long key)
{
if (copy_with_mvcos())
- return copy_from_user_mvcos(to, from, n);
- return copy_from_user_mvcp(to, from, n);
+ return copy_from_user_mvcos(to, from, n, key);
+ return copy_from_user_mvcp(to, from, n, key);
+}
+
+unsigned long raw_copy_from_user(void *to, const void __user *from, unsigned long n)
+{
+ return raw_copy_from_user_key(to, from, n, 0);
}
EXPORT_SYMBOL(raw_copy_from_user);

+unsigned long _copy_from_user_key(void *to, const void __user *from,
+ unsigned long n, unsigned long key)
+{
+ unsigned long res = n;
+
+ might_fault();
+ if (!should_fail_usercopy()) {
+ instrument_copy_from_user(to, from, n);
+ res = raw_copy_from_user_key(to, from, n, key);
+ }
+ if (unlikely(res))
+ memset(to + (n - res), 0, res);
+ return res;
+}
+EXPORT_SYMBOL(_copy_from_user_key);
+
static inline unsigned long copy_to_user_mvcos(void __user *ptr, const void *x,
- unsigned long size)
+ unsigned long size, unsigned long key)
{
unsigned long tmp1, tmp2;
union oac spec = {
+ .oac1.key = key,
.oac1.as = PSW_BITS_AS_SECONDARY,
+ .oac1.k = 1,
.oac1.a = 1,
};

@@ -171,19 +198,19 @@ static inline unsigned long copy_to_user_mvcos(void __user *ptr, const void *x,
}

static inline unsigned long copy_to_user_mvcs(void __user *ptr, const void *x,
- unsigned long size)
+ unsigned long size, unsigned long key)
{
unsigned long tmp1, tmp2;

tmp1 = -256UL;
asm volatile(
" sacf 0\n"
- "0: mvcs 0(%0,%1),0(%2),%3\n"
+ "0: mvcs 0(%0,%1),0(%2),%[key]\n"
"7: jz 5f\n"
"1: algr %0,%3\n"
" la %1,256(%1)\n"
" la %2,256(%2)\n"
- "2: mvcs 0(%0,%1),0(%2),%3\n"
+ "2: mvcs 0(%0,%1),0(%2),%[key]\n"
"8: jnz 1b\n"
" j 5f\n"
"3: la %4,255(%1)\n" /* %4 = ptr + 255 */
@@ -192,7 +219,7 @@ static inline unsigned long copy_to_user_mvcs(void __user *ptr, const void *x,
" slgr %4,%1\n"
" clgr %0,%4\n" /* copy crosses next page boundary? */
" jnh 6f\n"
- "4: mvcs 0(%4,%1),0(%2),%3\n"
+ "4: mvcs 0(%4,%1),0(%2),%[key]\n"
"9: slgr %0,%4\n"
" j 6f\n"
"5: slgr %0,%0\n"
@@ -200,18 +227,36 @@ static inline unsigned long copy_to_user_mvcs(void __user *ptr, const void *x,
EX_TABLE(0b,3b) EX_TABLE(2b,3b) EX_TABLE(4b,6b)
EX_TABLE(7b,3b) EX_TABLE(8b,3b) EX_TABLE(9b,6b)
: "+a" (size), "+a" (ptr), "+a" (x), "+a" (tmp1), "=a" (tmp2)
- : : "cc", "memory");
+ : [key] "d" (key << 4)
+ : "cc", "memory");
return size;
}

-unsigned long raw_copy_to_user(void __user *to, const void *from, unsigned long n)
+static unsigned long raw_copy_to_user_key(void __user *to, const void *from,
+ unsigned long n, unsigned long key)
{
if (copy_with_mvcos())
- return copy_to_user_mvcos(to, from, n);
- return copy_to_user_mvcs(to, from, n);
+ return copy_to_user_mvcos(to, from, n, key);
+ return copy_to_user_mvcs(to, from, n, key);
+}
+
+unsigned long raw_copy_to_user(void __user *to, const void *from, unsigned long n)
+{
+ return raw_copy_to_user_key(to, from, n, 0);
}
EXPORT_SYMBOL(raw_copy_to_user);

+unsigned long _copy_to_user_key(void __user *to, const void *from,
+ unsigned long n, unsigned long key)
+{
+ might_fault();
+ if (should_fail_usercopy())
+ return n;
+ instrument_copy_to_user(to, from, n);
+ return raw_copy_to_user_key(to, from, n, key);
+}
+EXPORT_SYMBOL(_copy_to_user_key);
+
static inline unsigned long clear_user_mvcos(void __user *to, unsigned long size)
{
unsigned long tmp1, tmp2;
--
2.32.0



2022-02-09 09:52:39

by Heiko Carstens

[permalink] [raw]
Subject: Re: [PATCH v2 01/11] s390/uaccess: Add copy_from/to_user_key functions

On Mon, Feb 07, 2022 at 05:59:20PM +0100, Janis Schoetterl-Glausch wrote:
> Add copy_from/to_user_key functions, which perform storage key checking.
> These functions can be used by KVM for emulating instructions that need
> to be key checked.
> These functions differ from their non _key counterparts in
> include/linux/uaccess.h only in the additional key argument and must be
> kept in sync with those.
>
> Since the existing uaccess implementation on s390 makes use of move
> instructions that support having an additional access key supplied,
> we can implement raw_copy_from/to_user_key by enhancing the
> existing implementation.
>
> Signed-off-by: Janis Schoetterl-Glausch <[email protected]>
> ---
> arch/s390/include/asm/uaccess.h | 22 +++++++++
> arch/s390/lib/uaccess.c | 81 +++++++++++++++++++++++++--------
> 2 files changed, 85 insertions(+), 18 deletions(-)

Acked-by: Heiko Carstens <[email protected]>

Christian, Janosch, I think this can go via the kvm tree.

2022-02-09 10:08:22

by Christian Borntraeger

[permalink] [raw]
Subject: Re: [PATCH v2 01/11] s390/uaccess: Add copy_from/to_user_key functions

Am 07.02.22 um 17:59 schrieb Janis Schoetterl-Glausch:
> Add copy_from/to_user_key functions, which perform storage key checking.
> These functions can be used by KVM for emulating instructions that need
> to be key checked.
> These functions differ from their non _key counterparts in
> include/linux/uaccess.h only in the additional key argument and must be
> kept in sync with those.
>
> Since the existing uaccess implementation on s390 makes use of move
> instructions that support having an additional access key supplied,
> we can implement raw_copy_from/to_user_key by enhancing the
> existing implementation.
>
> Signed-off-by: Janis Schoetterl-Glausch <[email protected]>

Reviewed-by: Christian Borntraeger <[email protected]>

> ---
> arch/s390/include/asm/uaccess.h | 22 +++++++++
> arch/s390/lib/uaccess.c | 81 +++++++++++++++++++++++++--------
> 2 files changed, 85 insertions(+), 18 deletions(-)
>
> diff --git a/arch/s390/include/asm/uaccess.h b/arch/s390/include/asm/uaccess.h
> index d74e26b48604..ba1bcb91af95 100644
> --- a/arch/s390/include/asm/uaccess.h
> +++ b/arch/s390/include/asm/uaccess.h
> @@ -44,6 +44,28 @@ raw_copy_to_user(void __user *to, const void *from, unsigned long n);
> #define INLINE_COPY_TO_USER
> #endif
>
> +unsigned long __must_check
> +_copy_from_user_key(void *to, const void __user *from, unsigned long n, unsigned long key);
> +
> +static __always_inline unsigned long __must_check
> +copy_from_user_key(void *to, const void __user *from, unsigned long n, unsigned long key)
> +{
> + if (likely(check_copy_size(to, n, false)))
> + n = _copy_from_user_key(to, from, n, key);
> + return n;
> +}
> +
> +unsigned long __must_check
> +_copy_to_user_key(void __user *to, const void *from, unsigned long n, unsigned long key);
> +
> +static __always_inline unsigned long __must_check
> +copy_to_user_key(void __user *to, const void *from, unsigned long n, unsigned long key)
> +{
> + if (likely(check_copy_size(from, n, true)))
> + n = _copy_to_user_key(to, from, n, key);
> + return n;
> +}
> +
> int __put_user_bad(void) __attribute__((noreturn));
> int __get_user_bad(void) __attribute__((noreturn));
>
> diff --git a/arch/s390/lib/uaccess.c b/arch/s390/lib/uaccess.c
> index 8a5d21461889..b709239feb5d 100644
> --- a/arch/s390/lib/uaccess.c
> +++ b/arch/s390/lib/uaccess.c
> @@ -59,11 +59,13 @@ static inline int copy_with_mvcos(void)
> #endif
>
> static inline unsigned long copy_from_user_mvcos(void *x, const void __user *ptr,
> - unsigned long size)
> + unsigned long size, unsigned long key)
> {
> unsigned long tmp1, tmp2;
> union oac spec = {
> + .oac2.key = key,
> .oac2.as = PSW_BITS_AS_SECONDARY,
> + .oac2.k = 1,
> .oac2.a = 1,
> };
>
> @@ -94,19 +96,19 @@ static inline unsigned long copy_from_user_mvcos(void *x, const void __user *ptr
> }
>
> static inline unsigned long copy_from_user_mvcp(void *x, const void __user *ptr,
> - unsigned long size)
> + unsigned long size, unsigned long key)
> {
> unsigned long tmp1, tmp2;
>
> tmp1 = -256UL;
> asm volatile(
> " sacf 0\n"
> - "0: mvcp 0(%0,%2),0(%1),%3\n"
> + "0: mvcp 0(%0,%2),0(%1),%[key]\n"
> "7: jz 5f\n"
> "1: algr %0,%3\n"
> " la %1,256(%1)\n"
> " la %2,256(%2)\n"
> - "2: mvcp 0(%0,%2),0(%1),%3\n"
> + "2: mvcp 0(%0,%2),0(%1),%[key]\n"
> "8: jnz 1b\n"
> " j 5f\n"
> "3: la %4,255(%1)\n" /* %4 = ptr + 255 */
> @@ -115,7 +117,7 @@ static inline unsigned long copy_from_user_mvcp(void *x, const void __user *ptr,
> " slgr %4,%1\n"
> " clgr %0,%4\n" /* copy crosses next page boundary? */
> " jnh 6f\n"
> - "4: mvcp 0(%4,%2),0(%1),%3\n"
> + "4: mvcp 0(%4,%2),0(%1),%[key]\n"
> "9: slgr %0,%4\n"
> " j 6f\n"
> "5: slgr %0,%0\n"
> @@ -123,24 +125,49 @@ static inline unsigned long copy_from_user_mvcp(void *x, const void __user *ptr,
> EX_TABLE(0b,3b) EX_TABLE(2b,3b) EX_TABLE(4b,6b)
> EX_TABLE(7b,3b) EX_TABLE(8b,3b) EX_TABLE(9b,6b)
> : "+a" (size), "+a" (ptr), "+a" (x), "+a" (tmp1), "=a" (tmp2)
> - : : "cc", "memory");
> + : [key] "d" (key << 4)
> + : "cc", "memory");
> return size;
> }
>
> -unsigned long raw_copy_from_user(void *to, const void __user *from, unsigned long n)
> +static unsigned long raw_copy_from_user_key(void *to, const void __user *from,
> + unsigned long n, unsigned long key)
> {
> if (copy_with_mvcos())
> - return copy_from_user_mvcos(to, from, n);
> - return copy_from_user_mvcp(to, from, n);
> + return copy_from_user_mvcos(to, from, n, key);
> + return copy_from_user_mvcp(to, from, n, key);
> +}
> +
> +unsigned long raw_copy_from_user(void *to, const void __user *from, unsigned long n)
> +{
> + return raw_copy_from_user_key(to, from, n, 0);
> }
> EXPORT_SYMBOL(raw_copy_from_user);
>
> +unsigned long _copy_from_user_key(void *to, const void __user *from,
> + unsigned long n, unsigned long key)
> +{
> + unsigned long res = n;
> +
> + might_fault();
> + if (!should_fail_usercopy()) {
> + instrument_copy_from_user(to, from, n);
> + res = raw_copy_from_user_key(to, from, n, key);
> + }
> + if (unlikely(res))
> + memset(to + (n - res), 0, res);
> + return res;
> +}
> +EXPORT_SYMBOL(_copy_from_user_key);
> +
> static inline unsigned long copy_to_user_mvcos(void __user *ptr, const void *x,
> - unsigned long size)
> + unsigned long size, unsigned long key)
> {
> unsigned long tmp1, tmp2;
> union oac spec = {
> + .oac1.key = key,
> .oac1.as = PSW_BITS_AS_SECONDARY,
> + .oac1.k = 1,
> .oac1.a = 1,
> };
>
> @@ -171,19 +198,19 @@ static inline unsigned long copy_to_user_mvcos(void __user *ptr, const void *x,
> }
>
> static inline unsigned long copy_to_user_mvcs(void __user *ptr, const void *x,
> - unsigned long size)
> + unsigned long size, unsigned long key)
> {
> unsigned long tmp1, tmp2;
>
> tmp1 = -256UL;
> asm volatile(
> " sacf 0\n"
> - "0: mvcs 0(%0,%1),0(%2),%3\n"
> + "0: mvcs 0(%0,%1),0(%2),%[key]\n"
> "7: jz 5f\n"
> "1: algr %0,%3\n"
> " la %1,256(%1)\n"
> " la %2,256(%2)\n"
> - "2: mvcs 0(%0,%1),0(%2),%3\n"
> + "2: mvcs 0(%0,%1),0(%2),%[key]\n"
> "8: jnz 1b\n"
> " j 5f\n"
> "3: la %4,255(%1)\n" /* %4 = ptr + 255 */
> @@ -192,7 +219,7 @@ static inline unsigned long copy_to_user_mvcs(void __user *ptr, const void *x,
> " slgr %4,%1\n"
> " clgr %0,%4\n" /* copy crosses next page boundary? */
> " jnh 6f\n"
> - "4: mvcs 0(%4,%1),0(%2),%3\n"
> + "4: mvcs 0(%4,%1),0(%2),%[key]\n"
> "9: slgr %0,%4\n"
> " j 6f\n"
> "5: slgr %0,%0\n"
> @@ -200,18 +227,36 @@ static inline unsigned long copy_to_user_mvcs(void __user *ptr, const void *x,
> EX_TABLE(0b,3b) EX_TABLE(2b,3b) EX_TABLE(4b,6b)
> EX_TABLE(7b,3b) EX_TABLE(8b,3b) EX_TABLE(9b,6b)
> : "+a" (size), "+a" (ptr), "+a" (x), "+a" (tmp1), "=a" (tmp2)
> - : : "cc", "memory");
> + : [key] "d" (key << 4)
> + : "cc", "memory");
> return size;
> }
>
> -unsigned long raw_copy_to_user(void __user *to, const void *from, unsigned long n)
> +static unsigned long raw_copy_to_user_key(void __user *to, const void *from,
> + unsigned long n, unsigned long key)
> {
> if (copy_with_mvcos())
> - return copy_to_user_mvcos(to, from, n);
> - return copy_to_user_mvcs(to, from, n);
> + return copy_to_user_mvcos(to, from, n, key);
> + return copy_to_user_mvcs(to, from, n, key);
> +}
> +
> +unsigned long raw_copy_to_user(void __user *to, const void *from, unsigned long n)
> +{
> + return raw_copy_to_user_key(to, from, n, 0);
> }
> EXPORT_SYMBOL(raw_copy_to_user);
>
> +unsigned long _copy_to_user_key(void __user *to, const void *from,
> + unsigned long n, unsigned long key)
> +{
> + might_fault();
> + if (should_fail_usercopy())
> + return n;
> + instrument_copy_to_user(to, from, n);
> + return raw_copy_to_user_key(to, from, n, key);
> +}
> +EXPORT_SYMBOL(_copy_to_user_key);
> +
> static inline unsigned long clear_user_mvcos(void __user *to, unsigned long size)
> {
> unsigned long tmp1, tmp2;

2022-02-09 10:24:22

by Janosch Frank

[permalink] [raw]
Subject: Re: [PATCH v2 01/11] s390/uaccess: Add copy_from/to_user_key functions

On 2/7/22 17:59, Janis Schoetterl-Glausch wrote:
> Add copy_from/to_user_key functions, which perform storage key checking.
> These functions can be used by KVM for emulating instructions that need
> to be key checked.
> These functions differ from their non _key counterparts in
> include/linux/uaccess.h only in the additional key argument and must be
> kept in sync with those.
>
> Since the existing uaccess implementation on s390 makes use of move
> instructions that support having an additional access key supplied,
> we can implement raw_copy_from/to_user_key by enhancing the
> existing implementation.
>
> Signed-off-by: Janis Schoetterl-Glausch <[email protected]>

Acked-by: Janosch Frank <[email protected]>


> ---
> arch/s390/include/asm/uaccess.h | 22 +++++++++
> arch/s390/lib/uaccess.c | 81 +++++++++++++++++++++++++--------
> 2 files changed, 85 insertions(+), 18 deletions(-)
>
> diff --git a/arch/s390/include/asm/uaccess.h b/arch/s390/include/asm/uaccess.h
> index d74e26b48604..ba1bcb91af95 100644
> --- a/arch/s390/include/asm/uaccess.h
> +++ b/arch/s390/include/asm/uaccess.h
> @@ -44,6 +44,28 @@ raw_copy_to_user(void __user *to, const void *from, unsigned long n);
> #define INLINE_COPY_TO_USER
> #endif
>
> +unsigned long __must_check
> +_copy_from_user_key(void *to, const void __user *from, unsigned long n, unsigned long key);
> +
> +static __always_inline unsigned long __must_check
> +copy_from_user_key(void *to, const void __user *from, unsigned long n, unsigned long key)
> +{
> + if (likely(check_copy_size(to, n, false)))
> + n = _copy_from_user_key(to, from, n, key);
> + return n;
> +}
> +
> +unsigned long __must_check
> +_copy_to_user_key(void __user *to, const void *from, unsigned long n, unsigned long key);
> +
> +static __always_inline unsigned long __must_check
> +copy_to_user_key(void __user *to, const void *from, unsigned long n, unsigned long key)
> +{
> + if (likely(check_copy_size(from, n, true)))
> + n = _copy_to_user_key(to, from, n, key);
> + return n;
> +}
> +
> int __put_user_bad(void) __attribute__((noreturn));
> int __get_user_bad(void) __attribute__((noreturn));
>
> diff --git a/arch/s390/lib/uaccess.c b/arch/s390/lib/uaccess.c
> index 8a5d21461889..b709239feb5d 100644
> --- a/arch/s390/lib/uaccess.c
> +++ b/arch/s390/lib/uaccess.c
> @@ -59,11 +59,13 @@ static inline int copy_with_mvcos(void)
> #endif
>
> static inline unsigned long copy_from_user_mvcos(void *x, const void __user *ptr,
> - unsigned long size)
> + unsigned long size, unsigned long key)
> {
> unsigned long tmp1, tmp2;
> union oac spec = {
> + .oac2.key = key,
> .oac2.as = PSW_BITS_AS_SECONDARY,
> + .oac2.k = 1,
> .oac2.a = 1,
> };
>
> @@ -94,19 +96,19 @@ static inline unsigned long copy_from_user_mvcos(void *x, const void __user *ptr
> }
>
> static inline unsigned long copy_from_user_mvcp(void *x, const void __user *ptr,
> - unsigned long size)
> + unsigned long size, unsigned long key)
> {
> unsigned long tmp1, tmp2;
>
> tmp1 = -256UL;
> asm volatile(
> " sacf 0\n"
> - "0: mvcp 0(%0,%2),0(%1),%3\n"
> + "0: mvcp 0(%0,%2),0(%1),%[key]\n"
> "7: jz 5f\n"
> "1: algr %0,%3\n"
> " la %1,256(%1)\n"
> " la %2,256(%2)\n"
> - "2: mvcp 0(%0,%2),0(%1),%3\n"
> + "2: mvcp 0(%0,%2),0(%1),%[key]\n"
> "8: jnz 1b\n"
> " j 5f\n"
> "3: la %4,255(%1)\n" /* %4 = ptr + 255 */
> @@ -115,7 +117,7 @@ static inline unsigned long copy_from_user_mvcp(void *x, const void __user *ptr,
> " slgr %4,%1\n"
> " clgr %0,%4\n" /* copy crosses next page boundary? */
> " jnh 6f\n"
> - "4: mvcp 0(%4,%2),0(%1),%3\n"
> + "4: mvcp 0(%4,%2),0(%1),%[key]\n"
> "9: slgr %0,%4\n"
> " j 6f\n"
> "5: slgr %0,%0\n"
> @@ -123,24 +125,49 @@ static inline unsigned long copy_from_user_mvcp(void *x, const void __user *ptr,
> EX_TABLE(0b,3b) EX_TABLE(2b,3b) EX_TABLE(4b,6b)
> EX_TABLE(7b,3b) EX_TABLE(8b,3b) EX_TABLE(9b,6b)
> : "+a" (size), "+a" (ptr), "+a" (x), "+a" (tmp1), "=a" (tmp2)
> - : : "cc", "memory");
> + : [key] "d" (key << 4)
> + : "cc", "memory");
> return size;
> }
>
> -unsigned long raw_copy_from_user(void *to, const void __user *from, unsigned long n)
> +static unsigned long raw_copy_from_user_key(void *to, const void __user *from,
> + unsigned long n, unsigned long key)
> {
> if (copy_with_mvcos())
> - return copy_from_user_mvcos(to, from, n);
> - return copy_from_user_mvcp(to, from, n);
> + return copy_from_user_mvcos(to, from, n, key);
> + return copy_from_user_mvcp(to, from, n, key);
> +}
> +
> +unsigned long raw_copy_from_user(void *to, const void __user *from, unsigned long n)
> +{
> + return raw_copy_from_user_key(to, from, n, 0);
> }
> EXPORT_SYMBOL(raw_copy_from_user);
>
> +unsigned long _copy_from_user_key(void *to, const void __user *from,
> + unsigned long n, unsigned long key)
> +{
> + unsigned long res = n;
> +
> + might_fault();
> + if (!should_fail_usercopy()) {
> + instrument_copy_from_user(to, from, n);
> + res = raw_copy_from_user_key(to, from, n, key);
> + }
> + if (unlikely(res))
> + memset(to + (n - res), 0, res);
> + return res;
> +}
> +EXPORT_SYMBOL(_copy_from_user_key);
> +
> static inline unsigned long copy_to_user_mvcos(void __user *ptr, const void *x,
> - unsigned long size)
> + unsigned long size, unsigned long key)
> {
> unsigned long tmp1, tmp2;
> union oac spec = {
> + .oac1.key = key,
> .oac1.as = PSW_BITS_AS_SECONDARY,
> + .oac1.k = 1,
> .oac1.a = 1,
> };
>
> @@ -171,19 +198,19 @@ static inline unsigned long copy_to_user_mvcos(void __user *ptr, const void *x,
> }
>
> static inline unsigned long copy_to_user_mvcs(void __user *ptr, const void *x,
> - unsigned long size)
> + unsigned long size, unsigned long key)
> {
> unsigned long tmp1, tmp2;
>
> tmp1 = -256UL;
> asm volatile(
> " sacf 0\n"
> - "0: mvcs 0(%0,%1),0(%2),%3\n"
> + "0: mvcs 0(%0,%1),0(%2),%[key]\n"
> "7: jz 5f\n"
> "1: algr %0,%3\n"
> " la %1,256(%1)\n"
> " la %2,256(%2)\n"
> - "2: mvcs 0(%0,%1),0(%2),%3\n"
> + "2: mvcs 0(%0,%1),0(%2),%[key]\n"
> "8: jnz 1b\n"
> " j 5f\n"
> "3: la %4,255(%1)\n" /* %4 = ptr + 255 */
> @@ -192,7 +219,7 @@ static inline unsigned long copy_to_user_mvcs(void __user *ptr, const void *x,
> " slgr %4,%1\n"
> " clgr %0,%4\n" /* copy crosses next page boundary? */
> " jnh 6f\n"
> - "4: mvcs 0(%4,%1),0(%2),%3\n"
> + "4: mvcs 0(%4,%1),0(%2),%[key]\n"
> "9: slgr %0,%4\n"
> " j 6f\n"
> "5: slgr %0,%0\n"
> @@ -200,18 +227,36 @@ static inline unsigned long copy_to_user_mvcs(void __user *ptr, const void *x,
> EX_TABLE(0b,3b) EX_TABLE(2b,3b) EX_TABLE(4b,6b)
> EX_TABLE(7b,3b) EX_TABLE(8b,3b) EX_TABLE(9b,6b)
> : "+a" (size), "+a" (ptr), "+a" (x), "+a" (tmp1), "=a" (tmp2)
> - : : "cc", "memory");
> + : [key] "d" (key << 4)
> + : "cc", "memory");
> return size;
> }
>
> -unsigned long raw_copy_to_user(void __user *to, const void *from, unsigned long n)
> +static unsigned long raw_copy_to_user_key(void __user *to, const void *from,
> + unsigned long n, unsigned long key)
> {
> if (copy_with_mvcos())
> - return copy_to_user_mvcos(to, from, n);
> - return copy_to_user_mvcs(to, from, n);
> + return copy_to_user_mvcos(to, from, n, key);
> + return copy_to_user_mvcs(to, from, n, key);
> +}
> +
> +unsigned long raw_copy_to_user(void __user *to, const void *from, unsigned long n)
> +{
> + return raw_copy_to_user_key(to, from, n, 0);
> }
> EXPORT_SYMBOL(raw_copy_to_user);
>
> +unsigned long _copy_to_user_key(void __user *to, const void *from,
> + unsigned long n, unsigned long key)
> +{
> + might_fault();
> + if (should_fail_usercopy())
> + return n;
> + instrument_copy_to_user(to, from, n);
> + return raw_copy_to_user_key(to, from, n, key);
> +}
> +EXPORT_SYMBOL(_copy_to_user_key);
> +
> static inline unsigned long clear_user_mvcos(void __user *to, unsigned long size)
> {
> unsigned long tmp1, tmp2;


2022-02-09 12:02:01

by Christian Borntraeger

[permalink] [raw]
Subject: Re: [PATCH v2 01/11] s390/uaccess: Add copy_from/to_user_key functions



Am 08.02.22 um 13:31 schrieb Christian Borntraeger:
> Am 07.02.22 um 17:59 schrieb Janis Schoetterl-Glausch:
>> Add copy_from/to_user_key functions, which perform storage key checking.
>> These functions can be used by KVM for emulating instructions that need
>> to be key checked.
>> These functions differ from their non _key counterparts in
>> include/linux/uaccess.h only in the additional key argument and must be
>> kept in sync with those.
>>
>> Since the existing uaccess implementation on s390 makes use of move
>> instructions that support having an additional access key supplied,
>> we can implement raw_copy_from/to_user_key by enhancing the
>> existing implementation.
>>
>> Signed-off-by: Janis Schoetterl-Glausch <[email protected]>
>
> Reviewed-by: Christian Borntraeger <[email protected]>
please use
Reviewed-by: Christian Borntraeger <[email protected]>
instead...
>
>> ---
>>   arch/s390/include/asm/uaccess.h | 22 +++++++++
>>   arch/s390/lib/uaccess.c         | 81 +++++++++++++++++++++++++--------
>>   2 files changed, 85 insertions(+), 18 deletions(-)
>>
>> diff --git a/arch/s390/include/asm/uaccess.h b/arch/s390/include/asm/uaccess.h
>> index d74e26b48604..ba1bcb91af95 100644
>> --- a/arch/s390/include/asm/uaccess.h
>> +++ b/arch/s390/include/asm/uaccess.h
>> @@ -44,6 +44,28 @@ raw_copy_to_user(void __user *to, const void *from, unsigned long n);
>>   #define INLINE_COPY_TO_USER
>>   #endif
>> +unsigned long __must_check
>> +_copy_from_user_key(void *to, const void __user *from, unsigned long n, unsigned long key);
>> +
>> +static __always_inline unsigned long __must_check
>> +copy_from_user_key(void *to, const void __user *from, unsigned long n, unsigned long key)
>> +{
>> +    if (likely(check_copy_size(to, n, false)))
>> +        n = _copy_from_user_key(to, from, n, key);
>> +    return n;
>> +}
>> +
>> +unsigned long __must_check
>> +_copy_to_user_key(void __user *to, const void *from, unsigned long n, unsigned long key);
>> +
>> +static __always_inline unsigned long __must_check
>> +copy_to_user_key(void __user *to, const void *from, unsigned long n, unsigned long key)
>> +{
>> +    if (likely(check_copy_size(from, n, true)))
>> +        n = _copy_to_user_key(to, from, n, key);
>> +    return n;
>> +}
>> +
>>   int __put_user_bad(void) __attribute__((noreturn));
>>   int __get_user_bad(void) __attribute__((noreturn));
>> diff --git a/arch/s390/lib/uaccess.c b/arch/s390/lib/uaccess.c
>> index 8a5d21461889..b709239feb5d 100644
>> --- a/arch/s390/lib/uaccess.c
>> +++ b/arch/s390/lib/uaccess.c
>> @@ -59,11 +59,13 @@ static inline int copy_with_mvcos(void)
>>   #endif
>>   static inline unsigned long copy_from_user_mvcos(void *x, const void __user *ptr,
>> -                         unsigned long size)
>> +                         unsigned long size, unsigned long key)
>>   {
>>       unsigned long tmp1, tmp2;
>>       union oac spec = {
>> +        .oac2.key = key,
>>           .oac2.as = PSW_BITS_AS_SECONDARY,
>> +        .oac2.k = 1,
>>           .oac2.a = 1,
>>       };
>> @@ -94,19 +96,19 @@ static inline unsigned long copy_from_user_mvcos(void *x, const void __user *ptr
>>   }
>>   static inline unsigned long copy_from_user_mvcp(void *x, const void __user *ptr,
>> -                        unsigned long size)
>> +                        unsigned long size, unsigned long key)
>>   {
>>       unsigned long tmp1, tmp2;
>>       tmp1 = -256UL;
>>       asm volatile(
>>           "   sacf  0\n"
>> -        "0: mvcp  0(%0,%2),0(%1),%3\n"
>> +        "0: mvcp  0(%0,%2),0(%1),%[key]\n"
>>           "7: jz    5f\n"
>>           "1: algr  %0,%3\n"
>>           "   la    %1,256(%1)\n"
>>           "   la    %2,256(%2)\n"
>> -        "2: mvcp  0(%0,%2),0(%1),%3\n"
>> +        "2: mvcp  0(%0,%2),0(%1),%[key]\n"
>>           "8: jnz   1b\n"
>>           "   j     5f\n"
>>           "3: la    %4,255(%1)\n"    /* %4 = ptr + 255 */
>> @@ -115,7 +117,7 @@ static inline unsigned long copy_from_user_mvcp(void *x, const void __user *ptr,
>>           "   slgr  %4,%1\n"
>>           "   clgr  %0,%4\n"    /* copy crosses next page boundary? */
>>           "   jnh   6f\n"
>> -        "4: mvcp  0(%4,%2),0(%1),%3\n"
>> +        "4: mvcp  0(%4,%2),0(%1),%[key]\n"
>>           "9: slgr  %0,%4\n"
>>           "   j     6f\n"
>>           "5: slgr  %0,%0\n"
>> @@ -123,24 +125,49 @@ static inline unsigned long copy_from_user_mvcp(void *x, const void __user *ptr,
>>           EX_TABLE(0b,3b) EX_TABLE(2b,3b) EX_TABLE(4b,6b)
>>           EX_TABLE(7b,3b) EX_TABLE(8b,3b) EX_TABLE(9b,6b)
>>           : "+a" (size), "+a" (ptr), "+a" (x), "+a" (tmp1), "=a" (tmp2)
>> -        : : "cc", "memory");
>> +        : [key] "d" (key << 4)
>> +        : "cc", "memory");
>>       return size;
>>   }
>> -unsigned long raw_copy_from_user(void *to, const void __user *from, unsigned long n)
>> +static unsigned long raw_copy_from_user_key(void *to, const void __user *from,
>> +                        unsigned long n, unsigned long key)
>>   {
>>       if (copy_with_mvcos())
>> -        return copy_from_user_mvcos(to, from, n);
>> -    return copy_from_user_mvcp(to, from, n);
>> +        return copy_from_user_mvcos(to, from, n, key);
>> +    return copy_from_user_mvcp(to, from, n, key);
>> +}
>> +
>> +unsigned long raw_copy_from_user(void *to, const void __user *from, unsigned long n)
>> +{
>> +    return raw_copy_from_user_key(to, from, n, 0);
>>   }
>>   EXPORT_SYMBOL(raw_copy_from_user);
>> +unsigned long _copy_from_user_key(void *to, const void __user *from,
>> +                  unsigned long n, unsigned long key)
>> +{
>> +    unsigned long res = n;
>> +
>> +    might_fault();
>> +    if (!should_fail_usercopy()) {
>> +        instrument_copy_from_user(to, from, n);
>> +        res = raw_copy_from_user_key(to, from, n, key);
>> +    }
>> +    if (unlikely(res))
>> +        memset(to + (n - res), 0, res);
>> +    return res;
>> +}
>> +EXPORT_SYMBOL(_copy_from_user_key);
>> +
>>   static inline unsigned long copy_to_user_mvcos(void __user *ptr, const void *x,
>> -                           unsigned long size)
>> +                           unsigned long size, unsigned long key)
>>   {
>>       unsigned long tmp1, tmp2;
>>       union oac spec = {
>> +        .oac1.key = key,
>>           .oac1.as = PSW_BITS_AS_SECONDARY,
>> +        .oac1.k = 1,
>>           .oac1.a = 1,
>>       };
>> @@ -171,19 +198,19 @@ static inline unsigned long copy_to_user_mvcos(void __user *ptr, const void *x,
>>   }
>>   static inline unsigned long copy_to_user_mvcs(void __user *ptr, const void *x,
>> -                          unsigned long size)
>> +                          unsigned long size, unsigned long key)
>>   {
>>       unsigned long tmp1, tmp2;
>>       tmp1 = -256UL;
>>       asm volatile(
>>           "   sacf  0\n"
>> -        "0: mvcs  0(%0,%1),0(%2),%3\n"
>> +        "0: mvcs  0(%0,%1),0(%2),%[key]\n"
>>           "7: jz    5f\n"
>>           "1: algr  %0,%3\n"
>>           "   la    %1,256(%1)\n"
>>           "   la    %2,256(%2)\n"
>> -        "2: mvcs  0(%0,%1),0(%2),%3\n"
>> +        "2: mvcs  0(%0,%1),0(%2),%[key]\n"
>>           "8: jnz   1b\n"
>>           "   j     5f\n"
>>           "3: la    %4,255(%1)\n" /* %4 = ptr + 255 */
>> @@ -192,7 +219,7 @@ static inline unsigned long copy_to_user_mvcs(void __user *ptr, const void *x,
>>           "   slgr  %4,%1\n"
>>           "   clgr  %0,%4\n"    /* copy crosses next page boundary? */
>>           "   jnh   6f\n"
>> -        "4: mvcs  0(%4,%1),0(%2),%3\n"
>> +        "4: mvcs  0(%4,%1),0(%2),%[key]\n"
>>           "9: slgr  %0,%4\n"
>>           "   j     6f\n"
>>           "5: slgr  %0,%0\n"
>> @@ -200,18 +227,36 @@ static inline unsigned long copy_to_user_mvcs(void __user *ptr, const void *x,
>>           EX_TABLE(0b,3b) EX_TABLE(2b,3b) EX_TABLE(4b,6b)
>>           EX_TABLE(7b,3b) EX_TABLE(8b,3b) EX_TABLE(9b,6b)
>>           : "+a" (size), "+a" (ptr), "+a" (x), "+a" (tmp1), "=a" (tmp2)
>> -        : : "cc", "memory");
>> +        : [key] "d" (key << 4)
>> +        : "cc", "memory");
>>       return size;
>>   }
>> -unsigned long raw_copy_to_user(void __user *to, const void *from, unsigned long n)
>> +static unsigned long raw_copy_to_user_key(void __user *to, const void *from,
>> +                      unsigned long n, unsigned long key)
>>   {
>>       if (copy_with_mvcos())
>> -        return copy_to_user_mvcos(to, from, n);
>> -    return copy_to_user_mvcs(to, from, n);
>> +        return copy_to_user_mvcos(to, from, n, key);
>> +    return copy_to_user_mvcs(to, from, n, key);
>> +}
>> +
>> +unsigned long raw_copy_to_user(void __user *to, const void *from, unsigned long n)
>> +{
>> +    return raw_copy_to_user_key(to, from, n, 0);
>>   }
>>   EXPORT_SYMBOL(raw_copy_to_user);
>> +unsigned long _copy_to_user_key(void __user *to, const void *from,
>> +                unsigned long n, unsigned long key)
>> +{
>> +    might_fault();
>> +    if (should_fail_usercopy())
>> +        return n;
>> +    instrument_copy_to_user(to, from, n);
>> +    return raw_copy_to_user_key(to, from, n, key);
>> +}
>> +EXPORT_SYMBOL(_copy_to_user_key);
>> +
>>   static inline unsigned long clear_user_mvcos(void __user *to, unsigned long size)
>>   {
>>       unsigned long tmp1, tmp2;