Received-SPF: pass (google.com: domain of linux-kernel-owner@vger.kernel.org designates 23.128.96.18 as permitted sender) client-ip=23.128.96.18;
Message-ID: <b6896b1a-d529-1504-091c-d41287c01dc8@linux.ibm.com>
Date:   Fri, 21 Jan 2022 08:32:25 +0100
MIME-Version: 1.0
User-Agent: Mozilla/5.0 (X11; Linux x86_64; rv:91.0) Gecko/20100101
 Thunderbird/91.4.0
Subject: Re: [RFC PATCH v1 01/10] s390/uaccess: Add storage key checked access
 to user memory
Content-Language: en-US
To:     Heiko Carstens <hca@linux.ibm.com>
Cc:     Janis Schoetterl-Glausch <scgl@linux.ibm.com>,
        Vasily Gorbik <gor@linux.ibm.com>,
        Sven Schnelle <svens@linux.ibm.com>,
        Nico Boehr <nrb@linux.ibm.com>,
        Alexander Gordeev <agordeev@linux.ibm.com>,
        linux-s390@vger.kernel.org, linux-kernel@vger.kernel.org
References: <20220118095210.1651483-1-scgl@linux.ibm.com>
 <20220118095210.1651483-2-scgl@linux.ibm.com> <YefeakONMN4PLlml@osiris>
 <422595a5-b24b-8760-ff0e-112322142de7@linux.ibm.com>
 <YegQCTqEsiFTUZ2R@osiris>
 <dbfec527-b995-e382-dafa-c3459e1e45ed@linux.ibm.com>
 <YelcCEuVbIg9ND90@osiris> <YemnprCIEjjGFeTn@osiris>
From:   Christian Borntraeger <borntraeger@linux.ibm.com>
In-Reply-To: <YemnprCIEjjGFeTn@osiris>
Content-Type: text/plain; charset=UTF-8; format=flowed
Content-Transfer-Encoding: 7bit
Precedence: bulk


Am 20.01.22 um 19:19 schrieb Heiko Carstens:
> On Thu, Jan 20, 2022 at 01:56:40PM +0100, Heiko Carstens wrote:
>>> 2. Implementation changes
>>>     2.1 Modify common code
>>
>> In general such changes are done in way that common code is or _may_ be
>> modified to fulfill our needs. Common code header file explicitely states
>> that architectures should get rid of private instances of
>> copy_{to,from}_user() and __copy_{to,from}_user{,_inatomic}().
>>
>> So we should not add anything like that to arch code again, since nobody
>> would expect that.
> 
> Or to be more specific: I think the most simple solution would be to
> try to get the new *key variants into include/linux/uaccess.h, and add
> the raw variants in architecture code, similar to the rest of the
> uaccess functions.
> There is some (sort of) prior art with copy_mc_to_kernel() even,
> though that can only partially be compared.

So in essence adding something like this and then providing raw_copy_from/to_user_key?
(whitespace damaged, just pasted in)

diff --git a/include/linux/uaccess.h b/include/linux/uaccess.h
index ac0394087f7d..3b6e78ee211c 100644
--- a/include/linux/uaccess.h
+++ b/include/linux/uaccess.h
@@ -201,6 +201,59 @@ copy_to_user(void __user *to, const void *from, unsigned long n)
         return n;
  }
  
+
+#if defined(__s390x__) && defined(CONFIG_KVM)
+/*
+ * Variants that pass along an access key. Uses by KVM on s390x to implement
+ * key checks for guests that use storage keys Must be kept in sync with the
+ * non-key variants from above. The only difference is the _key suffix when
+ * calling raw_copy_from/to_user_key.
+ */
+static inline __must_check unsigned long
+_copy_from_user_key(void *to, const void __user *from, unsigned long n, u8 key)
+{
+       unsigned long res = n;
+       might_fault();
+       if (!should_fail_usercopy() && likely(access_ok(from, n))) {
+               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;
+}
+
+static inline __must_check unsigned long
+_copy_to_user_key(void __user *to, const void *from, unsigned long n, u8 key)
+{
+       might_fault();
+       if (should_fail_usercopy())
+               return n;
+       if (access_ok(to, n)) {
+               instrument_copy_to_user(to, from, n);
+               n = raw_copy_to_user_key(to, from, n, key);
+       }
+       return n;
+}
+
+static __always_inline unsigned long __must_check
+copy_from_user_key(void *to, const void __user *from, unsigned long n, u8 key)
+{
+       if (likely(check_copy_size(to, n, false)))
+               n = _copy_from_user_key(to, from, n, key);
+       return n;
+}
+
+static __always_inline unsigned long __must_check
+copy_to_user_key(void __user *to, const void *from, unsigned long n, u8 key)
+{
+       if (likely(check_copy_size(from, n, true)))
+               n = _copy_to_user_key(to, from, n);
+       return n;
+}
+#endif
+
+
  #ifndef copy_mc_to_kernel
  /*
   * Without arch opt-in this generic copy_mc_to_kernel() will not handle