All of lore.kernel.org
 help / color / mirror / Atom feed
From: Kees Cook <keescook@chromium.org>
To: linux-hardening@vger.kernel.org
Cc: Kees Cook <keescook@chromium.org>, linux-kernel@vger.kernel.org
Subject: [PATCH 16/17] fortify: Detect struct member overflows in memset() at compile-time
Date: Mon, 13 Dec 2021 14:33:30 -0800	[thread overview]
Message-ID: <20211213223331.135412-17-keescook@chromium.org> (raw)
In-Reply-To: <20211213223331.135412-1-keescook@chromium.org>

As done for memcpy(), also update memset() to use the same tightened
compile-time bounds checking under CONFIG_FORTIFY_SOURCE.

Signed-off-by: Kees Cook <keescook@chromium.org>
---
 include/linux/fortify-string.h                | 54 ++++++++++++++++---
 .../write_overflow_field-memset.c             |  5 ++
 2 files changed, 51 insertions(+), 8 deletions(-)
 create mode 100644 lib/test_fortify/write_overflow_field-memset.c

diff --git a/include/linux/fortify-string.h b/include/linux/fortify-string.h
index c07871a3fcd0..c45159dbdaa1 100644
--- a/include/linux/fortify-string.h
+++ b/include/linux/fortify-string.h
@@ -200,17 +200,56 @@ __FORTIFY_INLINE char *strncat(char *p, const char *q, __kernel_size_t count)
 	return p;
 }
 
-__FORTIFY_INLINE void *memset(void *p, int c, __kernel_size_t size)
+__FORTIFY_INLINE void fortify_memset_chk(__kernel_size_t size,
+					 const size_t p_size,
+					 const size_t p_size_field)
 {
-	size_t p_size = __builtin_object_size(p, 0);
+	if (__builtin_constant_p(size)) {
+		/*
+		 * Length argument is a constant expression, so we
+		 * can perform compile-time bounds checking where
+		 * buffer sizes are known.
+		 */
 
-	if (__builtin_constant_p(size) && p_size < size)
-		__write_overflow();
-	if (p_size < size)
-		fortify_panic(__func__);
-	return __underlying_memset(p, c, size);
+		/* Error when size is larger than enclosing struct. */
+		if (p_size > p_size_field && p_size < size)
+			__write_overflow();
+
+		/* Warn when write size is larger than dest field. */
+		if (p_size_field < size)
+			__write_overflow_field(p_size_field, size);
+	}
+	/*
+	 * At this point, length argument may not be a constant expression,
+	 * so run-time bounds checking can be done where buffer sizes are
+	 * known. (This is not an "else" because the above checks may only
+	 * be compile-time warnings, and we want to still warn for run-time
+	 * overflows.)
+	 */
+
+	/*
+	 * Always stop accesses beyond the struct that contains the
+	 * field, when the buffer's remaining size is known.
+	 * (The -1 test is to optimize away checks where the buffer
+	 * lengths are unknown.)
+	 */
+	if (p_size != (size_t)(-1) && p_size < size)
+		fortify_panic("memset");
 }
 
+#define __fortify_memset_chk(p, c, size, p_size, p_size_field) ({	\
+	size_t __fortify_size = (size_t)(size);				\
+	fortify_memset_chk(__fortify_size, p_size, p_size_field),	\
+	__underlying_memset(p, c, __fortify_size);			\
+})
+
+/*
+ * __builtin_object_size() must be captured here to avoid evaluating argument
+ * side-effects further into the macro layers.
+ */
+#define memset(p, c, s) __fortify_memset_chk(p, c, s,			\
+		__builtin_object_size(p, 0), __builtin_object_size(p, 1))
+
 /*
  * To make sure the compiler can enforce protection against buffer overflows,
  * memcpy(), memmove(), and memset() must not be used beyond individual
@@ -401,7 +440,6 @@ __FORTIFY_INLINE char *strcpy(char *p, const char *q)
 /* Don't use these outside the FORITFY_SOURCE implementation */
 #undef __underlying_memchr
 #undef __underlying_memcmp
-#undef __underlying_memset
 #undef __underlying_strcat
 #undef __underlying_strcpy
 #undef __underlying_strlen
