linux-hardening - Re: [REGRESSION][BISECTED] erroneous buffer overflow detected in bch2_xattr

lists.openwall.net		lists / announce owl-users owl-dev john-users john-dev passwdqc-users yescrypt popa3d-users / oss-security kernel-hardening musl sabotage tlsify passwords / crypt-dev xvendor / Bugtraq Full-Disclosure linux-kernel linux-netdev linux-ext4 linux-hardening linux-cve-announce PHC
Open Source and information security mailing list archives

Hash Suite: Windows password security audit tool. GUI, reports in PDF.

[<prev] [next>] [<thread-prev] [thread-next>] [day] [month] [year] [list]

Message-ID: <ZxB-uh1KzfD4ww2a@archlinux>
Date: Thu, 17 Oct 2024 05:04:26 +0200
From: Jan Hendrik Farr <kernel@...rr.cc>
To: Bill Wendling <morbo@...gle.com>
Cc: Kees Cook <kees@...nel.org>, Thorsten Blum <thorsten.blum@...lux.com>,
	kent.overstreet@...ux.dev, regressions@...ts.linux.dev,
	linux-bcachefs@...r.kernel.org, linux-hardening@...r.kernel.org,
	linux-kernel@...r.kernel.org, ardb@...nel.org
Subject: Re: [REGRESSION][BISECTED] erroneous buffer overflow detected in
 bch2_xattr_validate

On 16 17:09:42, Bill Wendling wrote:
> On Wed, Oct 16, 2024 at 4:41 PM Bill Wendling <morbo@...gle.com> wrote:
> >
> > On Sun, Oct 6, 2024 at 8:56 PM Jan Hendrik Farr <kernel@...rr.cc> wrote:
> > > > I want to separate several easily confused issues. Instead of just
> > > > saying __bdos, let's clearly refer to what calculation within bdos is
> > > > being used. There are 3 choices currently:
> > > > - alloc_size attribute
> > > > - counted_by attribute
> > > > - fallback to __bos (which is similar to sizeof(), except that FAMs are 0 sized)
> > > >
> > > > Additionally there are (for all intents and purposes) 2 size
> > > > determinations to be made by __bos and __bdos, via argument 2:
> > > > - containing object size (type 0) ("maximum size")
> > > > - specific object size (type 1) ("minimum size")
> > >
> > > "maximum" vs "minimum" size would by type 0 vs type 2, but I think you
> > > do mean type 0 and type 1 as those are the types currently used by
> > > __struct_size and __member_size. Those are both "maximum" sizes.
> > >
> > > >
> > > > For example, consider:
> > > >
> > > > struct posix_acl *acl = malloc(1024);
> > > > acl->a_count = 1;
> > > >
> > > > what should these return:
> > > >
> > > >       __bos(acl, 0)
> > > >       __bos(acl, 1)
> > > >       __bdos(acl, 0)
> > > >       __bdos(acl, 1)
> > > >       __bos(acl->a_entries, 0)
> > > >       __bos(acl->a_entries, 1)
> > > >       __bdos(acl->a_entries, 0)
> > > >       __bdos(acl->a_entries, 1)
> > > >
> > >
> > Thank you for this detailed write-up! I'm sorry for my late response.
> >
> [snip]
> >
> > So in conclusion, if turning off the calculation for a pointer to the
> > whole struct works, then I'm okay with it.
> >
> Here's a potential fix:
> 
>   https://github.com/llvm/llvm-project/pull/112636

Here's the patch to disable __counted_by for clang < 19.1.3. I'll submit
it properly when your PR is merged. I hope I got all the correct tags in
there as there were multiple reports of these issues. Let me know if
anything should be added, I'm new to the process.

From: Jan Hendrik Farr <kernel@...rr.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
two issues.

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

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

Cc: stable@...r.kernel.org
Reported-by: Nathan Chancellor <nathan@...nel.org>
Reported-by: kernel test robot <oliver.sang@...el.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@...rr.cc>
---
 include/linux/compiler_attributes.h | 11 ++++++++++-
 1 file changed, 10 insertions(+), 1 deletion(-)

diff --git a/include/linux/compiler_attributes.h b/include/linux/compiler_attributes.h
index 32284cd26d52..7966a533aaec 100644
--- a/include/linux/compiler_attributes.h
+++ b/include/linux/compiler_attributes.h
@@ -100,8 +100,17 @@
  *
  *   gcc: https://gcc.gnu.org/bugzilla/show_bug.cgi?id=108896
  * clang: https://github.com/llvm/llvm-project/pull/76348
+ *
+ * clang versions < 19.1.2 have a bug that can lead to __bdos returning 0:
+ * https://github.com/llvm/llvm-project/pull/110497
+ *
+ * clang versions < 19.1.3 have a bug that can lead to __bdos being off by 4:
+ * https://github.com/llvm/llvm-project/pull/112636
  */
-#if __has_attribute(__counted_by__)
+#if __has_attribute(__counted_by__) && \
+	(!defined(__clang__) || (__clang_major__ > 19) || \
+	(__clang_major__ == 19 && (__clang_minor__ > 1 || \
+	(__clang_minor__ == 1 && __clang_patchlevel__ >= 3))))
 # define __counted_by(member)		__attribute__((__counted_by__(member)))
 #else
 # define __counted_by(member)
-- 
2.47.0