linux-kernel.vger.kernel.org archive mirror
 help / color / mirror / Atom feed
* [PATCH 0/2] bpf, arm64: relax constraint in BPF JIT compiler
@ 2025-07-09  8:36 Alexis Lothoré (eBPF Foundation)
  2025-07-09  8:36 ` [PATCH 1/2] bpf, arm64: remove structs on stack constraint Alexis Lothoré (eBPF Foundation)
                   ` (2 more replies)
  0 siblings, 3 replies; 9+ messages in thread
From: Alexis Lothoré (eBPF Foundation) @ 2025-07-09  8:36 UTC (permalink / raw)
  To: Alexei Starovoitov, Daniel Borkmann, Andrii Nakryiko,
	Martin KaFai Lau, Eduard Zingerman, Song Liu, Yonghong Song,
	John Fastabend, KP Singh, Stanislav Fomichev, Hao Luo, Jiri Olsa,
	Puranjay Mohan, Xu Kuohai, Catalin Marinas, Will Deacon,
	Mykola Lysenko, Shuah Khan
  Cc: ebpf, Thomas Petazzoni, Bastien Curutchet, Ihor Solodrai, bpf,
	linux-arm-kernel, linux-kernel, linux-kselftest,
	Alexis Lothoré (eBPF Foundation)

Hello,
this series follows up on the one introducing 9+ args for tracing
programs [1]. It has been observed with this series that there are cases
for which we can not identify accurately the location of the target
function arguments to prepare correctly the corresponding BPF
trampoline. This is the case for example if:
- the function consumes a struct variable _by value_
- it is passed on the stack (no more register available for it)
- it has some __packed__ or __aligned(X)__ attribute

As a consequence, a small restrictive check has been added to the ARM64
side, highlighting that other arch supporting 9+ args in BPF trampolines
are already suffering from the same issue.  After a bit of discussions
and attempts, the chosen solution is, rather than applying the same
constraint to all JIT compilers, to prevent such function from being
encoded at all in BTF info([2]). As the pahole side is closed to be
integrated, we can now remove the restrictive check from kernel side.
 
[1] https://lore.kernel.org/bpf/20250527-many_args_arm64-v3-0-3faf7bb8e4a2@bootlin.com/
[2] 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>
---
Alexis Lothoré (eBPF Foundation) (2):
      bpf, arm64: remove structs on stack constraint
      selftests/bpf: enable tracing_struct tests for arm64

 arch/arm64/net/bpf_jit_comp.c                | 5 -----
 tools/testing/selftests/bpf/DENYLIST.aarch64 | 1 -
 2 files changed, 6 deletions(-)
---
base-commit: 8da1e37fc84868b50ba6a7cdf082aa3b0d11e006
change-id: 20250708-arm64_relax_jit_comp-e8889647d8d2

Best regards,
-- 
Alexis Lothoré, Bootlin
Embedded Linux and Kernel engineering
https://bootlin.com


^ permalink raw reply	[flat|nested] 9+ messages in thread

* [PATCH 1/2] bpf, arm64: remove structs on stack constraint
  2025-07-09  8:36 [PATCH 0/2] bpf, arm64: relax constraint in BPF JIT compiler Alexis Lothoré (eBPF Foundation)
@ 2025-07-09  8:36 ` Alexis Lothoré (eBPF Foundation)
  2025-07-15 13:32   ` Will Deacon
  2025-07-09  8:36 ` [PATCH 2/2] selftests/bpf: enable tracing_struct tests for arm64 Alexis Lothoré (eBPF Foundation)
  2025-07-17  1:30 ` [PATCH 0/2] bpf, arm64: relax constraint in BPF JIT compiler patchwork-bot+netdevbpf
  2 siblings, 1 reply; 9+ messages in thread
