[PATCH bpf-next 0/3] bpf: Perform necessary sign/zero extension for kfunc return values

[PATCH bpf-next 0/3] bpf: Perform necessary sign/zero extension for kfunc return values

[Yonghong Song]

Tejun reported a bpf program kfunc return value mis-handling which
may cause incorrect result. If the kfunc return value is boolean
or u8, the bpf program produce incorrect results.

The main reason is due to mismatch of return value expectation between
native architecture and bpf. For example, for x86_64, if a kfunc
returns a u8, the kfunc returns 64-bit %rax, the top 56 bits might
be garbage. This is okay if the caller is x86_64 as the caller can
use special instruction to access lower 8-bit register %al. But this
will cause a problem for bpf program since bpf program assumes
the whole r0 register should contain correct value.
This patch set fixed the issue by doing necessary zero/sign extension
for the kfunc return value to meet bpf requirement.

For the rest of patches, Patch 1 is a preparation patch. Patch 2
implemented kernel support to perform necessary zero/sign extension
for kfunc return value. Patch 3 added two tests, one with return type
u8 and another with s16.

Yonghong Song (3):
  bpf: Always return corresponding btf_type in __get_type_size()
  bpf: Perform necessary sign/zero extension for kfunc return values
  selftests/bpf: Add tests with u8/s16 kfunc return types

 include/linux/bpf.h                           |  2 ++
 kernel/bpf/btf.c                              | 18 +++++++---
 kernel/bpf/verifier.c                         | 35 +++++++++++++++++--
 net/bpf/test_run.c                            | 12 +++++++
 .../selftests/bpf/prog_tests/kfunc_call.c     | 10 ++++++
 .../selftests/bpf/progs/kfunc_call_test.c     | 32 +++++++++++++++++
 6 files changed, 102 insertions(+), 7 deletions(-)

-- 
2.30.2

[PATCH bpf-next 1/3] bpf: Always return corresponding btf_type in __get_type_size()

[Yonghong Song]

Currently in funciton __get_type_size(), the corresponding
btf_type is returned only in invalid cases. Let us always
return btf_type regardless of valid or invalid cases.
Such a new functionality will be used in subsequent patches.

Signed-off-by: Yonghong Song <yhs@fb.com>
---
 kernel/bpf/btf.c | 9 ++++-----
 1 file changed, 4 insertions(+), 5 deletions(-)

diff --git a/kernel/bpf/btf.c b/kernel/bpf/btf.c
index 7e64447659f3..8119dc3994db 100644
--- a/kernel/bpf/btf.c
+++ b/kernel/bpf/btf.c
@@ -5864,26 +5864,25 @@ bool btf_struct_ids_match(struct bpf_verifier_log *log,
 }
 
 static int __get_type_size(struct btf *btf, u32 btf_id,
-			   const struct btf_type **bad_type)
+			   const struct btf_type **ret_type)
 {
 	const struct btf_type *t;
 
+	*ret_type = btf_type_by_id(btf, 0);
 	if (!btf_id)
 		/* void */
 		return 0;
 	t = btf_type_by_id(btf, btf_id);
 	while (t && btf_type_is_modifier(t))
 		t = btf_type_by_id(btf, t->type);
-	if (!t) {
-		*bad_type = btf_type_by_id(btf, 0);
+	if (!t)
 		return -EINVAL;
-	}
+	*ret_type = t;
 	if (btf_type_is_ptr(t))
 		/* kernel size of pointer. Not BPF's size of pointer*/
 		return sizeof(void *);
 	if (btf_type_is_int(t) || btf_is_any_enum(t))
 		return t->size;
-	*bad_type = t;
 	return -EINVAL;
 }
 
-- 
2.30.2

[PATCH bpf-next 2/3] bpf: Perform necessary sign/zero extension for kfunc return values

[Yonghong Song]

Tejun reported a bpf program kfunc return value mis-handling which
may cause incorrect result. The following is an example to show
the problem.
  $ cat t.c
  unsigned char bar();
  int foo() {
        if (bar() != 10) return 0; else return 1;
  }
  $ clang -target bpf -O2 -c t.c
  $ llvm-objdump -d t.o
  ...
  0000000000000000 <foo>:
       0:       85 10 00 00 ff ff ff ff call -1
       1:       bf 01 00 00 00 00 00 00 r1 = r0
       2:       b7 00 00 00 01 00 00 00 r0 = 1
       3:       15 01 01 00 0a 00 00 00 if r1 == 10 goto +1 <LBB0_2>
       4:       b7 00 00 00 00 00 00 00 r0 = 0

  0000000000000028 <LBB0_2>:
       5:       95 00 00 00 00 00 00 00 exit
  $

In the above example, the return type for bar() is 'unsigned char'.
But in the disassembly code, the whole register 'r1' is used to
compare to 10 without truncating upper 56 bits.

If function bar() is implemented as a bpf function, everything
should be okay since bpf ABI will make sure the caller do
proper truncation of upper 56 bits.

But if function bar() is implemented as a non-bpf kfunc,
there could a mismatch between bar() implementation and bpf program.
For example, if the host arch is x86_64, the bar() function
may just put the return value in lower 8-bit subregister and all
upper 56 bits could contain garbage. This is not a problem
if bar() is called in x86_64 context as the caller will use
%al to get the value.

But this could be a problem if bar() is called in bpf context
and there is a mismatch expectation between bpf and native architecture.
Currently, bpf programs use the default llvm ABI ([1], function
isPromotableIntegerTypeForABI()) such that if an integer type size
is less than int type size, it is assumed proper sign or zero
extension has been done to the return value. There will be a problem
if the kfunc return value type is u8/s8/u16/s16.

This patch intends to address this issue by doing proper sign or zero
extension for the kfunc return value before it is used later.

 [1] https://github.com/llvm/llvm-project/blob/main/clang/lib/CodeGen/TargetInfo.cpp

Reported-by: Tejun Heo <tj@kernel.org>
Signed-off-by: Yonghong Song <yhs@fb.com>
---
 include/linux/bpf.h   |  2 ++
 kernel/bpf/btf.c      |  9 +++++++++
 kernel/bpf/verifier.c | 35 +++++++++++++++++++++++++++++++++--
 3 files changed, 44 insertions(+), 2 deletions(-)

