public inbox for llvm@lists.linux.dev
 help / color / mirror / Atom feed
From: Kees Cook <keescook@chromium.org>
To: Jakub Kicinski <kuba@kernel.org>
Cc: Kees Cook <keescook@chromium.org>,
	Andy Shevchenko <andy@kernel.org>,
	Masahiro Yamada <masahiroy@kernel.org>,
	Nathan Chancellor <nathan@kernel.org>,
	Nick Desaulniers <ndesaulniers@google.com>,
	Nicolas Schier <nicolas@fjasle.eu>, Tom Rix <trix@redhat.com>,
	Josh Poimboeuf <jpoimboe@kernel.org>,
	Miroslav Benes <mbenes@suse.cz>, Marco Elver <elver@google.com>,
	Andrew Morton <akpm@linux-foundation.org>,
	Linus Walleij <linus.walleij@linaro.org>,
	Cezary Rojewski <cezary.rojewski@intel.com>,
	Mark Brown <broonie@kernel.org>, Puyou Lu <puyou.lu@gmail.com>,
	linux-hardening@vger.kernel.org, linux-kbuild@vger.kernel.org,
	llvm@lists.linux.dev, linux-kernel@vger.kernel.org
Subject: [PATCH] fortify: Improve buffer overflow reporting
Date: Thu,  2 Mar 2023 14:58:12 -0800	[thread overview]
Message-ID: <20230302225808.never.375-kees@kernel.org> (raw)

Improve the reporting of buffer overflows under CONFIG_FORTIFY_SOURCE to
help accelerate debugging efforts. The calculations are all just sitting
in registers anyway, so pass them along to the function to be reported.

For example, before:

  detected buffer overflow in memcpy

and after:

  memcpy: detected buffer overflow: 4096 byte read from buffer of size 1

Cc: Jakub Kicinski <kuba@kernel.org>
Cc: Andy Shevchenko <andy@kernel.org>
Cc: Masahiro Yamada <masahiroy@kernel.org>
Cc: Nathan Chancellor <nathan@kernel.org>
Cc: Nick Desaulniers <ndesaulniers@google.com>
Cc: Nicolas Schier <nicolas@fjasle.eu>
Cc: Tom Rix <trix@redhat.com>
Cc: Josh Poimboeuf <jpoimboe@kernel.org>
Cc: Miroslav Benes <mbenes@suse.cz>
Cc: Marco Elver <elver@google.com>
Cc: Andrew Morton <akpm@linux-foundation.org>
Cc: Linus Walleij <linus.walleij@linaro.org>
Cc: Cezary Rojewski <cezary.rojewski@intel.com>
Cc: Mark Brown <broonie@kernel.org>
Cc: Puyou Lu <puyou.lu@gmail.com>
Cc: linux-hardening@vger.kernel.org
Cc: linux-kbuild@vger.kernel.org
Cc: llvm@lists.linux.dev
Signed-off-by: Kees Cook <keescook@chromium.org>
---
 include/linux/fortify-string.h | 51 +++++++++++++++++++---------------
 lib/string_helpers.c           |  5 ++--
 2 files changed, 32 insertions(+), 24 deletions(-)

diff --git a/include/linux/fortify-string.h b/include/linux/fortify-string.h
index c9de1f59ee80..981e2838f99a 100644
--- a/include/linux/fortify-string.h
+++ b/include/linux/fortify-string.h
@@ -9,7 +9,7 @@
 #define __FORTIFY_INLINE extern __always_inline __gnu_inline __overloadable
 #define __RENAME(x) __asm__(#x)
 
-void fortify_panic(const char *name) __noreturn __cold;
+void fortify_panic(const char *name, bool dest, size_t avail, size_t len) __noreturn __cold;
 void __read_overflow(void) __compiletime_error("detected read beyond size of object (1st parameter)");
 void __read_overflow2(void) __compiletime_error("detected read beyond size of object (2nd parameter)");
 void __read_overflow2_field(size_t avail, size_t wanted) __compiletime_warning("detected read beyond size of field (2nd parameter); maybe use struct_group()?");
@@ -147,7 +147,7 @@ char *strncpy(char * const POS p, const char *q, __kernel_size_t size)
 	if (__compiletime_lessthan(p_size, size))
 		__write_overflow();
 	if (p_size < size)
-		fortify_panic(__func__);
+		fortify_panic(__func__, 1, p_size, size);
 	return __underlying_strncpy(p, q, size);
 }
 
