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@xxxxxxxxxxx/
> >> [2] https://lore.kernel.org/bpf/CAADnVQ+sj9XhscN9PdmTzjVa7Eif21noAUH3y1K6x5bWcL-5pg@xxxxxxxxxxxxxx/
> >> [3] https://lore.kernel.org/bpf/20250707-btf_skip_structs_on_stack-v3-0-29569e086c12@xxxxxxxxxxx/
> >>
> >> Signed-off-by: Alexis Lothoré (eBPF Foundation) <alexis.lothore@xxxxxxxxxxx>
> >> ---
> >> 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