diff --git a/include/linux/bpf.h b/include/linux/bpf.h
index 20c26aed7896..b6f6bb1b707d 100644
--- a/include/linux/bpf.h
+++ b/include/linux/bpf.h
@@ -727,6 +727,8 @@ enum bpf_cgroup_storage_type {
 #define MAX_BPF_FUNC_REG_ARGS 5
 
 struct btf_func_model {
+	u8 ret_integer:1;
+	u8 ret_integer_signed:1;
 	u8 ret_size;
 	u8 nr_args;
 	u8 arg_size[MAX_BPF_FUNC_ARGS];
diff --git a/kernel/bpf/btf.c b/kernel/bpf/btf.c
index 8119dc3994db..f30a02018701 100644
--- a/kernel/bpf/btf.c
+++ b/kernel/bpf/btf.c
@@ -5897,6 +5897,7 @@ int btf_distill_func_proto(struct bpf_verifier_log *log,
 	u32 i, nargs;
 	int ret;
 
+	m->ret_integer = false;
 	if (!func) {
 		/* BTF function prototype doesn't match the verifier types.
 		 * Fall back to MAX_BPF_FUNC_REG_ARGS u64 args.
@@ -5923,6 +5924,14 @@ int btf_distill_func_proto(struct bpf_verifier_log *log,
 		return -EINVAL;
 	}
 	m->ret_size = ret;
+	if (btf_type_is_int(t)) {
+		m->ret_integer = true;
+		/* BTF_INT_BOOL is considered as unsigned */
+		if (BTF_INT_ENCODING(btf_type_int(t)) == BTF_INT_SIGNED)
+			m->ret_integer_signed = true;
+		else
+			m->ret_integer_signed = false;
+	}
 
 	for (i = 0; i < nargs; i++) {
 		if (i == nargs - 1 && args[i].type == 0) {
diff --git a/kernel/bpf/verifier.c b/kernel/bpf/verifier.c
index 096fdac70165..684f8606f341 100644
--- a/kernel/bpf/verifier.c
+++ b/kernel/bpf/verifier.c
@@ -13834,8 +13834,9 @@ static int fixup_call_args(struct bpf_verifier_env *env)
 }
 
 static int fixup_kfunc_call(struct bpf_verifier_env *env,
-			    struct bpf_insn *insn)
+			    struct bpf_insn *insn, struct bpf_insn *insn_buf, int *cnt)
 {
+	u8 ret_size, shift_cnt, rshift_opcode;
 	const struct bpf_kfunc_desc *desc;
 
 	if (!insn->imm) {
@@ -13855,6 +13856,26 @@ static int fixup_kfunc_call(struct bpf_verifier_env *env,
 
 	insn->imm = desc->imm;
 
+	*cnt = 0;
+	ret_size = desc->func_model.ret_size;
+
+	/* If the kfunc return type is an integer and the type size is one byte or two
+	 * bytes, currently llvm/bpf assumes proper sign/zero extension has been done
+	 * in the caller. But such an asumption may not hold for non-bpf architectures.
+	 * For example, for x86_64, if the return type is 'u8', it is possible that only
+	 * %al register is set properly and upper 56 bits of %rax register may contain
+	 * garbage. To resolve this case, Let us do a necessary truncation to zero-out
+	 * or properly sign-extend upper 56 bits.
+	 */
+	if (desc->func_model.ret_integer && ret_size < sizeof(int)) {
+		shift_cnt = (sizeof(u64) - ret_size) * 8;
+		rshift_opcode = desc->func_model.ret_integer_signed ? BPF_ARSH : BPF_RSH;
+		insn_buf[0] = *insn;
+		insn_buf[1] = BPF_ALU64_IMM(BPF_LSH, BPF_REG_0, shift_cnt);
+		insn_buf[2] = BPF_ALU64_IMM(rshift_opcode, BPF_REG_0, shift_cnt);
+		*cnt = 3;
+	}
+
 	return 0;
 }
 
@@ -13996,9 +14017,19 @@ static int do_misc_fixups(struct bpf_verifier_env *env)
 		if (insn->src_reg == BPF_PSEUDO_CALL)
 			continue;
 		if (insn->src_reg == BPF_PSEUDO_KFUNC_CALL) {
-			ret = fixup_kfunc_call(env, insn);
+			ret = fixup_kfunc_call(env, insn, insn_buf, &cnt);
 			if (ret)
 				return ret;
+			if (cnt == 0)
+				continue;
+
+			new_prog = bpf_patch_insn_data(env, i + delta, insn_buf, cnt);
+			if (!new_prog)
+				return -ENOMEM;
+
+			delta    += cnt - 1;
+			env->prog = prog = new_prog;
+			insn      = new_prog->insnsi + i + delta;
 			continue;
 		}
 
-- 
2.30.2

[PATCH bpf-next 3/3] selftests/bpf: Add tests with u8/s16 kfunc return types

[Yonghong Song]

Add two program tests with kfunc return types u8/s16.
With previous patch, xlated codes looks like below:
  ...
  ; return bpf_kfunc_call_test4((struct sock *)sk, (1 << 16) + 0xff00, (1 << 16) + 0xff);
     5: (bf) r1 = r0
     6: (b4) w2 = 130816
     7: (b4) w3 = 65791
     8: (85) call bpf_kfunc_call_test4#8931696
     9: (67) r0 <<= 48
    10: (c7) r0 s>>= 48
    11: (bc) w6 = w0
  ; }
    12: (bc) w0 = w6
    13: (95) exit
  ...
  ; return bpf_kfunc_call_test5((struct sock *)sk, (1 << 8) + 1, (1 << 8) + 2);
     5: (bf) r1 = r0
     6: (b4) w2 = 257
     7: (b4) w3 = 258
     8: (85) call bpf_kfunc_call_test5#8931712
     9: (67) r0 <<= 56
    10: (77) r0 >>= 56
    11: (bc) w6 = w0
  ; }
    12: (bc) w0 = w6
    13: (95) exit

For return type s16, proper sign extension for the return value is done
for kfunc bpf_kfunc_call_test4(). For return type s8, proper zero
extension for the return value is done for bpf_kfunc_call_test5().

Without the previous patch, the test kfunc_call will fail with
  ...
  test_main:FAIL:test4-retval unexpected test4-retval: actual 196607 != expected 4294967295
  ...
  test_main:FAIL:test5-retval unexpected test5-retval: actual 515 != expected 3

Signed-off-by: Yonghong Song <yhs@fb.com>
---
 net/bpf/test_run.c                            | 12 +++++++
 .../selftests/bpf/prog_tests/kfunc_call.c     | 10 ++++++
 .../selftests/bpf/progs/kfunc_call_test.c     | 32 +++++++++++++++++++
 3 files changed, 54 insertions(+)

diff --git a/net/bpf/test_run.c b/net/bpf/test_run.c
index cbc9cd5058cb..3a17ab4107f5 100644
--- a/net/bpf/test_run.c
+++ b/net/bpf/test_run.c
@@ -551,6 +551,16 @@ struct sock * noinline bpf_kfunc_call_test3(struct sock *sk)
 	return sk;
 }
 
+s16 noinline bpf_kfunc_call_test4(struct sock *sk, u32 a, u32 b)
+{
+	return a + b;
+}
+
+u8 noinline bpf_kfunc_call_test5(struct sock *sk, u32 a, u32 b)
+{
+	return a + b;
+}
+
 struct prog_test_member1 {
 	int a;
 };
@@ -703,6 +713,8 @@ BTF_SET8_START(test_sk_check_kfunc_ids)
 BTF_ID_FLAGS(func, bpf_kfunc_call_test1)
 BTF_ID_FLAGS(func, bpf_kfunc_call_test2)
 BTF_ID_FLAGS(func, bpf_kfunc_call_test3)
+BTF_ID_FLAGS(func, bpf_kfunc_call_test4)
+BTF_ID_FLAGS(func, bpf_kfunc_call_test5)
 BTF_ID_FLAGS(func, bpf_kfunc_call_test_acquire, KF_ACQUIRE | KF_RET_NULL)
 BTF_ID_FLAGS(func, bpf_kfunc_call_memb_acquire, KF_ACQUIRE | KF_RET_NULL)
 BTF_ID_FLAGS(func, bpf_kfunc_call_test_release, KF_RELEASE)
diff --git a/tools/testing/selftests/bpf/prog_tests/kfunc_call.c b/tools/testing/selftests/bpf/prog_tests/kfunc_call.c
index c00eb974eb85..a355c98080f2 100644
--- a/tools/testing/selftests/bpf/prog_tests/kfunc_call.c
+++ b/tools/testing/selftests/bpf/prog_tests/kfunc_call.c
@@ -35,6 +35,16 @@ static void test_main(void)
 	ASSERT_OK(err, "bpf_prog_test_run(test_ref_btf_id)");
 	ASSERT_EQ(topts.retval, 0, "test_ref_btf_id-retval");
 
+	prog_fd = skel->progs.kfunc_call_test4.prog_fd;
+	err = bpf_prog_test_run_opts(prog_fd, &topts);
+	ASSERT_OK(err, "bpf_prog_test_run(test4)");
+	ASSERT_EQ(topts.retval, 0xffffffff, "test4-retval");
+
+	prog_fd = skel->progs.kfunc_call_test5.prog_fd;
+	err = bpf_prog_test_run_opts(prog_fd, &topts);
+	ASSERT_OK(err, "bpf_prog_test_run(test5)");
+	ASSERT_EQ(topts.retval, 3, "test5-retval");
+
 	kfunc_call_test_lskel__destroy(skel);
 }
 
diff --git a/tools/testing/selftests/bpf/progs/kfunc_call_test.c b/tools/testing/selftests/bpf/progs/kfunc_call_test.c
index 5aecbb9fdc68..0636cb13e574 100644
--- a/tools/testing/selftests/bpf/progs/kfunc_call_test.c
+++ b/tools/testing/selftests/bpf/progs/kfunc_call_test.c
@@ -6,6 +6,8 @@
 extern int bpf_kfunc_call_test2(struct sock *sk, __u32 a, __u32 b) __ksym;
 extern __u64 bpf_kfunc_call_test1(struct sock *sk, __u32 a, __u64 b,
 				  __u32 c, __u64 d) __ksym;
+extern __s16 bpf_kfunc_call_test4(struct sock *sk, __u32 a, __u32 b) __ksym;
+extern __u8 bpf_kfunc_call_test5(struct sock *sk, __u32 a, __u32 b) __ksym;
 
 extern struct prog_test_ref_kfunc *bpf_kfunc_call_test_acquire(unsigned long *sp) __ksym;
 extern void bpf_kfunc_call_test_release(struct prog_test_ref_kfunc *p) __ksym;
@@ -30,6 +32,36 @@ int kfunc_call_test2(struct __sk_buff *skb)
 	return bpf_kfunc_call_test2((struct sock *)sk, 1, 2);
 }
 
