linux-kernel - 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: <20241021192557.GA2041610@thelio-3990X>
Date: Mon, 21 Oct 2024 12:25:57 -0700
From: Nathan Chancellor <nathan@...nel.org>
To: Jan Hendrik Farr <kernel@...rr.cc>
Cc: Bill Wendling <morbo@...gle.com>, 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, ojeda@...nel.org
Subject: Re: [REGRESSION][BISECTED] erroneous buffer overflow detected in
 bch2_xattr_validate

On Mon, Oct 21, 2024 at 03:33:36AM +0200, Jan Hendrik Farr wrote:
> > +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)
> > +	# 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
> > +	depends on CC_IS_GCC || CLANG_VERSION >= 190103
> 
> I think I prefer
> 
> 	depends on !(CC_IS_CLANG && CLANG_VERSION < 190103)
> 
> to make it more clear that the purpose is to disable this for clang
> versions below 19.1.3, but keep it enabled for every other compiler
> including pre-release gcc versions that pass the compile test.

Sure, that's a reasonable tweak to keep it a little bit more concise and
to the point. It's obviously logically equivalent.

> Also after gcc 15 is released I don't think a version check for gcc
> should be necessary. I only see an explicit version check as required
> when we know a certain version is broken. Otherwise I would prefer using
> the build test.

Yeah, I think this mostly got addressed with the comments downthread, I
think we are all in agreement.

> I guess an alternative would be to just create a
> CC_COUNTED_BY_BROKEN that is enabled for clang versions below 19.1.3
> and continue to use __has_attribute together with that option. That
> would make the build test unnecesarry. The downside is that it
> will require checking both __has_attribute and
> CONFIG_CC_COUNTED_BY_BROKEN for __counted_by support. So I think
> CC_HAS_COUNTED_BY is better.

Yeah I thought about something like that briefly but came to the same
conclusion quickly, especially once I realized how many places were
using __has_attribute for __counted_by already.

> I'll submit it once Bill's fix is in the release/19.x branch. Which
> maintainer should I address this too? You (Nathan), Miguel, Kees, or
> someone else?

Like Miguel said, you can send it to all the people you have mentioned
here but I would probably expect Kees to chauffeur this to Linus with
Miguel's Ack for compiler_attributes.h since Kees has generally owned
__counted_by up until this point.

Cheers,
Nathan