@@ -170,11 +170,13 @@ __FORTIFY_INLINE __diagnose_as(__builtin_strcat, 1, 2)
 char *strcat(char * const POS p, const char *q)
 {
 	size_t p_size = __member_size(p);
+	size_t size;
 
 	if (p_size == SIZE_MAX)
 		return __underlying_strcat(p, q);
-	if (strlcat(p, q, p_size) >= p_size)
-		fortify_panic(__func__);
+	size = strlcat(p, q, p_size);
+	if (p_size < size)
+		fortify_panic(__func__, 1, p_size, size);
 	return p;
 }
 
@@ -205,7 +207,7 @@ __FORTIFY_INLINE __kernel_size_t strnlen(const char * const POS p, __kernel_size
 	/* Do not check characters beyond the end of p. */
 	ret = __real_strnlen(p, maxlen < p_size ? maxlen : p_size);
 	if (p_size <= ret && maxlen != ret)
-		fortify_panic(__func__);
+		fortify_panic(__func__, 1, p_size, ret);
 	return ret;
 }
 
@@ -241,7 +243,7 @@ __kernel_size_t __fortify_strlen(const char * const POS p)
 		return __underlying_strlen(p);
 	ret = strnlen(p, p_size);
 	if (p_size <= ret)
-		fortify_panic(__func__);
+		fortify_panic(__func__, 1, p_size, ret);
 	return ret;
 }
 
@@ -282,8 +284,8 @@ __FORTIFY_INLINE size_t strlcpy(char * const POS p, const char * const POS q, si
 			__write_overflow();
 	}
 	if (size) {
-		if (len >= p_size)
-			fortify_panic(__func__);
+		if (p_size < len)
+			fortify_panic(__func__, 1, p_size, len);
 		__underlying_memcpy(p, q, len);
 		p[len] = '\0';
 	}
@@ -361,7 +363,7 @@ __FORTIFY_INLINE ssize_t strscpy(char * const POS p, const char * const POS q, s
 	 * p_size.
 	 */
 	if (len > p_size)
-		fortify_panic(__func__);
+		fortify_panic(__func__, 1, p_size, len);
 
 	/*
 	 * We can now safely call vanilla strscpy because we are protected from:
@@ -397,13 +399,15 @@ char *strncat(char * const POS p, const char * const POS q, __kernel_size_t coun
 	size_t p_len, copy_len;
 	size_t p_size = __member_size(p);
 	size_t q_size = __member_size(q);
+	size_t size;
 
 	if (p_size == SIZE_MAX && q_size == SIZE_MAX)
 		return __underlying_strncat(p, q, count);
 	p_len = strlen(p);
 	copy_len = strnlen(q, count);
-	if (p_size < p_len + copy_len + 1)
-		fortify_panic(__func__);
+	size = p_len + copy_len + 1;
+	if (p_size < size)
+		fortify_panic(__func__, 1, p_size, size);
 	__underlying_memcpy(p + p_len, q, copy_len);
 	p[p_len + copy_len] = '\0';
 	return p;
@@ -444,7 +448,7 @@ __FORTIFY_INLINE void fortify_memset_chk(__kernel_size_t size,
 	 * lengths are unknown.)
 	 */
 	if (p_size != SIZE_MAX && p_size < size)
-		fortify_panic("memset");
+		fortify_panic("memset", 1, p_size, size);
 }
 
 #define __fortify_memset_chk(p, c, size, p_size, p_size_field) ({	\
