[PATCH] bpf: check mem for dynptr type

[PATCH] bpf: check mem for dynptr type

[Haojian Zhuang]

When user sends message to bpf prog by a user ring buffer, a callback
in bpf prog should load data from the user ring buffer.

By default, check_mem_access() doesn't handle the type of
CONST_PTR_TO_DYNPTR. So verifier reports an invalid memory access issue.

So add the case of CONST_PTR_TO_DYNPTR type. Make bpf prog to handle
content in the user ring buffer.

Signed-off-by: Haojian Zhuang <haojian.zhuang@linaro.org>
Cc: Alexei Starovoitov <ast@kernel.org>
Cc: Daniel Borkmann <daniel@iogearbox.net>
Cc: John Fastabend <john.fastabend@gmail.com>
Cc: bpf@vger.kernel.org
---
 kernel/bpf/verifier.c | 9 +++++++++
 1 file changed, 9 insertions(+)

diff --git a/kernel/bpf/verifier.c b/kernel/bpf/verifier.c
index 65f598694d55..84066e7246f9 100644
--- a/kernel/bpf/verifier.c
+++ b/kernel/bpf/verifier.c
@@ -6862,6 +6862,15 @@ static int check_mem_access(struct bpf_verifier_env *env, int insn_idx, u32 regn
 
 		if (!err && value_regno >= 0 && (rdonly_mem || t == BPF_READ))
 			mark_reg_unknown(env, regs, value_regno);
+	} else if (reg->type == CONST_PTR_TO_DYNPTR) {
+		if (t == BPF_WRITE) {
+			verbose(env, "R%d cannot write into %s\n",
+				regno, reg_type_str(env, reg->type));
+			return -EACCES;
+		}
+
+		if (value_regno >= 0)
+			mark_reg_unknown(env, regs, value_regno);
 	} else {
 		verbose(env, "R%d invalid mem access '%s'\n", regno,
 			reg_type_str(env, reg->type));
-- 
2.43.0

Re: [PATCH] bpf: check mem for dynptr type

[Eduard Zingerman]

On Sun, 2024-03-03 at 02:37 +0000, Haojian Zhuang wrote:
> When user sends message to bpf prog by a user ring buffer, a callback
> in bpf prog should load data from the user ring buffer.
> 
> By default, check_mem_access() doesn't handle the type of
> CONST_PTR_TO_DYNPTR. So verifier reports an invalid memory access issue.
> 
> So add the case of CONST_PTR_TO_DYNPTR type. Make bpf prog to handle
> content in the user ring buffer.
> 

You are referring to bpf_user_ringbuf_drain() helper function, right?
Could you please provide an example of program that fails to verify?
(ideally the patch set should extend
 tools/testing/selftests/bpf/progs/user_ringbuf_success.c
 to make sure that intended use case is tested).

Re: [PATCH] bpf: check mem for dynptr type

[Haojian Zhuang]

On Mon, Mar 04, 2024 at 02:46:06PM +0200, Eduard Zingerman wrote:
> On Sun, 2024-03-03 at 02:37 +0000, Haojian Zhuang wrote:
> > When user sends message to bpf prog by a user ring buffer, a callback
> > in bpf prog should load data from the user ring buffer.
> > 
> > By default, check_mem_access() doesn't handle the type of
> > CONST_PTR_TO_DYNPTR. So verifier reports an invalid memory access issue.
> > 
> > So add the case of CONST_PTR_TO_DYNPTR type. Make bpf prog to handle
> > content in the user ring buffer.
> > 
> 
> You are referring to bpf_user_ringbuf_drain() helper function, right?
> Could you please provide an example of program that fails to verify?
> (ideally the patch set should extend
>  tools/testing/selftests/bpf/progs/user_ringbuf_success.c
>  to make sure that intended use case is tested).
> 

Yes, I'm referring to bpf_user_ringbuf_drain() helper function.

Yes, I should extend bpf/progs/user_ringbuf_success.c. And it could be
loaded by bpf/prog_tests/user_ringbuf.c.

But I failed to find the binary of user_ringbuf.c after bpf test cases
built. And there're no binaries for the test cases in bpf/prog_tests
directory. How to make use of these test cases? I failed to find
documents on it. Could you help to share any tips to me? Thanks

Best Regards
Haojian

Re: [PATCH] bpf: check mem for dynptr type

[Eduard Zingerman]

On Mon, 2024-03-04 at 13:02 +0000, Haojian Zhuang wrote:
[...]

> Yes, I'm referring to bpf_user_ringbuf_drain() helper function.
> 
> Yes, I should extend bpf/progs/user_ringbuf_success.c. And it could be
> loaded by bpf/prog_tests/user_ringbuf.c.

Before doing so, could you please share example you have in mind?

> But I failed to find the binary of user_ringbuf.c after bpf test cases
> built. And there're no binaries for the test cases in bpf/prog_tests
> directory. How to make use of these test cases? I failed to find
> documents on it. Could you help to share any tips to me? Thanks

The binary that runs tests is called "test_progs" and is compiled
using target "test_progs" under "tools/testing/selftests/bpf" directory.
Specific tests could be run using commands like:
- "./test_progs -a user_ringbuf" (to run all subtests in the group);
- "./test_progs -a user_ringbuf/test_user_ringbuf_loop"
  (to run one subtest from the group).

I have an old writeup on setting up local testing environment,
if you need it I can update and share it.

Re: [PATCH] bpf: check mem for dynptr type

[Haojian Zhuang]

On Mon, Mar 04, 2024 at 03:13:29PM +0200, Eduard Zingerman wrote:
> On Mon, 2024-03-04 at 13:02 +0000, Haojian Zhuang wrote:
> [...]
> 
> > Yes, I'm referring to bpf_user_ringbuf_drain() helper function.
> > 
> > Yes, I should extend bpf/progs/user_ringbuf_success.c. And it could be
> > loaded by bpf/prog_tests/user_ringbuf.c.
> 
> Before doing so, could you please share example you have in mind?
> 

Sure. My test case is in the link (https://github.com/hzhuang1/linux/blob/bpf02/tools/testing/selftests/bpf/progs/test_crypto_kern.c).

> > But I failed to find the binary of user_ringbuf.c after bpf test cases
> > built. And there're no binaries for the test cases in bpf/prog_tests
> > directory. How to make use of these test cases? I failed to find
> > documents on it. Could you help to share any tips to me? Thanks
> 
> The binary that runs tests is called "test_progs" and is compiled
> using target "test_progs" under "tools/testing/selftests/bpf" directory.
> Specific tests could be run using commands like:
> - "./test_progs -a user_ringbuf" (to run all subtests in the group);
> - "./test_progs -a user_ringbuf/test_user_ringbuf_loop"
>   (to run one subtest from the group).
> 

Thanks. I'll follow this instruction.

> I have an old writeup on setting up local testing environment,
> if you need it I can update and share it.

It's better if you can share it.

Best Regards
Haojian

Re: [PATCH] bpf: check mem for dynptr type

[Eduard Zingerman]

On Mon, 2024-03-04 at 13:46 +0000, Haojian Zhuang wrote:
[...]

> Sure. My test case is in the link (https://github.com/hzhuang1/linux/blob/bpf02/tools/testing/selftests/bpf/progs/test_crypto_kern.c).

Strange, I've tried your branch w/o suggested verifier change and
program loaded w/o error. I'll wait for a smaller selftest.

> > I have an old writeup on setting up local testing environment,
> > if you need it I can update and share it.
> 
> It's better if you can share it.

Well, looking at your branch I don't think you really need it,
I assume that you already have elaborated setup, so this could only
serve to establish some kind of baseline for tests execution.
Anyways at [0] is a small recipe on how to setup local build
environment capable to build and run selftests in chroot assuming
certain directory structure.
All heavy-lifting is done by the script vmtest.sh already in the repo:
- it makes sure kernel is compiled with correct config;
- compiles selftests;
- downloads qemu rootfs for VM;
- starts VM and executes tests.

Some info about 'test_progs' binary:
- this binary is a driver for multiple test groups execution;
- each test group is split in two parts:
  - BPF programs used by group are defined under progs/
  - userspace part that drives the testing and sets up the
    BPF programs is defined in C files under prog_tests/
- Makefile for selftests scans all C files under prog_tests/
  looking for functions defined as "void test_<something>(void)"
  and assumes that these are entry points for test groups;
- Makefile organizes an array of pointers to functions,
  defined in test_progs.c, same file contains the main
  logic that initiates tests execution for each test case.

[0] https://gist.github.com/eddyz87/204a47145452f1fc4792c8a55be53d6d