Re: [PATCH bpf v2 1/5] bpf: Adapt 32-bit return value kfunc for 32-bit ARM when zext extension

From: Yang Jihong
Date: Sat Nov 26 2022 - 04:46:08 EST


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@xxxxxxxxxx>
---
  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