From: Namhyung Kim <namhyung@kernel.org>
To: sashiko-reviews@lists.linux.dev
Cc: Trevor Allison <tallison@redhat.com>, linux-perf-users@vger.kernel.org
Subject: Re: [PATCH 3/3] perf x86: Add .note.gnu.property to assembly linked into perf
Date: Mon, 13 Jul 2026 13:33:26 -0700 [thread overview]
Message-ID: <alVLlpmtps__MBCl@google.com> (raw)
In-Reply-To: <20260708050839.BD9491F000E9@smtp.kernel.org>
Hello,
On Wed, Jul 08, 2026 at 05:08:39AM +0000, sashiko-bot@kernel.org wrote:
> Thank you for your contribution! Sashiko AI review found 2 potential issue(s) to consider:
> - [Medium] Adding .note.gnu.property asserts IBT support without providing ENDBR64 instructions, causing a #CP crash in perf memory benchmarks.
> - [Medium] The .note.gnu.property section hardcodes 8-byte alignment, violating the 32-bit x86 ELF specification.
> --
Can you please take a look and address the both comments?
Thanks,
Namhyung
>
> --- Patch [3]: [PATCH 3/3] perf x86: Add .note.gnu.property to assembly linked into perf ---
> commit b48d7edb7fb18bc3dace2af272de108eb561d69b
> Author: Trevor Allison <tallison@redhat.com>
>
> perf x86: Add .note.gnu.property to assembly linked into perf
>
> This commit adds `.note.gnu.property` to x86 assembly files linked into
> perf to indicate CET properties support. This is intended to prevent static
> linking from dropping CET from the final binary.
>
> > diff --git a/tools/perf/arch/x86/tests/regs_load.S b/tools/perf/arch/x86/tests/regs_load.S
> > index 80f14f52e3f60..1947f95cceda5 100644
> > --- a/tools/perf/arch/x86/tests/regs_load.S
> > +++ b/tools/perf/arch/x86/tests/regs_load.S
> [ ... ]
> > @@ -97,3 +97,27 @@ SYM_FUNC_END(perf_regs_load)
> > * the ELF stack should not be restricted at all and set it RWX.
> > */
> > .section .note.GNU-stack,"",@progbits
> > +
> > +/*
> > + * We need to provide .note.gnu.property section with IBT and SHSTK,
> > + * saying that this object supports the same CET properties as C code
> > + * built with -fcf-protection. Otherwise static linking drops CET from
> > + * the final perf binary when other objects are CET-enabled.
> > + */
> > + .pushsection .note.gnu.property, "a"
> > + .p2align 3
>
> [Severity: Medium]
> Does this unconditionally hardcode 8-byte alignment for 32-bit x86 builds?
>
> The assembly file `regs_load.S` can be compiled for 32-bit x86, which requires
> 4-byte alignment (`.p2align 2`) for ELF notes. When the linker merges these,
> the forced 8-byte alignment can create zero-byte padding gaps between notes,
> corrupting the contiguous ELF note structure.
>
> Could this cause the dynamic loader or tools like `readelf` to fail to parse
> the ELF notes properly?
>
> [ ... ]
> > diff --git a/tools/perf/bench/mem-memcpy-x86-64-asm.S b/tools/perf/bench/mem-memcpy-x86-64-asm.S
> > index 1b9fef7efcdcc..e7b84aa43ab44 100644
> > --- a/tools/perf/bench/mem-memcpy-x86-64-asm.S
> > +++ b/tools/perf/bench/mem-memcpy-x86-64-asm.S
> [ ... ]
> > @@ -18,3 +18,27 @@
> > * the ELF stack should not be restricted at all and set it RWX.
> > */
> > .section .note.GNU-stack,"",@progbits
> > +
> > +/*
> > + * We need to provide .note.gnu.property section with IBT and SHSTK,
> > + * saying that this object supports the same CET properties as C code
> > + * built with -fcf-protection. Otherwise static linking drops CET from
> > + * the final perf binary when other objects are CET-enabled.
> > + */
> > + .pushsection .note.gnu.property, "a"
> > + .p2align 3
>
> [Severity: Medium]
> Will this cause a Control Protection Exception (#CP) crash on IBT-capable
> hardware when running `perf bench mem memcpy`?
>
> Adding this section artificially claims IBT support and forces the linker to
> enable CET on the final `perf` binary. However, `perf`'s local linkage headers
> do not automatically emit `endbr64` instructions for `SYM_FUNC_START`.
>
> When `do_memcpy()` in `tools/perf/bench/mem-functions.c` indirectly calls
> these assembly entry points like `memcpy_orig`:
>
> fn(dst, src, size);
>
> the hardware may detect the missing `endbr64` on the indirect branch target
> and throw a #CP fault.
>
> --
> Sashiko AI review · https://sashiko.dev/#/patchset/20260708045606.43649-1-tallison@redhat.com?part=3
prev parent reply other threads:[~2026-07-13 20:33 UTC|newest]
Thread overview: 7+ messages / expand[flat|nested] mbox.gz Atom feed top
2026-07-08 4:56 [PATCH 0/3] perf: Fix missing CET property notes on x86 with -fcf-protection Trevor Allison
2026-07-08 4:56 ` [PATCH 1/3] perf build: Preserve EXTRA_CFLAGS when building in-tree libbpf Trevor Allison
2026-07-08 4:56 ` [PATCH 2/3] perf build: Apply EXTRA_CXXFLAGS to C++ builds Trevor Allison
2026-07-08 5:03 ` sashiko-bot
2026-07-08 4:56 ` [PATCH 3/3] perf x86: Add .note.gnu.property to assembly linked into perf Trevor Allison
2026-07-08 5:08 ` sashiko-bot
2026-07-13 20:33 ` Namhyung Kim [this message]
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=alVLlpmtps__MBCl@google.com \
--to=namhyung@kernel.org \
--cc=linux-perf-users@vger.kernel.org \
--cc=sashiko-reviews@lists.linux.dev \
--cc=tallison@redhat.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 an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.