[PATCH 1/2] bpf, arm64: remove structs on stack constraint
Alexis Lothoré
alexis.lothore at bootlin.com
Wed Jul 16 00:37:59 PDT 2025
On Tue Jul 15, 2025 at 5:40 PM CEST, Alexei Starovoitov wrote:
> On Tue, Jul 15, 2025 at 7:31 AM Will Deacon <will at kernel.org> wrote:
>>
>> 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 at bootlin.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.
>
> I frankly don't see the point in adding and maintaining such checks
> and code in the kernel for hypothetical cases that are not present
> in the kernel and highly unlikely ever be.
> The arm64 jit check was added out of abundance of caution.
> There was way too much "caution".
To complete Alexei's point: the check currently implemented in ARM64 JIT
comp is filtering _too many_ functions. It prevents attachment to any
function consuming a struct passed by value on the stack. But what we
really want is only about filtering those _when their alignment is
altered_, which is something that can not currently be deduced at runtime
in JIT comps. That's part of the reason why this has been moved to pahole.
I would also add that there is another small drawback about keeping this
check on ARM64 only, while not adding it to other JIT comps: we have to
keep filtering out some tests for ARM64, while the feature set is actually
the same as for the other archs. Sure, this discrepancy could be eliminated
once pahole minimum version in kernel build system contains the needed
development, as Will suggests, but I don't see that hapenning before many
months/years.
Alexis
--
Alexis Lothoré, Bootlin
Embedded Linux and Kernel engineering
https://bootlin.com
More information about the linux-arm-kernel
mailing list