All of lore.kernel.org
 help / color / mirror / Atom feed
* [PATCH] string.h: Provide basic sanity checks for fallback memcpy()
@ 2025-05-20 16:33 Kees Cook
  0 siblings, 0 replies; only message in thread
From: Kees Cook @ 2025-05-20 16:33 UTC (permalink / raw)
  To: Mickaël Salaün
  Cc: Kees Cook, kernel test robot, Randy Dunlap, Thomas Gleixner,
	Ingo Molnar, Borislav Petkov, Dave Hansen, x86, H. Peter Anvin,
	Andy Shevchenko, Uros Bizjak, linux-hardening, linux-kernel

Instead of defining memcpy() in terms of __builtin_memcpy() deep
in arch/x86/include/asm/string_32.h, notice that it is needed up in
the general string.h, as done with other common C String APIs. This
allows us to add basic sanity checking for pathological "size"
arguments to memcpy(). Besides the run-time checking benefit, this
avoids GCC trying to be very smart about value range tracking[1] when
CONFIG_PROFILE_ALL_BRANCHES=y but FORTIFY_SOURCE=n.

Link: https://lore.kernel.org/all/202505191117.C094A90F88@keescook/ [1]
Reported-by: kernel test robot <lkp@intel.com>
Closes: https://lore.kernel.org/all/202501040747.S3LYfvYq-lkp@intel.com/
Reported-by: Randy Dunlap <rdunlap@infradead.org>
Closes: https://lore.kernel.org/all/e3754f69-1dea-4542-8de0-a567a14fb95b@infradead.org/
Signed-off-by: Kees Cook <kees@kernel.org>
---
Cc: "Mickaël Salaün" <mic@digikod.net>
Cc: Thomas Gleixner <tglx@linutronix.de>
Cc: Ingo Molnar <mingo@redhat.com>
Cc: Borislav Petkov <bp@alien8.de>
Cc: Dave Hansen <dave.hansen@linux.intel.com>
Cc: <x86@kernel.org>
Cc: "H. Peter Anvin" <hpa@zytor.com>
Cc: Andy Shevchenko <andy@kernel.org>
Cc: Uros Bizjak <ubizjak@gmail.com>
Cc: <linux-hardening@vger.kernel.org>
---
 arch/x86/include/asm/string_32.h |  6 ------
 include/linux/string.h           | 13 +++++++++++++
 2 files changed, 13 insertions(+), 6 deletions(-)

diff --git a/arch/x86/include/asm/string_32.h b/arch/x86/include/asm/string_32.h
index 32c0d981a82a..fad9566f43a6 100644
--- a/arch/x86/include/asm/string_32.h
+++ b/arch/x86/include/asm/string_32.h
@@ -145,12 +145,6 @@ static __always_inline void *__constant_memcpy(void *to, const void *from,
 #define __HAVE_ARCH_MEMCPY
 extern void *memcpy(void *, const void *, size_t);
 
-#ifndef CONFIG_FORTIFY_SOURCE
-
-#define memcpy(t, f, n) __builtin_memcpy(t, f, n)
-
-#endif /* !CONFIG_FORTIFY_SOURCE */
-
 #define __HAVE_ARCH_MEMMOVE
 void *memmove(void *dest, const void *src, size_t n);
 
diff --git a/include/linux/string.h b/include/linux/string.h
index 01621ad0f598..a1f8fdcf8482 100644
--- a/include/linux/string.h
+++ b/include/linux/string.h
@@ -4,6 +4,7 @@
 
 #include <linux/args.h>
 #include <linux/array_size.h>
+#include <linux/bug.h>
 #include <linux/cleanup.h>	/* for DEFINE_FREE() */
 #include <linux/compiler.h>	/* for inline */
 #include <linux/types.h>	/* for size_t */
@@ -390,7 +391,19 @@ static inline const char *kbasename(const char *path)
 
 #if !defined(__NO_FORTIFY) && defined(__OPTIMIZE__) && defined(CONFIG_FORTIFY_SOURCE)
 #include <linux/fortify-string.h>
+#else
+/* Basic sanity checking even without FORTIFY_SOURCE */
+# ifndef __HAVE_ARCH_MEMCPY
+#  define memcpy(t, f, n)					\
+	do {							\
+		typeof(n) __n = (n);				\
+		/* Skip impossible sizes. */			\
+		if (!WARN_ON(__n < 0 || __n == SIZE_MAX))	\
+			__builtin_memcpy(t, f, __n);		\
+	} while (0)
+# endif
 #endif
+
 #ifndef unsafe_memcpy
 #define unsafe_memcpy(dst, src, bytes, justification)		\
 	memcpy(dst, src, bytes)
-- 
2.34.1


^ permalink raw reply related	[flat|nested] only message in thread

only message in thread, other threads:[~2025-05-20 16:33 UTC | newest]

Thread overview: (only message) (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
2025-05-20 16:33 [PATCH] string.h: Provide basic sanity checks for fallback memcpy() Kees Cook

This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.