| Message ID | 20221107092032.178235-2-yangjihong1@huawei.com (mailing list archive) |
|---|---|
| State | New |
| Headers | show |
| Series | bpf: Support kernel function call in 32-bit ARM | expand |
On 11/7/22 1:20 AM, Yang Jihong wrote: > For ARM32 architecture, if data width of kfunc return value is 32 bits, > need to do explicit zero extension for high 32-bit, insn_def_regno should > return dst_reg for BPF_JMP type of BPF_PSEUDO_KFUNC_CALL. Otherwise, > opt_subreg_zext_lo32_rnd_hi32 returns -EFAULT, resulting in BPF failure. > > Signed-off-by: Yang Jihong <yangjihong1@huawei.com> > --- > kernel/bpf/verifier.c | 3 +++ > 1 file changed, 3 insertions(+) > > diff --git a/kernel/bpf/verifier.c b/kernel/bpf/verifier.c > index 7f0a9f6cb889..bac37757ffca 100644 > --- a/kernel/bpf/verifier.c > +++ b/kernel/bpf/verifier.c > @@ -2404,6 +2404,9 @@ static int insn_def_regno(const struct bpf_insn *insn) > { > switch (BPF_CLASS(insn->code)) { > case BPF_JMP: > + if (insn->src_reg == BPF_PSEUDO_KFUNC_CALL) > + return insn->dst_reg; This does not look right. A kfunc can return void. The btf type of the kfunc's return value needs to be checked against "void" first? Also, this will affect insn_has_def32(), does is_reg64 (called from insn_has_def32) need to be adjusted also? For patch 2, as replied earlier in v1, I would separate out the prog that does __sk_buff->sk and use the uapi's bpf.h instead of vmlinux.h since it does not need CO-RE. This set should target for bpf-next instead of bpf. > + fallthrough; > case BPF_JMP32: > case BPF_ST: > return -1;
Hello, On 2022/11/9 7:12, Martin KaFai Lau wrote: > On 11/7/22 1:20 AM, Yang Jihong wrote: >> For ARM32 architecture, if data width of kfunc return value is 32 bits, >> need to do explicit zero extension for high 32-bit, insn_def_regno should >> return dst_reg for BPF_JMP type of BPF_PSEUDO_KFUNC_CALL. Otherwise, >> opt_subreg_zext_lo32_rnd_hi32 returns -EFAULT, resulting in BPF failure. >> >> Signed-off-by: Yang Jihong <yangjihong1@huawei.com> >> --- >> kernel/bpf/verifier.c | 3 +++ >> 1 file changed, 3 insertions(+) >> >> diff --git a/kernel/bpf/verifier.c b/kernel/bpf/verifier.c >> index 7f0a9f6cb889..bac37757ffca 100644 >> --- a/kernel/bpf/verifier.c >> +++ b/kernel/bpf/verifier.c >> @@ -2404,6 +2404,9 @@ static int insn_def_regno(const struct bpf_insn >> *insn) >> { >> switch (BPF_CLASS(insn->code)) { >> case BPF_JMP: >> + if (insn->src_reg == BPF_PSEUDO_KFUNC_CALL) >> + return insn->dst_reg; > > This does not look right. A kfunc can return void. The btf type of the > kfunc's return value needs to be checked against "void" first? OK, will add the check in next version. > Also, this will affect insn_has_def32(), does is_reg64 (called from > insn_has_def32) need to be adjusted also? Yes, is_reg64 need to be adjusted, will fix in next version. > > > For patch 2, as replied earlier in v1, I would separate out the prog > that does __sk_buff->sk and use the uapi's bpf.h instead of vmlinux.h > since it does not need CO-RE. OK, will remove adjust sk check patches in next verion. As mentioned in v1: "bpf-tc program can take'struct sk_buff *skb' instead of'struct __sk_buff *skb' but it will be a separate topic." It is a separate topic, only the lskel test cases are affected. The ARM32 kfunc function is not affected. > > This set should target for bpf-next instead of bpf. OK, will send to bpf-next in next version. Thanks, Yang
diff --git a/kernel/bpf/verifier.c b/kernel/bpf/verifier.c index 7f0a9f6cb889..bac37757ffca 100644 --- a/kernel/bpf/verifier.c +++ b/kernel/bpf/verifier.c @@ -2404,6 +2404,9 @@ static int insn_def_regno(const struct bpf_insn *insn) { switch (BPF_CLASS(insn->code)) { case BPF_JMP: + if (insn->src_reg == BPF_PSEUDO_KFUNC_CALL) + return insn->dst_reg; + fallthrough; case BPF_JMP32: case BPF_ST: return -1;
For ARM32 architecture, if data width of kfunc return value is 32 bits, need to do explicit zero extension for high 32-bit, insn_def_regno should return dst_reg for BPF_JMP type of BPF_PSEUDO_KFUNC_CALL. Otherwise, opt_subreg_zext_lo32_rnd_hi32 returns -EFAULT, resulting in BPF failure. Signed-off-by: Yang Jihong <yangjihong1@huawei.com> --- kernel/bpf/verifier.c | 3 +++ 1 file changed, 3 insertions(+)