On 9/9/2024 6:38 PM, Leon Hwang wrote:
On 9/9/24 17:02, Xu Kuohai wrote:
On 9/8/2024 9:01 PM, Leon Hwang wrote:
On 1/9/24 21:38, Leon Hwang wrote:
Like "bpf, x64: Fix tailcall infinite loop caused by freplace", the same
issue happens on arm64, too.
For example:
tc_bpf2bpf.c:
// SPDX-License-Identifier: GPL-2.0
\#include <linux/bpf.h>
\#include <bpf/bpf_helpers.h>
__noinline
int subprog_tc(struct __sk_buff *skb)
{
return skb->len * 2;
}
SEC("tc")
int entry_tc(struct __sk_buff *skb)
{
return subprog(skb);
}
char __license[] SEC("license") = "GPL";
tailcall_bpf2bpf_hierarchy_freplace.c:
// SPDX-License-Identifier: GPL-2.0
\#include <linux/bpf.h>
\#include <bpf/bpf_helpers.h>
struct {
__uint(type, BPF_MAP_TYPE_PROG_ARRAY);
__uint(max_entries, 1);
__uint(key_size, sizeof(__u32));
__uint(value_size, sizeof(__u32));
} jmp_table SEC(".maps");
int count = 0;
static __noinline
int subprog_tail(struct __sk_buff *skb)
{
bpf_tail_call_static(skb, &jmp_table, 0);
return 0;
}
SEC("freplace")
int entry_freplace(struct __sk_buff *skb)
{
count++;
subprog_tail(skb);
subprog_tail(skb);
return count;
}
char __license[] SEC("license") = "GPL";
The attach target of entry_freplace is subprog_tc, and the tail callee
in subprog_tail is entry_tc.
Then, the infinite loop will be entry_tc -> entry_tc ->
entry_freplace ->
subprog_tail --tailcall-> entry_tc, because tail_call_cnt in
entry_freplace will count from zero for every time of entry_freplace
execution.
This patch fixes the issue by avoiding touching tail_call_cnt at
prologue when it's subprog or freplace prog.
Then, when freplace prog attaches to entry_tc, it has to initialize
tail_call_cnt and tail_call_cnt_ptr, because its target is main prog and
its target's prologue hasn't initialize them before the attach hook.
So, this patch uses x7 register to tell freplace prog that its target
prog is main prog or not.
Meanwhile, while tail calling to a freplace prog, it is required to
reset x7 register to prevent re-initializing tail_call_cnt at freplace
prog's prologue.
Fixes: 1c123c567fb1 ("bpf: Resolve fext program type when checking
map compatibility")
Signed-off-by: Leon Hwang <leon.hwang@xxxxxxxxx>
---
arch/arm64/net/bpf_jit_comp.c | 44 +++++++++++++++++++++++++++++++----
1 file changed, 39 insertions(+), 5 deletions(-)
Hi Puranjay and Kuohai,
As it's not recommended to introduce arch_bpf_run(), this is my approach
to fix the niche case on arm64.
Do you have any better idea to fix it?
IIUC, the recommended appraoch is to teach verifier to reject the
freplace + tailcall combination. If this combiation is allowed, we
will face more than just this issue. For example, what happens if
a freplace prog is attached to tail callee? The freplace prog is not
reachable through the tail call, right?
It's to reject the freplace + tailcall combination partially, see "bpf,
x64: Fix tailcall infinite loop caused by freplace". (Oh, I should
separate the rejection to a standalone patch.)
It rejects the case that freplace prog has tailcall and its attach
target has no tailcall.
As for your example, it depends on:
freplace target reject?
Has tailcall? YES NO YES
Has tailcall? YES YES NO
Has tailcall? NO YES NO
Has tailcall? NO YES NO
Then, freplace prog can be tail callee always. I haven't seen any bad
case when freplace prog is tail callee.
Here is a concrete case. prog1 tail calls prog2, and prog2_new is
attached to prog2 via freplace.
SEC("tc")
int prog1(struct __sk_buff *skb)
{
bpf_tail_call_static(skb, &progs, 0); // tail call prog2
return 0;
}
SEC("tc")
int prog2(struct __sk_buff *skb)
{
return 0;
}
SEC("freplace")
int prog2_new(struct __sk_buff *skb) // target is prog2
{
return 0;
}
In this case, prog2_new is not reachable, since the tail call
target in prog2 is start address of prog2 + TAIL_CALL_OFFSET,
which locates behind freplace/fentry callsite of prog2.
I'm not planning to disable freplace + tailcall combination totally,
because I use this combination in an in-house XDP project of my company.
Thanks,
Leon
