linux-mm.kvack.org archive mirror
 help / color / mirror / Atom feed
From: Ilya Leoshkevich <iii@linux.ibm.com>
To: Alexander Gordeev <agordeev@linux.ibm.com>,
	Alexander Potapenko <glider@google.com>,
	Andrew Morton <akpm@linux-foundation.org>,
	Christoph Lameter <cl@linux.com>,
	David Rientjes <rientjes@google.com>,
	Heiko Carstens <hca@linux.ibm.com>,
	Joonsoo Kim <iamjoonsoo.kim@lge.com>,
	Marco Elver <elver@google.com>,
	Masami Hiramatsu <mhiramat@kernel.org>,
	Pekka Enberg <penberg@kernel.org>,
	Steven Rostedt <rostedt@goodmis.org>,
	Vasily Gorbik <gor@linux.ibm.com>,
	Vlastimil Babka <vbabka@suse.cz>
Cc: Christian Borntraeger <borntraeger@linux.ibm.com>,
	Dmitry Vyukov <dvyukov@google.com>,
	Hyeonggon Yoo <42.hyeyoo@gmail.com>,
	kasan-dev@googlegroups.com, linux-kernel@vger.kernel.org,
	linux-mm@kvack.org, linux-s390@vger.kernel.org,
	linux-trace-kernel@vger.kernel.org,
	Mark Rutland <mark.rutland@arm.com>,
	Roman Gushchin <roman.gushchin@linux.dev>,
	Sven Schnelle <svens@linux.ibm.com>,
	Ilya Leoshkevich <iii@linux.ibm.com>
Subject: [PATCH v5 33/37] s390/uaccess: Add KMSAN support to put_user() and get_user()
Date: Wed, 19 Jun 2024 17:44:08 +0200	[thread overview]
Message-ID: <20240619154530.163232-34-iii@linux.ibm.com> (raw)
In-Reply-To: <20240619154530.163232-1-iii@linux.ibm.com>

put_user() uses inline assembly with precise constraints, so Clang is
in principle capable of instrumenting it automatically. Unfortunately,
one of the constraints contains a dereferenced user pointer, and Clang
does not currently distinguish user and kernel pointers. Therefore
KMSAN attempts to access shadow for user pointers, which is not a right
thing to do.

An obvious fix to add __no_sanitize_memory to __put_user_fn() does not
work, since it's __always_inline. And __always_inline cannot be removed
due to the __put_user_bad() trick.

A different obvious fix of using the "a" instead of the "+Q" constraint
degrades the code quality, which is very important here, since it's a
hot path.

Instead, repurpose the __put_user_asm() macro to define
__put_user_{char,short,int,long}_noinstr() functions and mark them with
__no_sanitize_memory. For the non-KMSAN builds make them
__always_inline in order to keep the generated code quality. Also
define __put_user_{char,short,int,long}() functions, which call the
aforementioned ones and which *are* instrumented, because they call
KMSAN hooks, which may be implemented as macros.

The same applies to get_user() as well.

Acked-by: Heiko Carstens <hca@linux.ibm.com>
Signed-off-by: Ilya Leoshkevich <iii@linux.ibm.com>
---
 arch/s390/include/asm/uaccess.h | 111 +++++++++++++++++++++++---------
 1 file changed, 79 insertions(+), 32 deletions(-)