+SEC("tc")
+int kfunc_call_test4(struct __sk_buff *skb)
+{
+	struct bpf_sock *sk = skb->sk;
+
+	if (!sk)
+		return -1;
+
+	sk = bpf_sk_fullsock(sk);
+	if (!sk)
+		return -1;
+
+	return bpf_kfunc_call_test4((struct sock *)sk, (1 << 16) + 0xff00, (1 << 16) + 0xff);
+}
+
+SEC("tc")
+int kfunc_call_test5(struct __sk_buff *skb)
+{
+	struct bpf_sock *sk = skb->sk;
+
+	if (!sk)
+		return -1;
+
+	sk = bpf_sk_fullsock(sk);
+	if (!sk)
+		return -1;
+
+	return bpf_kfunc_call_test5((struct sock *)sk, (1 << 8) + 1, (1 << 8) + 2);
+}
+
 SEC("tc")
 int kfunc_call_test1(struct __sk_buff *skb)
 {
-- 
2.30.2

Re: [PATCH bpf-next 0/3] bpf: Perform necessary sign/zero extension for kfunc return values

[Andrii Nakryiko]

On Sun, Aug 7, 2022 at 10:51 AM Yonghong Song <yhs@fb.com> wrote:
>
> Tejun reported a bpf program kfunc return value mis-handling which
> may cause incorrect result. If the kfunc return value is boolean
> or u8, the bpf program produce incorrect results.
>
> The main reason is due to mismatch of return value expectation between
> native architecture and bpf. For example, for x86_64, if a kfunc
> returns a u8, the kfunc returns 64-bit %rax, the top 56 bits might
> be garbage. This is okay if the caller is x86_64 as the caller can
> use special instruction to access lower 8-bit register %al. But this
> will cause a problem for bpf program since bpf program assumes
> the whole r0 register should contain correct value.
> This patch set fixed the issue by doing necessary zero/sign extension
> for the kfunc return value to meet bpf requirement.
>
> For the rest of patches, Patch 1 is a preparation patch. Patch 2
> implemented kernel support to perform necessary zero/sign extension
> for kfunc return value. Patch 3 added two tests, one with return type
> u8 and another with s16.
>
> Yonghong Song (3):
>   bpf: Always return corresponding btf_type in __get_type_size()
>   bpf: Perform necessary sign/zero extension for kfunc return values
>   selftests/bpf: Add tests with u8/s16 kfunc return types
>
>  include/linux/bpf.h                           |  2 ++
>  kernel/bpf/btf.c                              | 18 +++++++---
>  kernel/bpf/verifier.c                         | 35 +++++++++++++++++--
>  net/bpf/test_run.c                            | 12 +++++++
>  .../selftests/bpf/prog_tests/kfunc_call.c     | 10 ++++++
>  .../selftests/bpf/progs/kfunc_call_test.c     | 32 +++++++++++++++++
>  6 files changed, 102 insertions(+), 7 deletions(-)
>
> --
> 2.30.2
>

LGTM.

Acked-by: Andrii Nakryiko <andrii@kernel.org>

Re: [PATCH bpf-next 2/3] bpf: Perform necessary sign/zero extension for kfunc return values

[Andrii Nakryiko]

On Sun, Aug 7, 2022 at 10:51 AM Yonghong Song <yhs@fb.com> wrote:
>
> Tejun reported a bpf program kfunc return value mis-handling which
> may cause incorrect result. The following is an example to show
> the problem.
>   $ cat t.c
>   unsigned char bar();
>   int foo() {
>         if (bar() != 10) return 0; else return 1;
>   }
>   $ clang -target bpf -O2 -c t.c
>   $ llvm-objdump -d t.o
>   ...
>   0000000000000000 <foo>:
>        0:       85 10 00 00 ff ff ff ff call -1
>        1:       bf 01 00 00 00 00 00 00 r1 = r0
>        2:       b7 00 00 00 01 00 00 00 r0 = 1
>        3:       15 01 01 00 0a 00 00 00 if r1 == 10 goto +1 <LBB0_2>
>        4:       b7 00 00 00 00 00 00 00 r0 = 0
>
>   0000000000000028 <LBB0_2>:
>        5:       95 00 00 00 00 00 00 00 exit
>   $
>
> In the above example, the return type for bar() is 'unsigned char'.
> But in the disassembly code, the whole register 'r1' is used to
> compare to 10 without truncating upper 56 bits.
>
> If function bar() is implemented as a bpf function, everything
> should be okay since bpf ABI will make sure the caller do
> proper truncation of upper 56 bits.
>
> But if function bar() is implemented as a non-bpf kfunc,
> there could a mismatch between bar() implementation and bpf program.
> For example, if the host arch is x86_64, the bar() function
> may just put the return value in lower 8-bit subregister and all
> upper 56 bits could contain garbage. This is not a problem
> if bar() is called in x86_64 context as the caller will use
> %al to get the value.
>
> But this could be a problem if bar() is called in bpf context
> and there is a mismatch expectation between bpf and native architecture.
> Currently, bpf programs use the default llvm ABI ([1], function
> isPromotableIntegerTypeForABI()) such that if an integer type size
> is less than int type size, it is assumed proper sign or zero
> extension has been done to the return value. There will be a problem
> if the kfunc return value type is u8/s8/u16/s16.

Reading this I was still confused how (and whether) s32/u32 returns
are going to be handled correctly, especially on non-cpuv3 BPF object
code. So I played with this a bit and Clang does generate explicit <<
and >>/>>= shifts as expected. It might be worth it emphasizing that
for 32-bit returns Clang will generate explicit shifts?

>
> This patch intends to address this issue by doing proper sign or zero
> extension for the kfunc return value before it is used later.
>
>  [1] https://github.com/llvm/llvm-project/blob/main/clang/lib/CodeGen/TargetInfo.cpp
>
> Reported-by: Tejun Heo <tj@kernel.org>
> Signed-off-by: Yonghong Song <yhs@fb.com>
> ---
>  include/linux/bpf.h   |  2 ++
>  kernel/bpf/btf.c      |  9 +++++++++
>  kernel/bpf/verifier.c | 35 +++++++++++++++++++++++++++++++++--
>  3 files changed, 44 insertions(+), 2 deletions(-)
>

[...]

Re: [PATCH bpf-next 3/3] selftests/bpf: Add tests with u8/s16 kfunc return types

[Andrii Nakryiko]

On Sun, Aug 7, 2022 at 10:51 AM Yonghong Song <yhs@fb.com> wrote:
>
> Add two program tests with kfunc return types u8/s16.
> With previous patch, xlated codes looks like below:
>   ...
>   ; return bpf_kfunc_call_test4((struct sock *)sk, (1 << 16) + 0xff00, (1 << 16) + 0xff);
>      5: (bf) r1 = r0
>      6: (b4) w2 = 130816
>      7: (b4) w3 = 65791
>      8: (85) call bpf_kfunc_call_test4#8931696
>      9: (67) r0 <<= 48
>     10: (c7) r0 s>>= 48
>     11: (bc) w6 = w0
>   ; }
>     12: (bc) w0 = w6
>     13: (95) exit
>   ...
>   ; return bpf_kfunc_call_test5((struct sock *)sk, (1 << 8) + 1, (1 << 8) + 2);
>      5: (bf) r1 = r0
>      6: (b4) w2 = 257
>      7: (b4) w3 = 258
>      8: (85) call bpf_kfunc_call_test5#8931712
>      9: (67) r0 <<= 56
>     10: (77) r0 >>= 56
>     11: (bc) w6 = w0
>   ; }
>     12: (bc) w0 = w6
>     13: (95) exit
>
> For return type s16, proper sign extension for the return value is done
> for kfunc bpf_kfunc_call_test4(). For return type s8, proper zero
> extension for the return value is done for bpf_kfunc_call_test5().
>
> Without the previous patch, the test kfunc_call will fail with
>   ...
>   test_main:FAIL:test4-retval unexpected test4-retval: actual 196607 != expected 4294967295
>   ...
>   test_main:FAIL:test5-retval unexpected test5-retval: actual 515 != expected 3
>
> Signed-off-by: Yonghong Song <yhs@fb.com>
> ---
>  net/bpf/test_run.c                            | 12 +++++++
>  .../selftests/bpf/prog_tests/kfunc_call.c     | 10 ++++++
>  .../selftests/bpf/progs/kfunc_call_test.c     | 32 +++++++++++++++++++
>  3 files changed, 54 insertions(+)
>
> diff --git a/net/bpf/test_run.c b/net/bpf/test_run.c
> index cbc9cd5058cb..3a17ab4107f5 100644
> --- a/net/bpf/test_run.c
> +++ b/net/bpf/test_run.c
> @@ -551,6 +551,16 @@ struct sock * noinline bpf_kfunc_call_test3(struct sock *sk)
>         return sk;
>  }
>
> +s16 noinline bpf_kfunc_call_test4(struct sock *sk, u32 a, u32 b)
> +{
> +       return a + b;
> +}
> +
> +u8 noinline bpf_kfunc_call_test5(struct sock *sk, u32 a, u32 b)
> +{
> +       return a + b;
> +}