diff --git a/lib/test_fortify/write_overflow_field-memset.c b/lib/test_fortify/write_overflow_field-memset.c
new file mode 100644
index 000000000000..2331da26909e
--- /dev/null
+++ b/lib/test_fortify/write_overflow_field-memset.c
@@ -0,0 +1,5 @@
+// SPDX-License-Identifier: GPL-2.0-only
+#define TEST	\
+	memset(instance.buf, 0x42, sizeof(instance.buf) + 1)
+
+#include "test_fortify.h"
-- 
2.30.2


  parent reply	other threads:[~2021-12-13 22:39 UTC|newest]

Thread overview: 43+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2021-12-13 22:33 [PATCH 00/17] Enable strict compile-time memcpy() fortify checks Kees Cook
2021-12-13 22:33 ` [PATCH 01/17] KVM: x86: Replace memset() "optimization" with normal per-field writes Kees Cook
2021-12-13 22:33 ` [PATCH 02/17] net/mlx5e: Avoid field-overflowing memcpy() Kees Cook
2021-12-13 22:33 ` [PATCH 03/17] net/mlx5e: Use struct_group() for memcpy() region Kees Cook
2021-12-13 22:33 ` [PATCH 04/17] media: omap3isp: " Kees Cook
2021-12-13 22:33 ` [PATCH 05/17] sata_fsl: " Kees Cook
2021-12-13 22:33 ` [PATCH 06/17] fortify: Detect struct member overflows in memcpy() at compile-time Kees Cook
2021-12-14  3:56   ` kernel test robot
2021-12-14  8:46   ` kernel test robot
2021-12-14 11:50   ` kernel test robot
2021-12-14 16:32   ` kernel test robot
2021-12-14 19:06   ` kernel test robot
2021-12-16  8:56   ` kernel test robot
2021-12-16 11:08   ` Mark Rutland
2021-12-16 11:21     ` Mark Rutland
2021-12-16 18:00     ` Kees Cook
2021-12-17 13:34       ` Mark Rutland
2021-12-13 22:33 ` [PATCH 07/17] fortify: Detect struct member overflows in memmove() " Kees Cook
2021-12-13 22:33 ` [PATCH 08/17] ath11k: Use memset_startat() for clearing queue descriptors Kees Cook
2021-12-13 22:33   ` Kees Cook
2021-12-14  6:02   ` Kalle Valo
2021-12-14  6:02     ` Kalle Valo
2021-12-14 15:46     ` Kalle Valo
2021-12-14 15:46       ` Kalle Valo
2021-12-14 17:05       ` Kees Cook
2021-12-14 17:05         ` Kees Cook
2021-12-16 13:50         ` Kalle Valo
2021-12-16 13:50           ` Kalle Valo
2021-12-13 22:33 ` [PATCH 09/17] RDMA/mlx5: Use memset_after() to zero struct mlx5_ib_mr Kees Cook
2021-12-13 22:33 ` [PATCH 10/17] drbd: Use struct_group() to zero algs Kees Cook
2021-12-13 22:33 ` [PATCH 11/17] dm integrity: Use struct_group() to zero struct journal_sector Kees Cook
2021-12-13 22:33   ` [dm-devel] " Kees Cook
2021-12-13 22:33 ` [PATCH 12/17] iw_cxgb4: Use memset_startat() for cpl_t5_pass_accept_rpl Kees Cook
2021-12-13 22:33 ` [PATCH 13/17] intel_th: msu: Use memset_startat() for clearing hw header Kees Cook
2021-12-13 22:33 ` [PATCH 14/17] IB/mthca: Use memset_startat() for clearing mpt_entry Kees Cook
2021-12-13 22:33 ` [PATCH 15/17] scsi: lpfc: Use struct_group() to initialize struct lpfc_cgn_info Kees Cook
2021-12-13 22:33 ` Kees Cook [this message]
2021-12-14 12:31   ` [PATCH 16/17] fortify: Detect struct member overflows in memset() at compile-time kernel test robot
2021-12-13 22:33 ` [PATCH 17/17] fortify: Work around Clang inlining bugs Kees Cook
2021-12-14 13:22   ` kernel test robot
2021-12-14 13:22     ` kernel test robot
2021-12-15  0:26 ` [PATCH 00/17] Enable strict compile-time memcpy() fortify checks Jason Gunthorpe
2021-12-17  4:04 ` Martin K. Petersen

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=20211213223331.135412-17-keescook@chromium.org \
    --to=keescook@chromium.org \
    --cc=linux-hardening@vger.kernel.org \
    --cc=linux-kernel@vger.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.