Message-ID: <CAADnVQ+sj9XhscN9PdmTzjVa7Eif21noAUH3y1K6x5bWcL-5pg@mail.gmail.com>
Date: Fri, 13 Jun 2025 15:35:57 -0700
From: Alexei Starovoitov <alexei.starovoitov@...il.com>
To: Alexis Lothoré <alexis.lothore@...tlin.com>
Cc: Peter Zijlstra <peterz@...radead.org>, Alexei Starovoitov <ast@...nel.org>, 
	Daniel Borkmann <daniel@...earbox.net>, Andrii Nakryiko <andrii@...nel.org>, 
	Martin KaFai Lau <martin.lau@...ux.dev>, Eduard Zingerman <eddyz87@...il.com>, Song Liu <song@...nel.org>, 
	Yonghong Song <yonghong.song@...ux.dev>, John Fastabend <john.fastabend@...il.com>, 
	KP Singh <kpsingh@...nel.org>, Stanislav Fomichev <sdf@...ichev.me>, Hao Luo <haoluo@...gle.com>, 
	Jiri Olsa <jolsa@...nel.org>, "David S. Miller" <davem@...emloft.net>, David Ahern <dsahern@...nel.org>, 
	Thomas Gleixner <tglx@...utronix.de>, Ingo Molnar <mingo@...hat.com>, Borislav Petkov <bp@...en8.de>, 
	Dave Hansen <dave.hansen@...ux.intel.com>, X86 ML <x86@...nel.org>, 
	"H. Peter Anvin" <hpa@...or.com>, Menglong Dong <imagedong@...cent.com>, 
	Björn Töpel <bjorn@...nel.org>, 
	Pu Lehui <pulehui@...wei.com>, Puranjay Mohan <puranjay@...nel.org>, 
	Paul Walmsley <paul.walmsley@...ive.com>, Palmer Dabbelt <palmer@...belt.com>, 
	Albert Ou <aou@...s.berkeley.edu>, Alexandre Ghiti <alex@...ti.fr>, 
	Ilya Leoshkevich <iii@...ux.ibm.com>, Heiko Carstens <hca@...ux.ibm.com>, Vasily Gorbik <gor@...ux.ibm.com>, 
	Alexander Gordeev <agordeev@...ux.ibm.com>, Christian Borntraeger <borntraeger@...ux.ibm.com>, 
	Sven Schnelle <svens@...ux.ibm.com>, Hari Bathini <hbathini@...ux.ibm.com>, 
	Christophe Leroy <christophe.leroy@...roup.eu>, Naveen N Rao <naveen@...nel.org>, 
	Madhavan Srinivasan <maddy@...ux.ibm.com>, Michael Ellerman <mpe@...erman.id.au>, 
	Nicholas Piggin <npiggin@...il.com>, Mykola Lysenko <mykolal@...com>, Shuah Khan <shuah@...nel.org>, 
	Maxime Coquelin <mcoquelin.stm32@...il.com>, Alexandre Torgue <alexandre.torgue@...s.st.com>, 
	ebpf@...uxfoundation.org, Thomas Petazzoni <thomas.petazzoni@...tlin.com>, 
	Bastien Curutchet <bastien.curutchet@...tlin.com>, 
	Network Development <netdev@...r.kernel.org>, bpf <bpf@...r.kernel.org>, 
	LKML <linux-kernel@...r.kernel.org>, Björn Töpel <bjorn@...osinc.com>, 
	linux-riscv <linux-riscv@...ts.infradead.org>, linux-s390 <linux-s390@...r.kernel.org>, 
	ppc-dev <linuxppc-dev@...ts.ozlabs.org>, 
	"open list:KERNEL SELFTEST FRAMEWORK" <linux-kselftest@...r.kernel.org>, linux-stm32@...md-mailman.stormreply.com, 
	linux-arm-kernel <linux-arm-kernel@...ts.infradead.org>
Subject: Re: [PATCH bpf 2/7] bpf/x86: prevent trampoline attachment when args
 location on stack is uncertain

On Fri, Jun 13, 2025 at 1:59 AM Alexis Lothoré
<alexis.lothore@...tlin.com> wrote:
>
> On Fri Jun 13, 2025 at 10:32 AM CEST, Peter Zijlstra wrote:
> > On Fri, Jun 13, 2025 at 10:26:37AM +0200, Alexis Lothoré wrote:
> >> Hi Peter,
> >>
> >> On Fri Jun 13, 2025 at 10:11 AM CEST, Peter Zijlstra wrote:
> >> > On Fri, Jun 13, 2025 at 09:37:11AM +0200, Alexis Lothoré (eBPF Foundation) wrote:
>
> [...]
>
> >> Maybe my commit wording is not precise enough, but indeed, there's not
> >> doubt about whether the struct value is passed on the stack or through a
> >> register/a pair of registers. The doubt is rather about the struct location
> >> when it is passed _by value_ and _on the stack_: the ABI indeed clearly
> >> states that "Structures and unions assume the alignment of their most
> >> strictly aligned component" (p.13), but this rule is "silently broken" when
> >> a struct has an __attribute__((packed)) or and __attribute__((aligned(X))),
> >> and AFAICT this case can not be detected at runtime with current BTF info.
> >
> > Ah, okay. So it is a failure of BTF. That was indeed not clear.
>
> If I need to respin, I'll rewrite the commit message to include the details
> above.

No need to respin. The cover letter is quite detailed already.

But looking at the patch and this thread I think we need to agree
on the long term approach to BTF, since people assume that
it's a more compact dwarf and any missing information
should be added to it.
Like in this case special alignment case and packed attributes
are not expressed in BTF and I believe they should not be.
BTF is not a debug format and not a substitute for dwarf.
There is no goal to express everything possible in C.
It's minimal, because BTF is _practical_ description of
types and data present in the kernel.
I don't think the special case of packing and alignment exists
in the kernel today, so the current format is sufficient.
It doesn't miss anything.
I think we made arm64 JIT unnecessary restrictive and now considering
to make all other JITs restrictive too for hypothetical case
of some future kernel functions.
I feel we're going in the wrong direction.
Instead we should teach pahole to sanitize BTF where functions
are using this fancy alignment and packed structs.
pahole can see it in dwarf and can skip emitting BTF for such
functions. Then the kernel JITs on all architectures won't even
see such cases.

The issue was initially discovered by a selftest:
https://lore.kernel.org/bpf/20250411-many_args_arm64-v1-3-0a32fe72339e@bootlin.com/
that attempted to support these two types:
+struct bpf_testmod_struct_arg_4 {
+ __u64 a;
+ __u64 b;
+};
+
+struct bpf_testmod_struct_arg_5 {
+ __int128 a;
+};

The former is present in the kernel. It's more or less sockptr_t,
and people want to access it for observability in tracing.
The latter doesn't exist in the kernel and we cannot represent
it properly in BTF without losing alignment.

So I think we should go back to that series:
https://lore.kernel.org/bpf/20250411-many_args_arm64-v1-0-0a32fe72339e@bootlin.com/

remove __int128 selftest, but also teach pahole
to recognize types that cannot be represented in BTF and
don't emit them either into vmlinux or in kernel module
(like in this case it was bpf_testmod.ko)
I think that would be a better path forward aligned
with the long term goal of BTF.

And before people ask... pahole is a trusted component of the build
system. We trust it just as we trust gcc, clang, linker, objtool.

Powered by blists - more mailing lists