Is there any upside of adding this to net/bpf/test_run.c instead of
defining it in bpf_testmod?

> +
>  struct prog_test_member1 {
>         int a;
>  };

[...]

Re: [PATCH bpf-next 2/3] bpf: Perform necessary sign/zero extension for kfunc return values

[Yonghong Song]

On 8/8/22 4:25 PM, Andrii Nakryiko wrote:
> On Sun, Aug 7, 2022 at 10:51 AM Yonghong Song <yhs@fb.com> wrote:
>>
>> Tejun reported a bpf program kfunc return value mis-handling which
>> may cause incorrect result. The following is an example to show
>> the problem.
>>    $ cat t.c
>>    unsigned char bar();
>>    int foo() {
>>          if (bar() != 10) return 0; else return 1;
>>    }
>>    $ clang -target bpf -O2 -c t.c
>>    $ llvm-objdump -d t.o
>>    ...
>>    0000000000000000 <foo>:
>>         0:       85 10 00 00 ff ff ff ff call -1
>>         1:       bf 01 00 00 00 00 00 00 r1 = r0
>>         2:       b7 00 00 00 01 00 00 00 r0 = 1
>>         3:       15 01 01 00 0a 00 00 00 if r1 == 10 goto +1 <LBB0_2>
>>         4:       b7 00 00 00 00 00 00 00 r0 = 0
>>
>>    0000000000000028 <LBB0_2>:
>>         5:       95 00 00 00 00 00 00 00 exit
>>    $
>>
>> In the above example, the return type for bar() is 'unsigned char'.
>> But in the disassembly code, the whole register 'r1' is used to
>> compare to 10 without truncating upper 56 bits.
>>
>> If function bar() is implemented as a bpf function, everything
>> should be okay since bpf ABI will make sure the caller do
>> proper truncation of upper 56 bits.
>>
>> But if function bar() is implemented as a non-bpf kfunc,
>> there could a mismatch between bar() implementation and bpf program.
>> For example, if the host arch is x86_64, the bar() function
>> may just put the return value in lower 8-bit subregister and all
>> upper 56 bits could contain garbage. This is not a problem
>> if bar() is called in x86_64 context as the caller will use
>> %al to get the value.
>>
>> But this could be a problem if bar() is called in bpf context
>> and there is a mismatch expectation between bpf and native architecture.
>> Currently, bpf programs use the default llvm ABI ([1], function
>> isPromotableIntegerTypeForABI()) such that if an integer type size
>> is less than int type size, it is assumed proper sign or zero
>> extension has been done to the return value. There will be a problem
>> if the kfunc return value type is u8/s8/u16/s16.
> 
> Reading this I was still confused how (and whether) s32/u32 returns
> are going to be handled correctly, especially on non-cpuv3 BPF object
> code. So I played with this a bit and Clang does generate explicit <<
> and >>/>>= shifts as expected. It might be worth it emphasizing that
> for 32-bit returns Clang will generate explicit shifts?