diff --git a/arch/s390/include/asm/uaccess.h b/arch/s390/include/asm/uaccess.h
index 81ae8a98e7ec..70f0edc00c2a 100644
--- a/arch/s390/include/asm/uaccess.h
+++ b/arch/s390/include/asm/uaccess.h
@@ -78,13 +78,24 @@ union oac {
 
 int __noreturn __put_user_bad(void);
 
-#define __put_user_asm(to, from, size)					\
-({									\
+#ifdef CONFIG_KMSAN
+#define get_put_user_noinstr_attributes \
+	noinline __maybe_unused __no_sanitize_memory
+#else
+#define get_put_user_noinstr_attributes __always_inline
+#endif
+
+#define DEFINE_PUT_USER(type)						\
+static get_put_user_noinstr_attributes int				\
+__put_user_##type##_noinstr(unsigned type __user *to,			\
+			    unsigned type *from,			\
+			    unsigned long size)				\
+{									\
 	union oac __oac_spec = {					\
 		.oac1.as = PSW_BITS_AS_SECONDARY,			\
 		.oac1.a = 1,						\
 	};								\
-	int __rc;							\
+	int rc;								\
 									\
 	asm volatile(							\
 		"	lr	0,%[spec]\n"				\
@@ -93,12 +104,28 @@ int __noreturn __put_user_bad(void);
 		"2:\n"							\
 		EX_TABLE_UA_STORE(0b, 2b, %[rc])			\
 		EX_TABLE_UA_STORE(1b, 2b, %[rc])			\
-		: [rc] "=&d" (__rc), [_to] "+Q" (*(to))			\
+		: [rc] "=&d" (rc), [_to] "+Q" (*(to))			\
 		: [_size] "d" (size), [_from] "Q" (*(from)),		\
 		  [spec] "d" (__oac_spec.val)				\
 		: "cc", "0");						\
-	__rc;								\
-})
+	return rc;							\
+}									\
+									\
+static __always_inline int						\
+__put_user_##type(unsigned type __user *to, unsigned type *from,	\
+		  unsigned long size)					\
+{									\
+	int rc;								\
+									\
+	rc = __put_user_##type##_noinstr(to, from, size);		\
+	instrument_put_user(*from, to, size);				\
+	return rc;							\
+}
+
+DEFINE_PUT_USER(char);
+DEFINE_PUT_USER(short);
+DEFINE_PUT_USER(int);
+DEFINE_PUT_USER(long);
 
 static __always_inline int __put_user_fn(void *x, void __user *ptr, unsigned long size)
 {
@@ -106,24 +133,24 @@ static __always_inline int __put_user_fn(void *x, void __user *ptr, unsigned lon
 
 	switch (size) {
 	case 1:
-		rc = __put_user_asm((unsigned char __user *)ptr,
-				    (unsigned char *)x,
-				    size);
+		rc = __put_user_char((unsigned char __user *)ptr,
+				     (unsigned char *)x,
+				     size);
 		break;
 	case 2:
-		rc = __put_user_asm((unsigned short __user *)ptr,
-				    (unsigned short *)x,
-				    size);
+		rc = __put_user_short((unsigned short __user *)ptr,
+				      (unsigned short *)x,
+				      size);
 		break;
 	case 4:
-		rc = __put_user_asm((unsigned int __user *)ptr,
+		rc = __put_user_int((unsigned int __user *)ptr,
 				    (unsigned int *)x,
 				    size);
 		break;
 	case 8:
-		rc = __put_user_asm((unsigned long __user *)ptr,
-				    (unsigned long *)x,
-				    size);
+		rc = __put_user_long((unsigned long __user *)ptr,
+				     (unsigned long *)x,
+				     size);
 		break;
 	default:
 		__put_user_bad();
@@ -134,13 +161,17 @@ static __always_inline int __put_user_fn(void *x, void __user *ptr, unsigned lon
 
 int __noreturn __get_user_bad(void);
 
-#define __get_user_asm(to, from, size)					\
-({									\
+#define DEFINE_GET_USER(type)						\
+static get_put_user_noinstr_attributes int				\
+__get_user_##type##_noinstr(unsigned type *to,				\
+			    unsigned type __user *from,			\
+			    unsigned long size)				\
+{									\
 	union oac __oac_spec = {					\
 		.oac2.as = PSW_BITS_AS_SECONDARY,			\
 		.oac2.a = 1,						\
 	};								\
-	int __rc;							\
+	int rc;								\
 									\
 	asm volatile(							\
 		"	lr	0,%[spec]\n"				\
@@ -149,13 +180,29 @@ int __noreturn __get_user_bad(void);
 		"2:\n"							\
 		EX_TABLE_UA_LOAD_MEM(0b, 2b, %[rc], %[_to], %[_ksize])	\
 		EX_TABLE_UA_LOAD_MEM(1b, 2b, %[rc], %[_to], %[_ksize])	\
-		: [rc] "=&d" (__rc), "=Q" (*(to))			\
+		: [rc] "=&d" (rc), "=Q" (*(to))				\
 		: [_size] "d" (size), [_from] "Q" (*(from)),		\
 		  [spec] "d" (__oac_spec.val), [_to] "a" (to),		\
 		  [_ksize] "K" (size)					\
 		: "cc", "0");						\
-	__rc;								\
-})
+	return rc;							\
+}									\
+									\
+static __always_inline int						\
+__get_user_##type(unsigned type *to, unsigned type __user *from,	\
+		  unsigned long size)					\
+{									\
+	int rc;								\
+									\
+	rc = __get_user_##type##_noinstr(to, from, size);		\
+	instrument_get_user(*to);					\
+	return rc;							\
+}
+
+DEFINE_GET_USER(char);
+DEFINE_GET_USER(short);
+DEFINE_GET_USER(int);
+DEFINE_GET_USER(long);
 
 static __always_inline int __get_user_fn(void *x, const void __user *ptr, unsigned long size)
 {
@@ -163,24 +210,24 @@ static __always_inline int __get_user_fn(void *x, const void __user *ptr, unsign
 
 	switch (size) {
 	case 1:
-		rc = __get_user_asm((unsigned char *)x,
-				    (unsigned char __user *)ptr,
-				    size);
+		rc = __get_user_char((unsigned char *)x,
+				     (unsigned char __user *)ptr,
+				     size);
 		break;
 	case 2:
-		rc = __get_user_asm((unsigned short *)x,
-				    (unsigned short __user *)ptr,
-				    size);
+		rc = __get_user_short((unsigned short *)x,
+				      (unsigned short __user *)ptr,
+				      size);
 		break;
 	case 4:
-		rc = __get_user_asm((unsigned int *)x,
+		rc = __get_user_int((unsigned int *)x,
 				    (unsigned int __user *)ptr,
 				    size);
 		break;
 	case 8:
-		rc = __get_user_asm((unsigned long *)x,
-				    (unsigned long __user *)ptr,
-				    size);
+		rc = __get_user_long((unsigned long *)x,
+				     (unsigned long __user *)ptr,
+				     size);
 		break;
 	default:
 		__get_user_bad();
-- 
2.45.1



  parent reply	other threads:[~2024-06-19 17:32 UTC|newest]

Thread overview: 42+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2024-06-19 15:43 [PATCH v5 00/37] kmsan: Enable on s390 Ilya Leoshkevich
2024-06-19 15:43 ` [PATCH v5 01/37] ftrace: Unpoison ftrace_regs in ftrace_ops_list_func() Ilya Leoshkevich
2024-06-19 15:43 ` [PATCH v5 02/37] kmsan: Make the tests compatible with kmsan.panic=1 Ilya Leoshkevich
2024-06-19 15:43 ` [PATCH v5 03/37] kmsan: Disable KMSAN when DEFERRED_STRUCT_PAGE_INIT is enabled Ilya Leoshkevich
2024-06-19 15:43 ` [PATCH v5 04/37] kmsan: Increase the maximum store size to 4096 Ilya Leoshkevich
2024-06-19 15:43 ` [PATCH v5 07/37] kmsan: Remove a useless assignment from kmsan_vmap_pages_range_noflush() Ilya Leoshkevich
2024-06-19 15:43 ` [PATCH v5 08/37] kmsan: Remove an x86-specific #include from kmsan.h Ilya Leoshkevich
2024-06-19 15:43 ` [PATCH v5 10/37] kmsan: Export panic_on_kmsan Ilya Leoshkevich
2024-06-19 15:43 ` [PATCH v5 11/37] kmsan: Allow disabling KMSAN checks for the current task Ilya Leoshkevich
2024-06-19 15:43 ` [PATCH v5 14/37] kmsan: Use ALIGN_DOWN() in kmsan_get_metadata() Ilya Leoshkevich
2024-06-19 15:43 ` [PATCH v5 16/37] mm: slub: Let KMSAN access metadata Ilya Leoshkevich
2024-06-19 15:43 ` [PATCH v5 17/37] mm: slub: Disable KMSAN when checking the padding bytes Ilya Leoshkevich
2024-06-20  9:00   ` Alexander Potapenko
2024-06-19 15:43 ` [PATCH v5 18/37] mm: kfence: Disable KMSAN when checking the canary Ilya Leoshkevich
2024-06-19 15:43 ` [PATCH v5 19/37] lib/zlib: Unpoison DFLTCC output buffers Ilya Leoshkevich
2024-06-19 15:43 ` [PATCH v5 20/37] kmsan: Accept ranges starting with 0 on s390 Ilya Leoshkevich
2024-06-19 15:43 ` [PATCH v5 21/37] s390/boot: Turn off KMSAN Ilya Leoshkevich
2024-06-19 15:43 ` [PATCH v5 22/37] s390: Use a larger stack for KMSAN Ilya Leoshkevich
2024-06-19 15:43 ` [PATCH v5 23/37] s390/boot: Add the KMSAN runtime stub Ilya Leoshkevich
2024-06-19 15:43 ` [PATCH v5 24/37] s390/checksum: Add a KMSAN check Ilya Leoshkevich
2024-06-19 15:44 ` [PATCH v5 25/37] s390/cpacf: Unpoison the results of cpacf_trng() Ilya Leoshkevich
2024-06-19 15:44 ` [PATCH v5 26/37] s390/cpumf: Unpoison STCCTM output buffer Ilya Leoshkevich
2024-06-19 15:44 ` [PATCH v5 27/37] s390/diag: Unpoison diag224() " Ilya Leoshkevich
2024-06-19 15:44 ` [PATCH v5 29/37] s390/irqflags: Do not instrument arch_local_irq_*() with KMSAN Ilya Leoshkevich
2024-06-19 15:44 ` [PATCH v5 30/37] s390/mm: Define KMSAN metadata for vmalloc and modules Ilya Leoshkevich
2024-06-19 15:44 ` [PATCH v5 31/37] s390/string: Add KMSAN support Ilya Leoshkevich
2024-06-19 15:44 ` Ilya Leoshkevich [this message]
2024-06-20  8:36   ` [PATCH v5 33/37] s390/uaccess: Add KMSAN support to put_user() and get_user() Alexander Potapenko
     [not found]     ` <aaef3e0fe22ad9074de84717f36f316204ae088c.camel@linux.ibm.com>
2024-06-20 11:46       ` Alexander Potapenko
2024-06-20 17:05       ` Ilya Leoshkevich
2024-06-19 15:44 ` [PATCH v5 34/37] s390/uaccess: Add the missing linux/instrumented.h #include Ilya Leoshkevich
2024-06-20  8:15   ` Alexander Potapenko
2024-06-19 15:44 ` [PATCH v5 35/37] s390/unwind: Disable KMSAN checks Ilya Leoshkevich
2024-06-19 15:44 ` [PATCH v5 36/37] s390/kmsan: Implement the architecture-specific functions Ilya Leoshkevich
2024-06-20  9:25   ` Alexander Gordeev
2024-06-20 13:38     ` Ilya Leoshkevich
2024-06-20 14:18       ` Alexander Potapenko
2024-06-20 14:19         ` Alexander Potapenko
2024-06-20 13:59   ` Alexander Gordeev
2024-06-19 15:44 ` [PATCH v5 37/37] kmsan: Enable on s390 Ilya Leoshkevich
     [not found] ` <20240619154530.163232-13-iii@linux.ibm.com>
2024-06-20  8:14   ` [PATCH v5 12/37] kmsan: Introduce memset_no_sanitize_memory() Alexander Potapenko
     [not found] ` <20240619154530.163232-14-iii@linux.ibm.com>
2024-06-20 14:58   ` [PATCH v5 13/37] kmsan: Support SLAB_POISON Alexander Potapenko

Reply instructions:

You may reply publicly to this message via plain-text email
using any one of the following methods:

* Save the following mbox file, import it into your mail client,
  and reply-to-all from there: mbox

  Avoid top-posting and favor interleaved quoting:
  https://en.wikipedia.org/wiki/Posting_style#Interleaved_style

* Reply using the --to, --cc, and --in-reply-to
  switches of git-send-email(1):

  git send-email \
    --in-reply-to=20240619154530.163232-34-iii@linux.ibm.com \
    --to=iii@linux.ibm.com \
    --cc=42.hyeyoo@gmail.com \
    --cc=agordeev@linux.ibm.com \
    --cc=akpm@linux-foundation.org \
    --cc=borntraeger@linux.ibm.com \
    --cc=cl@linux.com \
    --cc=dvyukov@google.com \
    --cc=elver@google.com \
    --cc=glider@google.com \
    --cc=gor@linux.ibm.com \
    --cc=hca@linux.ibm.com \
    --cc=iamjoonsoo.kim@lge.com \
    --cc=kasan-dev@googlegroups.com \
    --cc=linux-kernel@vger.kernel.org \
    --cc=linux-mm@kvack.org \
    --cc=linux-s390@vger.kernel.org \
    --cc=linux-trace-kernel@vger.kernel.org \
    --cc=mark.rutland@arm.com \
    --cc=mhiramat@kernel.org \
    --cc=penberg@kernel.org \
    --cc=rientjes@google.com \
    --cc=roman.gushchin@linux.dev \
    --cc=rostedt@goodmis.org \
    --cc=svens@linux.ibm.com \
    --cc=vbabka@suse.cz \
    /path/to/YOUR_REPLY

  https://kernel.org/pub/software/scm/git/docs/git-send-email.html

* If your mail client supports setting the In-Reply-To header
  via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line before the message body.
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox