All of lore.kernel.org
 help / color / mirror / Atom feed
From: Jan Hendrik Farr <kernel@jfarr.cc>
To: Nathan Chancellor <nathan@kernel.org>
Cc: Bill Wendling <morbo@google.com>, Kees Cook <kees@kernel.org>,
	Thorsten Blum <thorsten.blum@toblux.com>,
	kent.overstreet@linux.dev, regressions@lists.linux.dev,
	linux-bcachefs@vger.kernel.org, linux-hardening@vger.kernel.org,
	linux-kernel@vger.kernel.org, ardb@kernel.org, ojeda@kernel.org
Subject: Re: [REGRESSION][BISECTED] erroneous buffer overflow detected in bch2_xattr_validate
Date: Thu, 24 Oct 2024 15:16:50 +0200	[thread overview]
Message-ID: <ZxpIwkfg9_mHO3lq@archlinux> (raw)
In-Reply-To: <20241021192557.GA2041610@thelio-3990X>

Hi Nathan,

Do you want me to add a Co-Developed-by tag for you? I feel bad just
taking it.

For reference here is the current state of the patch, still waiting on
the merge into clang 19.1.x:

It needs three prerequisite commits on top of 6.6.x, but unfortunately
still requires a small amount of manual conflict resolution, but it's
easy enough

1. include/linux/compiler_types.h:
	use the incoming change until before (but not including) the
	"Apply __counted_by() when the Endianness matches to increase test coverage."
	comment)

2. lib/overflow_kunit.c: 
	HEAD is correct


From 6c667a43af0c57cd3f260fd75d5c4a198ba94220 Mon Sep 17 00:00:00 2001
From: Jan Hendrik Farr <kernel@jfarr.cc>
Date: Thu, 17 Oct 2024 04:39:40 +0200
Subject: [PATCH] Compiler Attributes: disable __counted_by for clang < 19.1.3

This patch disables __counted_by for clang versions < 19.1.3 because
of the two issues listed below. It does this by introducing
CONFIG_CC_HAS_COUNTED_BY.

1. clang < 19.1.2 has a bug that can lead to __bdos returning 0:
https://github.com/llvm/llvm-project/pull/110497

2. clang < 19.1.3 has a bug that can lead to __bdos being off by 4:
https://github.com/llvm/llvm-project/pull/112636

Fixes: c8248faf3ca2 ("Compiler Attributes: counted_by: Adjust name and identifier expansion")
Cc: stable@vger.kernel.org # 6.6.x: 16c31dd7fdf6: Compiler Attributes: counted_by: bump min gcc version
Cc: stable@vger.kernel.org # 6.6.x: 2993eb7a8d34: Compiler Attributes: counted_by: fixup clang URL
Cc: stable@vger.kernel.org # 6.6.x: 231dc3f0c936: lkdtm/bugs: Improve warning message for compilers without counted_by support
Cc: stable@vger.kernel.org # 6.6.x
Reported-by: Nathan Chancellor <nathan@kernel.org>
Closes: https://lore.kernel.org/all/20240913164630.GA4091534@thelio-3990X/
Reported-by: kernel test robot <oliver.sang@intel.com>
Closes: https://lore.kernel.org/oe-lkp/202409260949.a1254989-oliver.sang@intel.com
Link: https://lore.kernel.org/all/Zw8iawAF5W2uzGuh@archlinux/T/#m204c09f63c076586a02d194b87dffc7e81b8de7b
Signed-off-by: Jan Hendrik Farr <kernel@jfarr.cc>
---
 drivers/misc/lkdtm/bugs.c           |  2 +-
 include/linux/compiler_attributes.h | 13 -------------
 include/linux/compiler_types.h      | 19 +++++++++++++++++++
 init/Kconfig                        |  8 ++++++++
 lib/overflow_kunit.c                |  2 +-
 5 files changed, 29 insertions(+), 15 deletions(-)

diff --git a/drivers/misc/lkdtm/bugs.c b/drivers/misc/lkdtm/bugs.c
index 62ba01525479..376047beea3d 100644
--- a/drivers/misc/lkdtm/bugs.c
+++ b/drivers/misc/lkdtm/bugs.c
@@ -445,7 +445,7 @@ static void lkdtm_FAM_BOUNDS(void)
 
 	pr_err("FAIL: survived access of invalid flexible array member index!\n");
 
-	if (!__has_attribute(__counted_by__))
+	if (!IS_ENABLED(CONFIG_CC_HAS_COUNTED_BY))
 		pr_warn("This is expected since this %s was built with a compiler that does not support __counted_by\n",
 			lkdtm_kernel_info);
 	else if (IS_ENABLED(CONFIG_UBSAN_BOUNDS))
diff --git a/include/linux/compiler_attributes.h b/include/linux/compiler_attributes.h
index 32284cd26d52..c16d4199bf92 100644
--- a/include/linux/compiler_attributes.h
+++ b/include/linux/compiler_attributes.h
@@ -94,19 +94,6 @@
 # define __copy(symbol)
 #endif
 
-/*
- * Optional: only supported since gcc >= 15
- * Optional: only supported since clang >= 18
- *
- *   gcc: https://gcc.gnu.org/bugzilla/show_bug.cgi?id=108896
- * clang: https://github.com/llvm/llvm-project/pull/76348
- */
-#if __has_attribute(__counted_by__)
-# define __counted_by(member)		__attribute__((__counted_by__(member)))
-#else
-# define __counted_by(member)
-#endif
-
 /*
  * Optional: not supported by gcc
  * Optional: only supported since clang >= 14.0
diff --git a/include/linux/compiler_types.h b/include/linux/compiler_types.h
index 1a957ea2f4fe..639be0f30b45 100644
--- a/include/linux/compiler_types.h
+++ b/include/linux/compiler_types.h
@@ -323,6 +323,25 @@ struct ftrace_likely_data {
 #define __no_sanitize_or_inline __always_inline
 #endif
 
+/*
+ * Optional: only supported since gcc >= 15
+ * Optional: only supported since clang >= 18
+ *
+ *   gcc: https://gcc.gnu.org/bugzilla/show_bug.cgi?id=108896
+ * clang: https://github.com/llvm/llvm-project/pull/76348
+ *
+ * __bdos on clang < 19.1.2 can erroneously return 0:
+ * https://github.com/llvm/llvm-project/pull/110497
+ *
+ * __bdos on clang < 19.1.3 can be off by 4:
+ * https://github.com/llvm/llvm-project/pull/112636
+ */
+#ifdef CONFIG_CC_HAS_COUNTED_BY
+# define __counted_by(member)		__attribute__((__counted_by__(member)))
+#else
+# define __counted_by(member)
+#endif
+
 /*
  * Apply __counted_by() when the Endianness matches to increase test coverage.
  */
diff --git a/init/Kconfig b/init/Kconfig
index 530a382ee0fe..5f1fe3583f20 100644
--- a/init/Kconfig
+++ b/init/Kconfig
@@ -116,6 +116,14 @@ config CC_HAS_ASM_INLINE
 config CC_HAS_NO_PROFILE_FN_ATTR
 	def_bool $(success,echo '__attribute__((no_profile_instrument_function)) int x();' | $(CC) -x c - -c -o /dev/null -Werror)
 
+# clang needs to be at least 19.1.3 to avoid __bdos miscalculations
+# https://github.com/llvm/llvm-project/pull/110497
+# https://github.com/llvm/llvm-project/pull/112636
+# TODO: when gcc 15 is released remove the build test and add gcc version check
+config CC_HAS_COUNTED_BY
+	def_bool $(success,echo 'struct flex { int count; int array[] __attribute__((__counted_by__(count))); };' | $(CC) $(CLANG_FLAGS) -x c - -c -o /dev/null -Werror)
+	depends on !(CC_IS_CLANG && CLANG_VERSION < 190103)
+
 config PAHOLE_VERSION
 	int
 	default $(shell,$(srctree)/scripts/pahole-version.sh $(PAHOLE))
