Re: [PATCH v5 06/26] tools build: Append -fzero-init-padding-bits=all to extra cflags

[email protected]
Newsgroups org.kernel.vger.linux-perf-users,org.kernel.vger.bpf
Message-ID <[email protected]>
Thank you for your contribution! Sashiko AI review found 4 potential issue(s) to consider:
- [High] Unconditional appending to `EXTRA_CFLAGS` breaks default optimization and debug flags for `libbpf`.
- [High] The `try-run` macro attempts to write to the source tree during Makefile parsing, breaking out-of-tree builds on read-only filesystems.
- [Medium] The compiler flag `-fzero-init-padding-bits=all` is appended to `HOST_EXTRACFLAGS`, which is never evaluated by the tools build framework, resulting in silent omission of the flag for host programs like bpftool.
- [Medium] The `try-run` macro uses a predictable temporary directory name (`.tmp_$$`), exposing the build process to a local symlink attack (CWE-377).
--

--- Patch [6]: [PATCH v5 06/26] tools build: Append -fzero-init-padding-bits=all to extra cflags ---
commit 526bce400539d6fdc8b0869f05e77b5305dcbce2
Author: Leo Yan <[email protected]>

tools build: Append -fzero-init-padding-bits=all to extra cflags

This commit adds -fzero-init-padding-bits=all for tools building to
mitigate a new GCC-15 union initialization behavior. It implements cc-option
and host-cc-option macros to dynamically detect support for this flag.

Link: https://gcc.gnu.org/gcc-15/changes.html

> diff --git a/tools/scripts/Makefile.include b/tools/scripts/Makefile.include
> index b5ecf137febca..a2397ceae512c 100644
> --- a/tools/scripts/Makefile.include
> +++ b/tools/scripts/Makefile.include

[ ... ]

> @@ -137,6 +137,36 @@ else
>  EXTRA_WARNINGS += -Wshadow
>  endif
>  
> +# output directory for tests below
> +TMPOUT = .tmp_$$$$
> +
> +# try-run
> +# Usage: option = $(call try-run, $(CC)...-o "$$TMP",option-ok,otherwise)
> +# Exit code chooses option. "$$TMP" serves as a temporary file and is
> +# automatically cleaned up.
> +try-run = $(shell set -e;		\
> +	TMP=$(TMPOUT)/tmp;		\

[Severity: Medium]
Is this predictable temporary directory safe against local symlink attacks?

If a user builds the kernel tools in a shared directory, an attacker could
pre-create .tmp_<PID> directories containing a symlink named tmp pointing
to a target file. Because mkdir -p succeeds on existing directories, the
compiler command overwrites the attacker's symlink target.

> +	trap "rm -rf $(TMPOUT)" EXIT;	\
> +	mkdir -p $(TMPOUT);		\

[Severity: High]
Does this directory creation break out-of-tree builds on read-only trees?

Unlike Kbuild, tools Makefiles evaluate shell commands in the source tree
directory. This attempts to write to the source tree instead of the
designated output directory. The mkdir fails on a read-only filesystem,
and set -e aborts the snippet, dropping the compiler flag check.

> +	if ($(1)) >/dev/null 2>&1;	\
> +	then echo "$(2)";		\
> +	else echo "$(3)";		\
> +	fi)

[ ... ]

> +# Explicitly clear padding bits with the initializer '{ 0 }'
> +FLAG_ZERO_INIT := $(call cc-option,-fzero-init-padding-bits=all)
> +override EXTRA_CFLAGS += $(FLAG_ZERO_INIT)

[Severity: High]
Will unconditionally appending to EXTRA_CFLAGS break the default optimization
and debug flags for libraries like libbpf?

Because Makefile.include is included before libbpf evaluates its flags,
EXTRA_CFLAGS is always defined. This forces libbpf to bypass its fallback
logic:

tools/lib/bpf/Makefile:
  ifdef EXTRA_CFLAGS
    CFLAGS := $(EXTRA_CFLAGS)
  else
    CFLAGS := -g -O2
  endif

and builds the library without debug symbols and optimizations.

> +HOST_FLAG_ZERO_INIT := $(call host-cc-option,-fzero-init-padding-bits=all)
> +override HOST_EXTRACFLAGS += $(HOST_FLAG_ZERO_INIT)

[Severity: Medium]
Is HOST_EXTRACFLAGS evaluated by the standalone tools build framework?

The tools build framework relies exclusively on HOSTCFLAGS for host programs,
so appending to HOST_EXTRACFLAGS causes the flag to be ignored for tools
like bpftool.

> +
>  ifneq ($(findstring $(MAKEFLAGS), w),w)
>  PRINT_DIR = --no-print-directory
>  else

-- 
Sashiko AI review · https://sashiko.dev/#/patchset/[email protected]?part=6
lmpx.com only provides a reader for public news (NNTP) servers. It is not affiliated with the servers or forums shown here and is not responsible for the content of articles, which is written by their respective authors.