From: Alexis Lothoré (eBPF Foundation) @ 2025-07-09  8:36 UTC (permalink / raw)
  To: Alexei Starovoitov, Daniel Borkmann, Andrii Nakryiko,
	Martin KaFai Lau, Eduard Zingerman, Song Liu, Yonghong Song,
	John Fastabend, KP Singh, Stanislav Fomichev, Hao Luo, Jiri Olsa,
	Puranjay Mohan, Xu Kuohai, Catalin Marinas, Will Deacon,
	Mykola Lysenko, Shuah Khan
  Cc: ebpf, Thomas Petazzoni, Bastien Curutchet, Ihor Solodrai, bpf,
	linux-arm-kernel, linux-kernel, linux-kselftest,
	Alexis Lothoré (eBPF Foundation)

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


^ permalink raw reply related	[flat|nested] 9+ messages in thread

* [PATCH 2/2] selftests/bpf: enable tracing_struct tests for arm64
  2025-07-09  8:36 [PATCH 0/2] bpf, arm64: relax constraint in BPF JIT compiler Alexis Lothoré (eBPF Foundation)
  2025-07-09  8:36 ` [PATCH 1/2] bpf, arm64: remove structs on stack constraint Alexis Lothoré (eBPF Foundation)
@ 2025-07-09  8:36 ` Alexis Lothoré (eBPF Foundation)
  2025-07-17  1:30 ` [PATCH 0/2] bpf, arm64: relax constraint in BPF JIT compiler patchwork-bot+netdevbpf
  2 siblings, 0 replies; 9+ messages in thread
From: Alexis Lothoré (eBPF Foundation) @ 2025-07-09  8:36 UTC (permalink / raw)
  To: Alexei Starovoitov, Daniel Borkmann, Andrii Nakryiko,
	Martin KaFai Lau, Eduard Zingerman, Song Liu, Yonghong Song,
	John Fastabend, KP Singh, Stanislav Fomichev, Hao Luo, Jiri Olsa,
	Puranjay Mohan, Xu Kuohai, Catalin Marinas, Will Deacon,
	Mykola Lysenko, Shuah Khan
  Cc: ebpf, Thomas Petazzoni, Bastien Curutchet, Ihor Solodrai, bpf,
	linux-arm-kernel, linux-kernel, linux-kselftest,
	Alexis Lothoré (eBPF Foundation)

Now that the constraint preventing attachment to functions consuming
struct on stack has been removed from the kernel (and moved to pahole,
with a slightly smarter detection, to prevent only those that are
packed), re-enable the tracing_struct tests for arm64.

Signed-off-by: Alexis Lothoré (eBPF Foundation) <alexis.lothore@bootlin.com>
---
 tools/testing/selftests/bpf/DENYLIST.aarch64 | 1 -
 1 file changed, 1 deletion(-)

diff --git a/tools/testing/selftests/bpf/DENYLIST.aarch64 b/tools/testing/selftests/bpf/DENYLIST.aarch64
deleted file mode 100644
index 12e99c0277a8cbf9e63e8f6d3a108c8a1208407b..0000000000000000000000000000000000000000
--- a/tools/testing/selftests/bpf/DENYLIST.aarch64
+++ /dev/null
@@ -1 +0,0 @@
-tracing_struct/struct_many_args                  # struct_many_args:FAIL:tracing_struct_many_args__attach unexpected error: -524

-- 
2.50.0


^ permalink raw reply related	[flat|nested] 9+ messages in thread

* Re: [PATCH 1/2] bpf, arm64: remove structs on stack constraint
  2025-07-09  8:36 ` [PATCH 1/2] bpf, arm64: remove structs on stack constraint Alexis Lothoré (eBPF Foundation)
@ 2025-07-15 13:32   ` Will Deacon
  2025-07-15 14:02     ` Alexis Lothoré
  0 siblings, 1 reply; 9+ messages in thread
From: Will Deacon @ 2025-07-15 13:32 UTC (permalink / raw)
  To: Alexis Lothoré (eBPF Foundation)
  Cc: Alexei Starovoitov, Daniel Borkmann, Andrii Nakryiko,
	Martin KaFai Lau, Eduard Zingerman, Song Liu, Yonghong Song,
	John Fastabend, KP Singh, Stanislav Fomichev, Hao Luo, Jiri Olsa,
	Puranjay Mohan, Xu Kuohai, Catalin Marinas, Mykola Lysenko,
	Shuah Khan, ebpf, Thomas Petazzoni, Bastien Curutchet,
	Ihor Solodrai, bpf, linux-arm-kernel, linux-kernel,
	linux-kselftest

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