Yes, I can reword the commit message to emphasize 32-bit return
value won't be an issue.

> 
>>
>> This patch intends to address this issue by doing proper sign or zero
>> extension for the kfunc return value before it is used later.
>>
>>   [1] https://github.com/llvm/llvm-project/blob/main/clang/lib/CodeGen/TargetInfo.cpp
>>
>> Reported-by: Tejun Heo <tj@kernel.org>
>> Signed-off-by: Yonghong Song <yhs@fb.com>
>> ---
>>   include/linux/bpf.h   |  2 ++
>>   kernel/bpf/btf.c      |  9 +++++++++
>>   kernel/bpf/verifier.c | 35 +++++++++++++++++++++++++++++++++--
>>   3 files changed, 44 insertions(+), 2 deletions(-)
>>
> 
> [...]

Re: [PATCH bpf-next 3/3] selftests/bpf: Add tests with u8/s16 kfunc return types

[Yonghong Song]

On 8/8/22 4:25 PM, Andrii Nakryiko wrote:
> On Sun, Aug 7, 2022 at 10:51 AM Yonghong Song <yhs@fb.com> wrote:
>>
>> Add two program tests with kfunc return types u8/s16.
>> With previous patch, xlated codes looks like below:
>>    ...
>>    ; return bpf_kfunc_call_test4((struct sock *)sk, (1 << 16) + 0xff00, (1 << 16) + 0xff);
>>       5: (bf) r1 = r0
>>       6: (b4) w2 = 130816
>>       7: (b4) w3 = 65791
>>       8: (85) call bpf_kfunc_call_test4#8931696
>>       9: (67) r0 <<= 48
>>      10: (c7) r0 s>>= 48
>>      11: (bc) w6 = w0
>>    ; }
>>      12: (bc) w0 = w6
>>      13: (95) exit
>>    ...
>>    ; return bpf_kfunc_call_test5((struct sock *)sk, (1 << 8) + 1, (1 << 8) + 2);
>>       5: (bf) r1 = r0
>>       6: (b4) w2 = 257
>>       7: (b4) w3 = 258
>>       8: (85) call bpf_kfunc_call_test5#8931712
>>       9: (67) r0 <<= 56
>>      10: (77) r0 >>= 56
>>      11: (bc) w6 = w0
>>    ; }
>>      12: (bc) w0 = w6
>>      13: (95) exit
>>
>> For return type s16, proper sign extension for the return value is done
>> for kfunc bpf_kfunc_call_test4(). For return type s8, proper zero
>> extension for the return value is done for bpf_kfunc_call_test5().
>>
>> Without the previous patch, the test kfunc_call will fail with
>>    ...
>>    test_main:FAIL:test4-retval unexpected test4-retval: actual 196607 != expected 4294967295
>>    ...
>>    test_main:FAIL:test5-retval unexpected test5-retval: actual 515 != expected 3
>>
>> Signed-off-by: Yonghong Song <yhs@fb.com>
>> ---
>>   net/bpf/test_run.c                            | 12 +++++++
>>   .../selftests/bpf/prog_tests/kfunc_call.c     | 10 ++++++
>>   .../selftests/bpf/progs/kfunc_call_test.c     | 32 +++++++++++++++++++
>>   3 files changed, 54 insertions(+)
>>
>> diff --git a/net/bpf/test_run.c b/net/bpf/test_run.c
>> index cbc9cd5058cb..3a17ab4107f5 100644
>> --- a/net/bpf/test_run.c
>> +++ b/net/bpf/test_run.c
>> @@ -551,6 +551,16 @@ struct sock * noinline bpf_kfunc_call_test3(struct sock *sk)
>>          return sk;
>>   }
>>
>> +s16 noinline bpf_kfunc_call_test4(struct sock *sk, u32 a, u32 b)
>> +{
>> +       return a + b;
>> +}
>> +
>> +u8 noinline bpf_kfunc_call_test5(struct sock *sk, u32 a, u32 b)
>> +{
>> +       return a + b;
>> +}
> 
> Is there any upside of adding this to net/bpf/test_run.c instead of
> defining it in bpf_testmod?

