[<prev] [next>] [<thread-prev] [thread-next>] [day] [month] [year] [list]
Message-ID: <aHZmOVpcoyTvGY1u@willie-the-truck>
Date: Tue, 15 Jul 2025 15:31:21 +0100
From: Will Deacon <will@...nel.org>
To: Alexis Lothoré <alexis.lothore@...tlin.com>
Cc: 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>,
Puranjay Mohan <puranjay@...nel.org>,
Xu Kuohai <xukuohai@...weicloud.com>,
Catalin Marinas <catalin.marinas@....com>,
Mykola Lysenko <mykolal@...com>, Shuah Khan <shuah@...nel.org>,
ebpf@...uxfoundation.org,
Thomas Petazzoni <thomas.petazzoni@...tlin.com>,
Bastien Curutchet <bastien.curutchet@...tlin.com>,
Ihor Solodrai <ihor.solodrai@...ux.dev>, bpf@...r.kernel.org,
linux-arm-kernel@...ts.infradead.org, linux-kernel@...r.kernel.org,
linux-kselftest@...r.kernel.org
Subject: Re: [PATCH 1/2] bpf, arm64: remove structs on stack constraint
On Tue, Jul 15, 2025 at 04:02:25PM +0200, Alexis Lothoré wrote:
> On Tue Jul 15, 2025 at 3:32 PM CEST, Will Deacon wrote:
> > On Wed, Jul 09, 2025 at 10:36:55AM +0200, Alexis Lothoré (eBPF Foundation) wrote:
> >> While introducing support for 9+ arguments for tracing programs on
> >> ARM64, commit 9014cf56f13d ("bpf, arm64: Support up to 12 function
> >> arguments") has also introduced a constraint preventing BPF trampolines
> >> from being generated if the target function consumes a struct argument
> >> passed on stack, because of uncertainties around the exact struct
> >> location: if the struct has been marked as packed or with a custom
> >> alignment, this info is not reflected in BTF data, and so generated
> >> tracing trampolines could read the target function arguments at wrong
> >> offsets.
> >>
> >> This issue is not specific to ARM64: there has been an attempt (see [1])
> >> to bring the same constraint to other architectures JIT compilers. But
> >> discussions following this attempt led to the move of this constraint
> >> out of the kernel (see [2]): instead of preventing the kernel from
> >> generating trampolines for those functions consuming structs on stack,
> >> it is simpler to just make sure that those functions with uncertain
> >> struct arguments location are not encoded in BTF information, and so
> >> that one can not even attempt to attach a tracing program to such
> >> function. The task is then deferred to pahole (see [3]).
> >>
> >> Now that the constraint is handled by pahole, remove it from the arm64
> >> JIT compiler to keep it simple.
> >>
> >> [1] https://lore.kernel.org/bpf/20250613-deny_trampoline_structs_on_stack-v1-0-5be9211768c3@bootlin.com/
> >> [2] https://lore.kernel.org/bpf/CAADnVQ+sj9XhscN9PdmTzjVa7Eif21noAUH3y1K6x5bWcL-5pg@mail.gmail.com/
> >> [3] https://lore.kernel.org/bpf/20250707-btf_skip_structs_on_stack-v3-0-29569e086c12@bootlin.com/
> >>
> >> Signed-off-by: Alexis Lothoré (eBPF Foundation) <alexis.lothore@...tlin.com>
> >> ---
> >> arch/arm64/net/bpf_jit_comp.c | 5 -----
> >> 1 file changed, 5 deletions(-)
> >
> > This is a question born more out of ignorance that insight, but how do
> > we ensure that the version of pahole being used is sufficiently
> > up-to-date that the in-kernel check is not required?
>
> Based on earlier discussions, I am not convinced it is worth maintaining
> the check depending on the pahole version used in BTF. Other architectures
> exposing a JIT compiler don't have the in-kernel check and so are already
> exposed to this very specific case, but discussions around my attempt to
> enforce the check on other JIT comp showed that the rarity of this case do
> not justify protecting it on kernel side (see [1]).
I can understand why doing this in pahole rather than in each individual
JIT is preferable, but I don't think there's any harm leaving the
existing two line check in arm64 as long as older versions of pahole
might be used, is there? I wouldn't say that removing it really
simplifies the JIT compiler when you consider the rest of the
implementation.
Of course, once the kernel requires a version of pahole recent enough
to contain [3], we should drop the check in the JIT compiler as the
one in pahole looks like it's more selective about the functions it
rejects.
Will
Powered by blists - more mailing lists