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
next prev parent 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 a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).