@@ -542,9 +546,10 @@ __FORTIFY_INLINE bool fortify_memcpy_chk(__kernel_size_t size,
 	 * (The SIZE_MAX test is to optimize away checks where the buffer
 	 * lengths are unknown.)
 	 */
-	if ((p_size != SIZE_MAX && p_size < size) ||
-	    (q_size != SIZE_MAX && q_size < size))
-		fortify_panic(func);
+	if (p_size != SIZE_MAX && p_size < size)
+		fortify_panic(func, 1, p_size, size);
+	if (q_size != SIZE_MAX && q_size < size)
+		fortify_panic(func, 0, q_size, size);
 
 	/*
 	 * Warn when writing beyond destination field size.
@@ -644,7 +649,7 @@ __FORTIFY_INLINE void *memscan(void * const POS0 p, int c, __kernel_size_t size)
 	if (__compiletime_lessthan(p_size, size))
 		__read_overflow();
 	if (p_size < size)
-		fortify_panic(__func__);
+		fortify_panic(__func__, 1, p_size, size);
 	return __real_memscan(p, c, size);
 }
 
@@ -660,8 +665,10 @@ int memcmp(const void * const POS0 p, const void * const POS0 q, __kernel_size_t
 		if (__compiletime_lessthan(q_size, size))
 			__read_overflow2();
 	}
-	if (p_size < size || q_size < size)
-		fortify_panic(__func__);
+	if (p_size < size)
+		fortify_panic(__func__, 1, p_size, size);
+	if (q_size < size)
+		fortify_panic(__func__, 0, q_size, size);
 	return __underlying_memcmp(p, q, size);
 }
 
@@ -673,7 +680,7 @@ void *memchr(const void * const POS0 p, int c, __kernel_size_t size)
 	if (__compiletime_lessthan(p_size, size))
 		__read_overflow();
 	if (p_size < size)
-		fortify_panic(__func__);
+		fortify_panic(__func__, 1, p_size, size);
 	return __underlying_memchr(p, c, size);
 }
 
@@ -685,7 +692,7 @@ __FORTIFY_INLINE void *memchr_inv(const void * const POS0 p, int c, size_t size)
 	if (__compiletime_lessthan(p_size, size))
 		__read_overflow();
 	if (p_size < size)
-		fortify_panic(__func__);
+		fortify_panic(__func__, 1, p_size, size);
 	return __real_memchr_inv(p, c, size);
 }
 
@@ -698,7 +705,7 @@ __FORTIFY_INLINE void *kmemdup(const void * const POS0 p, size_t size, gfp_t gfp
 	if (__compiletime_lessthan(p_size, size))
 		__read_overflow();
 	if (p_size < size)
-		fortify_panic(__func__);
+		fortify_panic(__func__, 1, p_size, size);
 	return __real_kmemdup(p, size, gfp);
 }
 
@@ -735,7 +742,7 @@ char *strcpy(char * const POS p, const char * const POS q)
 		__write_overflow();
 	/* Run-time check for dynamic size overflow. */
 	if (p_size < size)
-		fortify_panic(__func__);
+		fortify_panic(__func__, 1, p_size, size);
 	__underlying_memcpy(p, q, size);
 	return p;
 }
diff --git a/lib/string_helpers.c b/lib/string_helpers.c
index 230020a2e076..b2d3e1d3931e 100644
--- a/lib/string_helpers.c
+++ b/lib/string_helpers.c
@@ -1021,9 +1021,10 @@ EXPORT_SYMBOL(__read_overflow2_field);
 void __write_overflow_field(size_t avail, size_t wanted) { }
 EXPORT_SYMBOL(__write_overflow_field);
 
-void fortify_panic(const char *name)
+void fortify_panic(const char *name, bool dest, size_t avail, size_t len)
 {
-	pr_emerg("detected buffer overflow in %s\n", name);
+	pr_emerg("%s: detected buffer overflow: %zu byte %s buffer of size %zu\n",
+		 name, len, dest ? "write to" : "read from", avail);
 	BUG();
 }
 EXPORT_SYMBOL(fortify_panic);
-- 
2.34.1


             reply	other threads:[~2023-03-02 22:58 UTC|newest]

Thread overview: 3+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2023-03-02 22:58 Kees Cook [this message]
2023-03-02 23:21 ` [PATCH] fortify: Improve buffer overflow reporting Nick Desaulniers
2023-03-03  1:37   ` Kees Cook

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=20230302225808.never.375-kees@kernel.org \
    --to=keescook@chromium.org \
    --cc=akpm@linux-foundation.org \
    --cc=andy@kernel.org \
    --cc=broonie@kernel.org \
    --cc=cezary.rojewski@intel.com \
    --cc=elver@google.com \
    --cc=jpoimboe@kernel.org \
    --cc=kuba@kernel.org \
    --cc=linus.walleij@linaro.org \
    --cc=linux-hardening@vger.kernel.org \
    --cc=linux-kbuild@vger.kernel.org \
    --cc=linux-kernel@vger.kernel.org \
    --cc=llvm@lists.linux.dev \
    --cc=masahiroy@kernel.org \
    --cc=mbenes@suse.cz \
    --cc=nathan@kernel.org \
    --cc=ndesaulniers@google.com \
    --cc=nicolas@fjasle.eu \
    --cc=puyou.lu@gmail.com \
    --cc=trix@redhat.com \
    /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