diff --git a/lib/overflow_kunit.c b/lib/overflow_kunit.c
index 2abc78367dd1..5222c6393f11 100644
--- a/lib/overflow_kunit.c
+++ b/lib/overflow_kunit.c
@@ -1187,7 +1187,7 @@ static void DEFINE_FLEX_test(struct kunit *test)
 {
 	/* Using _RAW_ on a __counted_by struct will initialize "counter" to zero */
 	DEFINE_RAW_FLEX(struct foo, two_but_zero, array, 2);
-#if __has_attribute(__counted_by__)
+#ifdef CONFIG_CC_HAS_COUNTED_BY
 	int expected_raw_size = sizeof(struct foo);
 #else
 	int expected_raw_size = sizeof(struct foo) + 2 * sizeof(s16);
-- 
2.47.0

  reply	other threads:[~2024-10-24 13:16 UTC|newest]

Thread overview: 62+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2024-09-26 15:14 [REGRESSION][BISECTED] erroneous buffer overflow detected in bch2_xattr_validate Jan Hendrik Farr
2024-09-26 15:28 ` Thorsten Blum
2024-09-26 16:09   ` Thorsten Blum
2024-09-26 16:37     ` Jan Hendrik Farr
2024-09-26 17:01     ` Jan Hendrik Farr
2024-09-26 17:45       ` Jan Hendrik Farr
2024-09-26 19:58         ` Ard Biesheuvel
2024-09-26 22:18           ` Bill Wendling
2024-09-27  1:30             ` Bill Wendling
2024-09-27  3:41               ` Jan Hendrik Farr
2024-09-28 20:50               ` Kees Cook
2024-09-28 23:33                 ` Jan Hendrik Farr
2024-09-29 19:59                   ` Jan Hendrik Farr
2024-09-28 17:36     ` Jan Hendrik Farr
2024-09-28 17:49       ` Jan Hendrik Farr
2024-09-28 20:34       ` Kees Cook
2024-10-02  9:18         ` Thorsten Blum
2024-10-03 11:33           ` Jan Hendrik Farr
2024-10-03 13:07             ` Thorsten Blum
2024-10-03 13:12               ` Jan Hendrik Farr
2024-10-03 15:02                 ` Thorsten Blum
2024-10-03 15:22                   ` Jan Hendrik Farr
2024-10-03 15:30                     ` Thorsten Blum
2024-10-03 15:35                       ` Jan Hendrik Farr
2024-10-03 15:43                         ` Thorsten Blum
2024-10-03 16:32                           ` Jan Hendrik Farr
2024-10-03 15:17               ` Jan Hendrik Farr
2024-10-03 21:28                 ` Kees Cook
2024-10-03 21:48                   ` Jan Hendrik Farr
2024-10-04 17:13                     ` Kees Cook
2024-10-07  3:56                       ` Jan Hendrik Farr
2024-10-07 15:10                         ` Jan Hendrik Farr
2024-10-16 21:13                           ` Kees Cook
2024-10-16 23:41                         ` Bill Wendling
2024-10-17  0:09                           ` Bill Wendling
2024-10-17  3:04                             ` Jan Hendrik Farr
2024-10-17 16:55                               ` Nathan Chancellor
2024-10-17 17:39                                 ` Miguel Ojeda
2024-10-17 18:55                                   ` Nathan Chancellor
2024-10-18 11:52                                     ` Miguel Ojeda
2024-10-21  1:33                                 ` Jan Hendrik Farr
2024-10-21  6:04                                   ` Miguel Ojeda
2024-10-21 17:01                                     ` Jan Hendrik Farr
2024-10-21 19:25                                   ` Nathan Chancellor
2024-10-24 13:16                                     ` Jan Hendrik Farr [this message]
2024-10-25  1:15                                       ` Nathan Chancellor
2024-10-25  8:10                                         ` Miguel Ojeda
2024-10-25 15:27                                           ` Jan Hendrik Farr
2025-05-01 14:30                                             ` Alan Huang
2025-05-01 16:45                                               ` Jan Hendrik Farr
2025-05-01 17:22                                               ` Jan Hendrik Farr
2025-05-01 17:28                                                 ` Alan Huang
2025-05-01 17:58                                                   ` Jan Hendrik Farr
2025-05-01 18:10                                                     ` Kees Cook
2025-05-01 18:18                                                     ` Alan Huang
2024-10-17  0:41                           ` Jan Hendrik Farr
2024-10-14 21:39                       ` Bill Wendling
2024-10-16  1:22             ` Bill Wendling
2024-10-16  2:18               ` Jan Hendrik Farr
2024-10-16 20:43                 ` Kees Cook
2024-10-03 21:23           ` Kees Cook
2024-10-03 22:05             ` Jan Hendrik Farr

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=ZxpIwkfg9_mHO3lq@archlinux \
    --to=kernel@jfarr.cc \
    --cc=ardb@kernel.org \
    --cc=kees@kernel.org \
    --cc=kent.overstreet@linux.dev \
    --cc=linux-bcachefs@vger.kernel.org \
    --cc=linux-hardening@vger.kernel.org \
    --cc=linux-kernel@vger.kernel.org \
    --cc=morbo@google.com \
    --cc=nathan@kernel.org \
    --cc=ojeda@kernel.org \
    --cc=regressions@lists.linux.dev \
    --cc=thorsten.blum@toblux.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 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.