^ permalink raw reply	[flat|nested] 9+ messages in thread

* Re: [PATCH 1/2] bpf, arm64: remove structs on stack constraint
  2025-07-15 13:32   ` Will Deacon
@ 2025-07-15 14:02     ` Alexis Lothoré
  2025-07-15 14:31       ` Will Deacon
  0 siblings, 1 reply; 9+ messages in thread
From: Alexis Lothoré @ 2025-07-15 14:02 UTC (permalink / raw)
  To: Will Deacon
  Cc: Alexei Starovoitov, Daniel Borkmann, Andrii Nakryiko,
	Martin KaFai Lau, Eduard Zingerman, Song Liu, Yonghong Song,
	John Fastabend, KP Singh, Stanislav Fomichev, Hao Luo, Jiri Olsa,
	Puranjay Mohan, Xu Kuohai, Catalin Marinas, Mykola Lysenko,
	Shuah Khan, ebpf, Thomas Petazzoni, Bastien Curutchet,
	Ihor Solodrai, bpf, linux-arm-kernel, linux-kernel,
	linux-kselftest

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


^ permalink raw reply	[flat|nested] 9+ messages in thread

* Re: [PATCH 1/2] bpf, arm64: remove structs on stack constraint
  2025-07-15 14:02     ` Alexis Lothoré
@ 2025-07-15 14:31       ` Will Deacon
  2025-07-15 15:40         ` Alexei Starovoitov
  0 siblings, 1 reply; 9+ messages in thread
From: Will Deacon @ 2025-07-15 14:31 UTC (permalink / raw)
  To: Alexis Lothoré
  Cc: Alexei Starovoitov, Daniel Borkmann, Andrii Nakryiko,
	Martin KaFai Lau, Eduard Zingerman, Song Liu, Yonghong Song,
	John Fastabend, KP Singh, Stanislav Fomichev, Hao Luo, Jiri Olsa,
	Puranjay Mohan, Xu Kuohai, Catalin Marinas, Mykola Lysenko,
	Shuah Khan, ebpf, Thomas Petazzoni, Bastien Curutchet,
	Ihor Solodrai, bpf, linux-arm-kernel, linux-kernel,
	linux-kselftest

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

^ permalink raw reply	[flat|nested] 9+ messages in thread

* Re: [PATCH 1/2] bpf, arm64: remove structs on stack constraint
  2025-07-15 14:31       ` Will Deacon
@ 2025-07-15 15:40         ` Alexei Starovoitov
  2025-07-16  7:37           ` Alexis Lothoré
  0 siblings, 1 reply; 9+ messages in thread
From: Alexei Starovoitov @ 2025-07-15 15:40 UTC (permalink / raw)
  To: Will Deacon
  Cc: Alexis Lothoré, Alexei Starovoitov, Daniel Borkmann,
	Andrii Nakryiko, Martin KaFai Lau, Eduard Zingerman, Song Liu,
	Yonghong Song, John Fastabend, KP Singh, Stanislav Fomichev,
	Hao Luo, Jiri Olsa, Puranjay Mohan, Xu Kuohai, Catalin Marinas,
	Mykola Lysenko, Shuah Khan, ebpf, Thomas Petazzoni,
	Bastien Curutchet, Ihor Solodrai, bpf, linux-arm-kernel, LKML,
	open list:KERNEL SELFTEST FRAMEWORK

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".

^ permalink raw reply	[flat|nested] 9+ messages in thread

* Re: [PATCH 1/2] bpf, arm64: remove structs on stack constraint
  2025-07-15 15:40         ` Alexei Starovoitov
