Re: [REGRESSION][BISECTED] erroneous buffer overflow detected in bch2_xattr_validate

From: Miguel Ojeda
Date: Thu Oct 17 2024 - 13:40:06 EST


On Thu, Oct 17, 2024 at 6:55 PM Nathan Chancellor <nathan@xxxxxxxxxx> wrote:
>
> Should this include a Fixes tag to give the stable folks a hint about
> how far back this should go? Maybe
>
> Fixes: c8248faf3ca2 ("Compiler Attributes: counted_by: Adjust name and identifier expansion")

Yeah, I am not sure -- it does not really fix that commit, but if it
helps the stable team...

> compiler_attributes.h is intended to be free from compiler and version
> checks, so adding a version check means that __counted_by() needs to be

Yeah, ideally we should avoid that since the goal was to have a file
with the straightforward ones.

Though if we do go for `CC_HAS_*`, I guess it would be simple enough
too, i.e. similar to `has_attribute` (but on our side), but it also
loses the simplicity of knowing those do not have arbitrarily complex
conditions which `CC_HAS_*` could hide.

> moved into compiler_types.h. This might be a good opportunity to
> introduce something like CC_HAS_COUNTED_BY in Kconfig, so that we can
> keep the checks unified (since there are already multiple places that
> want to know about __counted_by support for the sake of testing) and
> adjust versions like this easily in the future if something else comes
> up, especially since __counted_by() is not available in a released GCC
> version yet.

Sounds good to me (even if we did the unification somewhere else).
Using `CLANG_VERSION` looks better too.

> +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)

I am probably missing some context, but what is the reason for the
build test? i.e. is there a reason we cannot test the GCC version too?
If the reason it is that it is not released, should we change it
later?

Thanks! (and for the Cc).

Cheers,
Miguel