I put these two functions in test_run.c since bpf_kfunc_call_test{1,2,3}
are defined here and they easily fit the existing kfunc_call testing code.

But yes, I just checked the bpf_testmod.c. Looks like I am able
to define kfunc's in bpf_testmod. Will respin in v2 with this change.

> 
>> +
>>   struct prog_test_member1 {
>>          int a;
>>   };
> 
> [...]

Re: [PATCH bpf-next 2/3] bpf: Perform necessary sign/zero extension for kfunc return values

[Alexei Starovoitov]

On Sun, Aug 7, 2022 at 10:51 AM Yonghong Song <yhs@fb.com> wrote:
>
> Tejun reported a bpf program kfunc return value mis-handling which
> may cause incorrect result. The following is an example to show
> the problem.
>   $ cat t.c
>   unsigned char bar();
>   int foo() {
>         if (bar() != 10) return 0; else return 1;
>   }
>   $ clang -target bpf -O2 -c t.c
>   $ llvm-objdump -d t.o
>   ...
>   0000000000000000 <foo>:
>        0:       85 10 00 00 ff ff ff ff call -1
>        1:       bf 01 00 00 00 00 00 00 r1 = r0
>        2:       b7 00 00 00 01 00 00 00 r0 = 1
>        3:       15 01 01 00 0a 00 00 00 if r1 == 10 goto +1 <LBB0_2>
>        4:       b7 00 00 00 00 00 00 00 r0 = 0
>
>   0000000000000028 <LBB0_2>:
>        5:       95 00 00 00 00 00 00 00 exit
>   $
>
> In the above example, the return type for bar() is 'unsigned char'.
> But in the disassembly code, the whole register 'r1' is used to
> compare to 10 without truncating upper 56 bits.
>
> If function bar() is implemented as a bpf function, everything
> should be okay since bpf ABI will make sure the caller do
> proper truncation of upper 56 bits.
>
> But if function bar() is implemented as a non-bpf kfunc,
> there could a mismatch between bar() implementation and bpf program.
> For example, if the host arch is x86_64, the bar() function
> may just put the return value in lower 8-bit subregister and all
> upper 56 bits could contain garbage. This is not a problem
> if bar() is called in x86_64 context as the caller will use
> %al to get the value.
>
> But this could be a problem if bar() is called in bpf context
> and there is a mismatch expectation between bpf and native architecture.
> Currently, bpf programs use the default llvm ABI ([1], function
> isPromotableIntegerTypeForABI()) such that if an integer type size
> is less than int type size, it is assumed proper sign or zero
> extension has been done to the return value. There will be a problem
> if the kfunc return value type is u8/s8/u16/s16.
>
> This patch intends to address this issue by doing proper sign or zero
> extension for the kfunc return value before it is used later.
>
>  [1] https://github.com/llvm/llvm-project/blob/main/clang/lib/CodeGen/TargetInfo.cpp
>
> Reported-by: Tejun Heo <tj@kernel.org>
> Signed-off-by: Yonghong Song <yhs@fb.com>
> ---
>  include/linux/bpf.h   |  2 ++
>  kernel/bpf/btf.c      |  9 +++++++++
>  kernel/bpf/verifier.c | 35 +++++++++++++++++++++++++++++++++--
>  3 files changed, 44 insertions(+), 2 deletions(-)
>
> diff --git a/include/linux/bpf.h b/include/linux/bpf.h
> index 20c26aed7896..b6f6bb1b707d 100644
> --- a/include/linux/bpf.h
> +++ b/include/linux/bpf.h
> @@ -727,6 +727,8 @@ enum bpf_cgroup_storage_type {
>  #define MAX_BPF_FUNC_REG_ARGS 5
>
>  struct btf_func_model {
> +       u8 ret_integer:1;
> +       u8 ret_integer_signed:1;
>         u8 ret_size;
>         u8 nr_args;
>         u8 arg_size[MAX_BPF_FUNC_ARGS];
> diff --git a/kernel/bpf/btf.c b/kernel/bpf/btf.c
> index 8119dc3994db..f30a02018701 100644
> --- a/kernel/bpf/btf.c
> +++ b/kernel/bpf/btf.c
> @@ -5897,6 +5897,7 @@ int btf_distill_func_proto(struct bpf_verifier_log *log,
>         u32 i, nargs;
>         int ret;
>
> +       m->ret_integer = false;
>         if (!func) {
>                 /* BTF function prototype doesn't match the verifier types.
>                  * Fall back to MAX_BPF_FUNC_REG_ARGS u64 args.
> @@ -5923,6 +5924,14 @@ int btf_distill_func_proto(struct bpf_verifier_log *log,
>                 return -EINVAL;
>         }
>         m->ret_size = ret;
> +       if (btf_type_is_int(t)) {
> +               m->ret_integer = true;
> +               /* BTF_INT_BOOL is considered as unsigned */
> +               if (BTF_INT_ENCODING(btf_type_int(t)) == BTF_INT_SIGNED)
> +                       m->ret_integer_signed = true;
> +               else
> +                       m->ret_integer_signed = false;
> +       }
>
>         for (i = 0; i < nargs; i++) {
>                 if (i == nargs - 1 && args[i].type == 0) {
> diff --git a/kernel/bpf/verifier.c b/kernel/bpf/verifier.c
> index 096fdac70165..684f8606f341 100644
> --- a/kernel/bpf/verifier.c
> +++ b/kernel/bpf/verifier.c
> @@ -13834,8 +13834,9 @@ static int fixup_call_args(struct bpf_verifier_env *env)
>  }
>
>  static int fixup_kfunc_call(struct bpf_verifier_env *env,
> -                           struct bpf_insn *insn)
> +                           struct bpf_insn *insn, struct bpf_insn *insn_buf, int *cnt)
>  {
> +       u8 ret_size, shift_cnt, rshift_opcode;
>         const struct bpf_kfunc_desc *desc;
>
>         if (!insn->imm) {
> @@ -13855,6 +13856,26 @@ static int fixup_kfunc_call(struct bpf_verifier_env *env,
>
>         insn->imm = desc->imm;
>
> +       *cnt = 0;
> +       ret_size = desc->func_model.ret_size;
> +
> +       /* If the kfunc return type is an integer and the type size is one byte or two
> +        * bytes, currently llvm/bpf assumes proper sign/zero extension has been done
> +        * in the caller. But such an asumption may not hold for non-bpf architectures.
> +        * For example, for x86_64, if the return type is 'u8', it is possible that only
> +        * %al register is set properly and upper 56 bits of %rax register may contain
> +        * garbage. To resolve this case, Let us do a necessary truncation to zero-out
> +        * or properly sign-extend upper 56 bits.
> +        */
> +       if (desc->func_model.ret_integer && ret_size < sizeof(int)) {

Few questions...
Do we really need 'ret_integer' here?
and is it x86 specific?
afaik only x86 has 8 and 16-bit subregisters.
On all other archs the hw cannot write such quantities into
a register and don't touch the upper bits.
At the same time such return values from kfunc are rare.
I don't think we have such a case in the current set of kfuncs.
So being safe than sorry is a reasonable trade off and
gating by x86 only is unnecessary.
So how about just if (ret_size < sizeof(int)) here?

> +               shift_cnt = (sizeof(u64) - ret_size) * 8;
> +               rshift_opcode = desc->func_model.ret_integer_signed ? BPF_ARSH : BPF_RSH;
> +               insn_buf[0] = *insn;
> +               insn_buf[1] = BPF_ALU64_IMM(BPF_LSH, BPF_REG_0, shift_cnt);
> +               insn_buf[2] = BPF_ALU64_IMM(rshift_opcode, BPF_REG_0, shift_cnt);
> +               *cnt = 3;
> +       }
> +
>         return 0;
>  }
>
> @@ -13996,9 +14017,19 @@ static int do_misc_fixups(struct bpf_verifier_env *env)
>                 if (insn->src_reg == BPF_PSEUDO_CALL)
>                         continue;
>                 if (insn->src_reg == BPF_PSEUDO_KFUNC_CALL) {
> -                       ret = fixup_kfunc_call(env, insn);
> +                       ret = fixup_kfunc_call(env, insn, insn_buf, &cnt);
>                         if (ret)
>                                 return ret;
> +                       if (cnt == 0)
> +                               continue;
> +
> +                       new_prog = bpf_patch_insn_data(env, i + delta, insn_buf, cnt);
> +                       if (!new_prog)
> +                               return -ENOMEM;
> +
> +                       delta    += cnt - 1;
> +                       env->prog = prog = new_prog;
> +                       insn      = new_prog->insnsi + i + delta;
>                         continue;
>                 }
>
> --
> 2.30.2
>

Re: [PATCH bpf-next 2/3] bpf: Perform necessary sign/zero extension for kfunc return values

[Yonghong Song]

On 8/9/22 10:02 AM, Alexei Starovoitov wrote:
> On Sun, Aug 7, 2022 at 10:51 AM Yonghong Song <yhs@fb.com> wrote:
>>
>> Tejun reported a bpf program kfunc return value mis-handling which
>> may cause incorrect result. The following is an example to show
>> the problem.
>>    $ cat t.c
>>    unsigned char bar();
>>    int foo() {
>>          if (bar() != 10) return 0; else return 1;
>>    }
>>    $ clang -target bpf -O2 -c t.c
>>    $ llvm-objdump -d t.o
>>    ...
>>    0000000000000000 <foo>:
>>         0:       85 10 00 00 ff ff ff ff call -1
>>         1:       bf 01 00 00 00 00 00 00 r1 = r0
>>         2:       b7 00 00 00 01 00 00 00 r0 = 1
>>         3:       15 01 01 00 0a 00 00 00 if r1 == 10 goto +1 <LBB0_2>
>>         4:       b7 00 00 00 00 00 00 00 r0 = 0
>>
>>    0000000000000028 <LBB0_2>:
>>         5:       95 00 00 00 00 00 00 00 exit
>>    $
>>
>> In the above example, the return type for bar() is 'unsigned char'.
>> But in the disassembly code, the whole register 'r1' is used to
>> compare to 10 without truncating upper 56 bits.
>>
>> If function bar() is implemented as a bpf function, everything
>> should be okay since bpf ABI will make sure the caller do
>> proper truncation of upper 56 bits.
>>
>> But if function bar() is implemented as a non-bpf kfunc,
>> there could a mismatch between bar() implementation and bpf program.
>> For example, if the host arch is x86_64, the bar() function
>> may just put the return value in lower 8-bit subregister and all
>> upper 56 bits could contain garbage. This is not a problem
>> if bar() is called in x86_64 context as the caller will use
>> %al to get the value.
>>
>> But this could be a problem if bar() is called in bpf context
>> and there is a mismatch expectation between bpf and native architecture.
>> Currently, bpf programs use the default llvm ABI ([1], function
>> isPromotableIntegerTypeForABI()) such that if an integer type size
>> is less than int type size, it is assumed proper sign or zero
>> extension has been done to the return value. There will be a problem
>> if the kfunc return value type is u8/s8/u16/s16.
>>
>> This patch intends to address this issue by doing proper sign or zero
>> extension for the kfunc return value before it is used later.
>>
>>   [1] https://github.com/llvm/llvm-project/blob/main/clang/lib/CodeGen/TargetInfo.cpp
>>
>> Reported-by: Tejun Heo <tj@kernel.org>
>> Signed-off-by: Yonghong Song <yhs@fb.com>
>> ---
>>   include/linux/bpf.h   |  2 ++
>>   kernel/bpf/btf.c      |  9 +++++++++
>>   kernel/bpf/verifier.c | 35 +++++++++++++++++++++++++++++++++--
>>   3 files changed, 44 insertions(+), 2 deletions(-)
>>
>> diff --git a/include/linux/bpf.h b/include/linux/bpf.h
>> index 20c26aed7896..b6f6bb1b707d 100644
>> --- a/include/linux/bpf.h
>> +++ b/include/linux/bpf.h
>> @@ -727,6 +727,8 @@ enum bpf_cgroup_storage_type {
>>   #define MAX_BPF_FUNC_REG_ARGS 5
>>
>>   struct btf_func_model {
>> +       u8 ret_integer:1;
>> +       u8 ret_integer_signed:1;
>>          u8 ret_size;
>>          u8 nr_args;
>>          u8 arg_size[MAX_BPF_FUNC_ARGS];
>> diff --git a/kernel/bpf/btf.c b/kernel/bpf/btf.c
>> index 8119dc3994db..f30a02018701 100644
>> --- a/kernel/bpf/btf.c
>> +++ b/kernel/bpf/btf.c
>> @@ -5897,6 +5897,7 @@ int btf_distill_func_proto(struct bpf_verifier_log *log,
>>          u32 i, nargs;
>>          int ret;
>>
>> +       m->ret_integer = false;
>>          if (!func) {
>>                  /* BTF function prototype doesn't match the verifier types.
>>                   * Fall back to MAX_BPF_FUNC_REG_ARGS u64 args.
>> @@ -5923,6 +5924,14 @@ int btf_distill_func_proto(struct bpf_verifier_log *log,
>>                  return -EINVAL;
>>          }
>>          m->ret_size = ret;
>> +       if (btf_type_is_int(t)) {
>> +               m->ret_integer = true;
>> +               /* BTF_INT_BOOL is considered as unsigned */
>> +               if (BTF_INT_ENCODING(btf_type_int(t)) == BTF_INT_SIGNED)
>> +                       m->ret_integer_signed = true;
>> +               else
>> +                       m->ret_integer_signed = false;
>> +       }
>>
>>          for (i = 0; i < nargs; i++) {
>>                  if (i == nargs - 1 && args[i].type == 0) {
>> diff --git a/kernel/bpf/verifier.c b/kernel/bpf/verifier.c
>> index 096fdac70165..684f8606f341 100644
>> --- a/kernel/bpf/verifier.c
>> +++ b/kernel/bpf/verifier.c
>> @@ -13834,8 +13834,9 @@ static int fixup_call_args(struct bpf_verifier_env *env)
>>   }
>>
>>   static int fixup_kfunc_call(struct bpf_verifier_env *env,
>> -                           struct bpf_insn *insn)
>> +                           struct bpf_insn *insn, struct bpf_insn *insn_buf, int *cnt)
>>   {
>> +       u8 ret_size, shift_cnt, rshift_opcode;
>>          const struct bpf_kfunc_desc *desc;
>>
>>          if (!insn->imm) {
>> @@ -13855,6 +13856,26 @@ static int fixup_kfunc_call(struct bpf_verifier_env *env,
>>
>>          insn->imm = desc->imm;
>>
>> +       *cnt = 0;
>> +       ret_size = desc->func_model.ret_size;
>> +
>> +       /* If the kfunc return type is an integer and the type size is one byte or two
>> +        * bytes, currently llvm/bpf assumes proper sign/zero extension has been done
>> +        * in the caller. But such an asumption may not hold for non-bpf architectures.
>> +        * For example, for x86_64, if the return type is 'u8', it is possible that only
>> +        * %al register is set properly and upper 56 bits of %rax register may contain
>> +        * garbage. To resolve this case, Let us do a necessary truncation to zero-out
>> +        * or properly sign-extend upper 56 bits.
>> +        */
>> +       if (desc->func_model.ret_integer && ret_size < sizeof(int)) {
> 
> Few questions...
> Do we really need 'ret_integer' here?
> and is it x86 specific?
> afaik only x86 has 8 and 16-bit subregisters.
> On all other archs the hw cannot write such quantities into
> a register and don't touch the upper bits.
> At the same time such return values from kfunc are rare.
> I don't think we have such a case in the current set of kfuncs.
> So being safe than sorry is a reasonable trade off and
> gating by x86 only is unnecessary.
> So how about just if (ret_size < sizeof(int)) here?

good questions. yes, we don't need ret_integer here with the
current code base.

I added ret_integer because my kfunc struct argument/return value
support work (in RFC stage). In that case, we could have
a 1-byte or 2-bytes structure as return value in which case,
we should not do sign/extension. With checking ret_integer,
we can avoid the churn later.

But it is not clear whether we will support kfunc return struct
as we don't have a request so far. So I will remove ret_integer
in the next revision.

> 
>> +               shift_cnt = (sizeof(u64) - ret_size) * 8;
>> +               rshift_opcode = desc->func_model.ret_integer_signed ? BPF_ARSH : BPF_RSH;
>> +               insn_buf[0] = *insn;
>> +               insn_buf[1] = BPF_ALU64_IMM(BPF_LSH, BPF_REG_0, shift_cnt);
>> +               insn_buf[2] = BPF_ALU64_IMM(rshift_opcode, BPF_REG_0, shift_cnt);
>> +               *cnt = 3;
>> +       }
>> +
>>          return 0;
>>   }
>>
>> @@ -13996,9 +14017,19 @@ static int do_misc_fixups(struct bpf_verifier_env *env)
>>                  if (insn->src_reg == BPF_PSEUDO_CALL)
>>                          continue;
>>                  if (insn->src_reg == BPF_PSEUDO_KFUNC_CALL) {
>> -                       ret = fixup_kfunc_call(env, insn);
>> +                       ret = fixup_kfunc_call(env, insn, insn_buf, &cnt);
>>                          if (ret)
>>                                  return ret;
>> +                       if (cnt == 0)
>> +                               continue;
>> +
>> +                       new_prog = bpf_patch_insn_data(env, i + delta, insn_buf, cnt);
>> +                       if (!new_prog)
>> +                               return -ENOMEM;
>> +
>> +                       delta    += cnt - 1;
>> +                       env->prog = prog = new_prog;
>> +                       insn      = new_prog->insnsi + i + delta;
>>                          continue;
>>                  }
>>
>> --
>> 2.30.2
>>

Re: [PATCH bpf-next 0/3] bpf: Perform necessary sign/zero extension for kfunc return values

[patchwork-bot+netdevbpf]

Hello:

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

On Sun, 7 Aug 2022 10:51:11 -0700 you wrote:
> Tejun reported a bpf program kfunc return value mis-handling which
> may cause incorrect result. If the kfunc return value is boolean
> or u8, the bpf program produce incorrect results.
> 
> The main reason is due to mismatch of return value expectation between
> native architecture and bpf. For example, for x86_64, if a kfunc
> returns a u8, the kfunc returns 64-bit %rax, the top 56 bits might
> be garbage. This is okay if the caller is x86_64 as the caller can
> use special instruction to access lower 8-bit register %al. But this
> will cause a problem for bpf program since bpf program assumes
> the whole r0 register should contain correct value.
> This patch set fixed the issue by doing necessary zero/sign extension
> for the kfunc return value to meet bpf requirement.
> 
> [...]

Here is the summary with links:
  - [bpf-next,1/3] bpf: Always return corresponding btf_type in __get_type_size()
    https://git.kernel.org/bpf/bpf-next/c/a00ed8430199
  - [bpf-next,2/3] bpf: Perform necessary sign/zero extension for kfunc return values
    (no matching commit)
  - [bpf-next,3/3] selftests/bpf: Add tests with u8/s16 kfunc return types
    (no matching commit)

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