All of lore.kernel.org
 help / color / mirror / Atom feed
From: Kees Cook <kees@kernel.org>
To: "Mickaël Salaün" <mic@digikod.net>
Cc: Kees Cook <kees@kernel.org>, kernel test robot <lkp@intel.com>,
	Randy Dunlap <rdunlap@infradead.org>,
	Thomas Gleixner <tglx@linutronix.de>,
	Ingo Molnar <mingo@redhat.com>, Borislav Petkov <bp@alien8.de>,
	Dave Hansen <dave.hansen@linux.intel.com>,
	x86@kernel.org, "H. Peter Anvin" <hpa@zytor.com>,
	Andy Shevchenko <andy@kernel.org>,
	Uros Bizjak <ubizjak@gmail.com>,
	linux-hardening@vger.kernel.org, linux-kernel@vger.kernel.org
Subject: [PATCH] string.h: Provide basic sanity checks for fallback memcpy()
Date: Tue, 20 May 2025 09:33:21 -0700	[thread overview]
Message-ID: <20250520163320.work.924-kees@kernel.org> (raw)

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


                 reply	other threads:[~2025-05-20 16:33 UTC|newest]

Thread overview: [no followups] expand[flat|nested]  mbox.gz  Atom feed

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=20250520163320.work.924-kees@kernel.org \
    --to=kees@kernel.org \
    --cc=andy@kernel.org \
    --cc=bp@alien8.de \
    --cc=dave.hansen@linux.intel.com \
    --cc=hpa@zytor.com \
    --cc=linux-hardening@vger.kernel.org \
    --cc=linux-kernel@vger.kernel.org \
    --cc=lkp@intel.com \
    --cc=mic@digikod.net \
    --cc=mingo@redhat.com \
    --cc=rdunlap@infradead.org \
    --cc=tglx@linutronix.de \
    --cc=ubizjak@gmail.com \
    --cc=x86@kernel.org \
    /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 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.