@ 2025-07-16  7:37           ` Alexis Lothoré
  0 siblings, 0 replies; 9+ messages in thread
From: Alexis Lothoré @ 2025-07-16  7:37 UTC (permalink / raw)
  To: Alexei Starovoitov, Will Deacon
  Cc: Alexei Starovoitov, Daniel Borkmann, Andrii Nakryiko,
	Martin KaFai Lau, Eduard Zingerman, Song Liu, Yonghong Song,
	John Fastabend, KP Singh, Stanislav Fomichev, Hao Luo, Jiri Olsa,
	Puranjay Mohan, Xu Kuohai, Catalin Marinas, Mykola Lysenko,
	Shuah Khan, ebpf, Thomas Petazzoni, Bastien Curutchet,
	Ihor Solodrai, bpf, linux-arm-kernel, LKML,
	open list:KERNEL SELFTEST FRAMEWORK

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


^ permalink raw reply	[flat|nested] 9+ messages in thread

* Re: [PATCH 0/2] bpf, arm64: relax constraint in BPF JIT compiler
  2025-07-09  8:36 [PATCH 0/2] bpf, arm64: relax constraint in BPF JIT compiler Alexis Lothoré (eBPF Foundation)
  2025-07-09  8:36 ` [PATCH 1/2] bpf, arm64: remove structs on stack constraint Alexis Lothoré (eBPF Foundation)
  2025-07-09  8:36 ` [PATCH 2/2] selftests/bpf: enable tracing_struct tests for arm64 Alexis Lothoré (eBPF Foundation)
@ 2025-07-17  1:30 ` patchwork-bot+netdevbpf
  2 siblings, 0 replies; 9+ messages in thread
From: patchwork-bot+netdevbpf @ 2025-07-17  1:30 UTC (permalink / raw)
  To: =?utf-8?q?Alexis_Lothor=C3=A9_=3Calexis=2Elothore=40bootlin=2Ecom=3E?=
  Cc: ast, daniel, andrii, martin.lau, eddyz87, song, yonghong.song,
	john.fastabend, kpsingh, sdf, haoluo, jolsa, puranjay, xukuohai,
	catalin.marinas, will, mykolal, shuah, ebpf, thomas.petazzoni,
	bastien.curutchet, ihor.solodrai, bpf, linux-arm-kernel,
	linux-kernel, linux-kselftest

Hello:

This series was applied to bpf/bpf-next.git (master)
by Alexei Starovoitov <ast@kernel.org>:

On Wed, 09 Jul 2025 10:36:54 +0200 you wrote:
> Hello,
> this series follows up on the one introducing 9+ args for tracing
> programs [1]. It has been observed with this series that there are cases
> for which we can not identify accurately the location of the target
> function arguments to prepare correctly the corresponding BPF
> trampoline. This is the case for example if:
> - the function consumes a struct variable _by value_
> - it is passed on the stack (no more register available for it)
> - it has some __packed__ or __aligned(X)__ attribute
> 
> [...]

Here is the summary with links:
  - [1/2] bpf, arm64: remove structs on stack constraint
    https://git.kernel.org/bpf/bpf-next/c/dc704d0cfa43
  - [2/2] selftests/bpf: enable tracing_struct tests for arm64
    https://git.kernel.org/bpf/bpf-next/c/4a760d2d7aa6

You are awesome, thank you!
-- 
Deet-doot-dot, I am a bot.
https://korg.docs.kernel.org/patchwork/pwbot.html



^ permalink raw reply	[flat|nested] 9+ messages in thread

end of thread, other threads:[~2025-07-17  1:29 UTC | newest]

Thread overview: 9+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
2025-07-09  8:36 [PATCH 0/2] bpf, arm64: relax constraint in BPF JIT compiler Alexis Lothoré (eBPF Foundation)
2025-07-09  8:36 ` [PATCH 1/2] bpf, arm64: remove structs on stack constraint Alexis Lothoré (eBPF Foundation)
2025-07-15 13:32   ` Will Deacon
2025-07-15 14:02     ` Alexis Lothoré
2025-07-15 14:31       ` Will Deacon
2025-07-15 15:40         ` Alexei Starovoitov
2025-07-16  7:37           ` Alexis Lothoré
2025-07-09  8:36 ` [PATCH 2/2] selftests/bpf: enable tracing_struct tests for arm64 Alexis Lothoré (eBPF Foundation)
2025-07-17  1:30 ` [PATCH 0/2] bpf, arm64: relax constraint in BPF JIT compiler patchwork-bot+netdevbpf

This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).