[PATCH 1/2] bpf, arm64: remove structs on stack constraint

Alexis Lothoré (eBPF Foundation) posted 2 patches 3 months ago
[PATCH 1/2] bpf, arm64: remove structs on stack constraint
Posted by Alexis Lothoré (eBPF Foundation) 3 months ago
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@bootlin.com>
---
 arch/arm64/net/bpf_jit_comp.c | 5 -----
 1 file changed, 5 deletions(-)

diff --git a/arch/arm64/net/bpf_jit_comp.c b/arch/arm64/net/bpf_jit_comp.c
index b6c42b5c96688251ea24f5e771fa1effff896541..89b1b8c248c62e09cec61e13318d45b59006dce1 100644
--- a/arch/arm64/net/bpf_jit_comp.c
+++ b/arch/arm64/net/bpf_jit_comp.c
@@ -2240,11 +2240,6 @@ static int calc_arg_aux(const struct btf_func_model *m,
 
 	/* the rest arguments are passed through stack */
 	for (; i < m->nr_args; i++) {
-		/* We can not know for sure about exact alignment needs for
-		 * struct passed on stack, so deny those
-		 */
-		if (m->arg_flags[i] & BTF_FMODEL_STRUCT_ARG)
-			return -ENOTSUPP;
 		stack_slots = (m->arg_size[i] + 7) / 8;
 		a->bstack_for_args += stack_slots * 8;
 		a->ostack_for_args = a->ostack_for_args + stack_slots * 8;

-- 
2.50.0

Re: [PATCH 1/2] bpf, arm64: remove structs on stack constraint
Posted by Will Deacon 2 months, 3 weeks ago
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@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?

Will
Re: [PATCH 1/2] bpf, arm64: remove structs on stack constraint
Posted by Alexis Lothoré 2 months, 3 weeks ago
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@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]).

Alexis

[1] https://lore.kernel.org/bpf/CAADnVQ+sj9XhscN9PdmTzjVa7Eif21noAUH3y1K6x5bWcL-5pg@mail.gmail.com/


-- 
Alexis Lothoré, Bootlin
Embedded Linux and Kernel engineering
https://bootlin.com
Re: [PATCH 1/2] bpf, arm64: remove structs on stack constraint
Posted by Will Deacon 2 months, 3 weeks ago
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@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.

Will
Re: [PATCH 1/2] bpf, arm64: remove structs on stack constraint
Posted by Alexei Starovoitov 2 months, 3 weeks ago
On Tue, Jul 15, 2025 at 7:31 AM Will Deacon <will@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@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".
Re: [PATCH 1/2] bpf, arm64: remove structs on stack constraint
Posted by Alexis Lothoré 2 months, 3 weeks ago
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@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@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