diff mbox series

[bpf-next,v1,02/13] selftests/bpf: Test referenced kptr arguments of struct_ops programs

Message ID 20241213232958.2388301-3-amery.hung@bytedance.com (mailing list archive)
State Changes Requested
Delegated to: BPF
Headers show
Series bpf qdisc | expand

Checks

Context Check Description
netdev/tree_selection success Clearly marked for bpf-next, async
netdev/apply fail Patch does not apply to bpf-next-0
bpf/vmtest-bpf-net-VM_Test-0 success Logs for Lint
bpf/vmtest-bpf-net-VM_Test-1 success Logs for ShellCheck
bpf/vmtest-bpf-net-VM_Test-3 success Logs for Validate matrix.py
bpf/vmtest-bpf-net-VM_Test-2 success Logs for Unittests
bpf/vmtest-bpf-net-VM_Test-5 success Logs for aarch64-gcc / build-release
bpf/vmtest-bpf-net-VM_Test-9 success Logs for aarch64-gcc / test (test_verifier, false, 360) / test_verifier on aarch64 with gcc
bpf/vmtest-bpf-net-VM_Test-10 success Logs for aarch64-gcc / veristat-kernel
bpf/vmtest-bpf-net-VM_Test-11 success Logs for aarch64-gcc / veristat-meta
bpf/vmtest-bpf-net-VM_Test-4 success Logs for aarch64-gcc / build / build for aarch64 with gcc
bpf/vmtest-bpf-net-VM_Test-13 success Logs for s390x-gcc / build-release
bpf/vmtest-bpf-net-VM_Test-6 success Logs for aarch64-gcc / test (test_maps, false, 360) / test_maps on aarch64 with gcc
bpf/vmtest-bpf-net-VM_Test-12 success Logs for s390x-gcc / build / build for s390x with gcc
bpf/vmtest-bpf-net-VM_Test-16 success Logs for s390x-gcc / test (test_verifier, false, 360) / test_verifier on s390x with gcc
bpf/vmtest-bpf-net-VM_Test-17 success Logs for s390x-gcc / veristat-kernel
bpf/vmtest-bpf-net-VM_Test-18 success Logs for s390x-gcc / veristat-meta
bpf/vmtest-bpf-net-VM_Test-19 success Logs for set-matrix
bpf/vmtest-bpf-net-VM_Test-20 success Logs for x86_64-gcc / build / build for x86_64 with gcc
bpf/vmtest-bpf-net-VM_Test-21 success Logs for x86_64-gcc / build-release
bpf/vmtest-bpf-net-VM_Test-22 success Logs for x86_64-gcc / test (test_maps, false, 360) / test_maps on x86_64 with gcc
bpf/vmtest-bpf-net-VM_Test-25 success Logs for x86_64-gcc / test (test_progs_no_alu32_parallel, true, 30) / test_progs_no_alu32_parallel on x86_64 with gcc
bpf/vmtest-bpf-net-VM_Test-26 success Logs for x86_64-gcc / test (test_progs_parallel, true, 30) / test_progs_parallel on x86_64 with gcc
bpf/vmtest-bpf-net-VM_Test-27 success Logs for x86_64-gcc / test (test_verifier, false, 360) / test_verifier on x86_64 with gcc
bpf/vmtest-bpf-net-VM_Test-28 success Logs for x86_64-gcc / veristat-kernel / x86_64-gcc veristat_kernel
bpf/vmtest-bpf-net-VM_Test-29 fail Logs for x86_64-gcc / veristat-meta / x86_64-gcc veristat_meta
bpf/vmtest-bpf-net-VM_Test-30 success Logs for x86_64-llvm-17 / build / build for x86_64 with llvm-17
bpf/vmtest-bpf-net-VM_Test-31 success Logs for x86_64-llvm-17 / build-release / build for x86_64 with llvm-17-O2
bpf/vmtest-bpf-net-VM_Test-32 success Logs for x86_64-llvm-17 / test (test_maps, false, 360) / test_maps on x86_64 with llvm-17
bpf/vmtest-bpf-net-VM_Test-33 success Logs for x86_64-llvm-17 / test (test_progs, false, 360) / test_progs on x86_64 with llvm-17
bpf/vmtest-bpf-net-VM_Test-34 success Logs for x86_64-llvm-17 / test (test_progs_no_alu32, false, 360) / test_progs_no_alu32 on x86_64 with llvm-17
bpf/vmtest-bpf-net-VM_Test-35 success Logs for x86_64-llvm-17 / test (test_verifier, false, 360) / test_verifier on x86_64 with llvm-17
bpf/vmtest-bpf-net-VM_Test-36 success Logs for x86_64-llvm-17 / veristat-kernel
bpf/vmtest-bpf-net-VM_Test-37 success Logs for x86_64-llvm-17 / veristat-meta
bpf/vmtest-bpf-net-VM_Test-38 success Logs for x86_64-llvm-18 / build / build for x86_64 with llvm-18
bpf/vmtest-bpf-net-VM_Test-39 success Logs for x86_64-llvm-18 / build-release / build for x86_64 with llvm-18-O2
bpf/vmtest-bpf-net-VM_Test-40 success Logs for x86_64-llvm-18 / test (test_maps, false, 360) / test_maps on x86_64 with llvm-18
bpf/vmtest-bpf-net-VM_Test-44 success Logs for x86_64-llvm-18 / test (test_verifier, false, 360) / test_verifier on x86_64 with llvm-18
bpf/vmtest-bpf-net-VM_Test-45 success Logs for x86_64-llvm-18 / veristat-kernel
bpf/vmtest-bpf-net-VM_Test-46 success Logs for x86_64-llvm-18 / veristat-meta
bpf/vmtest-bpf-net-VM_Test-7 success Logs for aarch64-gcc / test (test_progs, false, 360) / test_progs on aarch64 with gcc
bpf/vmtest-bpf-net-VM_Test-8 success Logs for aarch64-gcc / test (test_progs_no_alu32, false, 360) / test_progs_no_alu32 on aarch64 with gcc
bpf/vmtest-bpf-net-VM_Test-15 success Logs for s390x-gcc / test (test_progs_no_alu32, false, 360) / test_progs_no_alu32 on s390x with gcc
bpf/vmtest-bpf-net-VM_Test-23 success Logs for x86_64-gcc / test (test_progs, false, 360) / test_progs on x86_64 with gcc
bpf/vmtest-bpf-net-VM_Test-24 success Logs for x86_64-gcc / test (test_progs_no_alu32, false, 360) / test_progs_no_alu32 on x86_64 with gcc
bpf/vmtest-bpf-net-VM_Test-41 success Logs for x86_64-llvm-18 / test (test_progs, false, 360) / test_progs on x86_64 with llvm-18
bpf/vmtest-bpf-net-VM_Test-42 success Logs for x86_64-llvm-18 / test (test_progs_cpuv4, false, 360) / test_progs_cpuv4 on x86_64 with llvm-18
bpf/vmtest-bpf-net-VM_Test-43 success Logs for x86_64-llvm-18 / test (test_progs_no_alu32, false, 360) / test_progs_no_alu32 on x86_64 with llvm-18
bpf/vmtest-bpf-net-PR fail PR summary
bpf/vmtest-bpf-net-VM_Test-14 success Logs for s390x-gcc / test (test_progs, false, 360) / test_progs on s390x with gcc

Commit Message

Amery Hung Dec. 13, 2024, 11:29 p.m. UTC 
Test referenced kptr acquired through struct_ops argument tagged with
"__ref". The success case checks whether 1) a reference to the correct
type is acquired, and 2) the referenced kptr argument can be accessed in
multiple paths as long as it hasn't been released. In the fail cases,
we first confirm that a referenced kptr acquried through a struct_ops
argument is not allowed to be leaked. Then, we make sure this new
referenced kptr acquiring mechanism does not accidentally allow referenced
kptrs to flow into global subprograms through their arguments.

Signed-off-by: Amery Hung <amery.hung@bytedance.com>
---
 .../selftests/bpf/bpf_testmod/bpf_testmod.c   |  7 ++
 .../selftests/bpf/bpf_testmod/bpf_testmod.h   |  2 +
 .../prog_tests/test_struct_ops_refcounted.c   | 58 ++++++++++++++++
 .../bpf/progs/struct_ops_refcounted.c         | 67 +++++++++++++++++++
 ...ruct_ops_refcounted_fail__global_subprog.c | 32 +++++++++
 .../struct_ops_refcounted_fail__ref_leak.c    | 17 +++++
 6 files changed, 183 insertions(+)
 create mode 100644 tools/testing/selftests/bpf/prog_tests/test_struct_ops_refcounted.c
 create mode 100644 tools/testing/selftests/bpf/progs/struct_ops_refcounted.c
 create mode 100644 tools/testing/selftests/bpf/progs/struct_ops_refcounted_fail__global_subprog.c
 create mode 100644 tools/testing/selftests/bpf/progs/struct_ops_refcounted_fail__ref_leak.c

Comments

Martin KaFai Lau Dec. 18, 2024, 1:17 a.m. UTC | #1 
On 12/13/24 3:29 PM, Amery Hung wrote:

> +void test_struct_ops_refcounted(void)
> +{
> +	if (test__start_subtest("refcounted"))
> +		refcounted();
> +	if (test__start_subtest("refcounted_fail__ref_leak"))
> +		refcounted_fail__ref_leak();

test_loader.c could make writing this test easier and it can also test the 
verifier failure message. e.g. for the ref_leak test, the following should do:

	RUN_TESTS(struct_ops_refcounted_fail__ref_leak);

The same for the other subtests in this patch.
	
> +	if (test__start_subtest("refcounted_fail__global_subprog"))
> +		refcounted_fail__global_subprog();
> +}

[ ... ]

> diff --git a/tools/testing/selftests/bpf/progs/struct_ops_refcounted_fail__ref_leak.c b/tools/testing/selftests/bpf/progs/struct_ops_refcounted_fail__ref_leak.c
> new file mode 100644
> index 000000000000..6e82859eb187
> --- /dev/null
> +++ b/tools/testing/selftests/bpf/progs/struct_ops_refcounted_fail__ref_leak.c
> @@ -0,0 +1,17 @@
> +#include <vmlinux.h>
> +#include <bpf/bpf_tracing.h>
> +#include "../bpf_testmod/bpf_testmod.h"
> +
> +char _license[] SEC("license") = "GPL";
> +

+#include "bpf_misc.h"

+__failure __msg("Unreleased reference")
> +SEC("struct_ops/test_refcounted")
> +int BPF_PROG(test_refcounted, int dummy,
> +	     struct task_struct *task)
> +{
> +	return 0;
> +}
> +
> +SEC(".struct_ops.link")
> +struct bpf_testmod_ops testmod_ref_acquire = {
> +	.test_refcounted = (void *)test_refcounted,
> +};

[ I will stop here for today and will continue the rest tomorrow. ]
Amery Hung Dec. 18, 2024, 4:10 p.m. UTC | #2 
On Tue, Dec 17, 2024 at 5:17 PM Martin KaFai Lau <martin.lau@linux.dev> wrote:
>
> On 12/13/24 3:29 PM, Amery Hung wrote:
>
> > +void test_struct_ops_refcounted(void)
> > +{
> > +     if (test__start_subtest("refcounted"))
> > +             refcounted();
> > +     if (test__start_subtest("refcounted_fail__ref_leak"))
> > +             refcounted_fail__ref_leak();
>
> test_loader.c could make writing this test easier and it can also test the
> verifier failure message. e.g. for the ref_leak test, the following should do:
>
>         RUN_TESTS(struct_ops_refcounted_fail__ref_leak);
>
> The same for the other subtests in this patch.
>

Thanks for the pointer. I will change selftests in this set to use test_loader.

> > +     if (test__start_subtest("refcounted_fail__global_subprog"))
> > +             refcounted_fail__global_subprog();
> > +}
>
> [ ... ]
>
> > diff --git a/tools/testing/selftests/bpf/progs/struct_ops_refcounted_fail__ref_leak.c b/tools/testing/selftests/bpf/progs/struct_ops_refcounted_fail__ref_leak.c
> > new file mode 100644
> > index 000000000000..6e82859eb187
> > --- /dev/null
> > +++ b/tools/testing/selftests/bpf/progs/struct_ops_refcounted_fail__ref_leak.c
> > @@ -0,0 +1,17 @@
> > +#include <vmlinux.h>
> > +#include <bpf/bpf_tracing.h>
> > +#include "../bpf_testmod/bpf_testmod.h"
> > +
> > +char _license[] SEC("license") = "GPL";
> > +
>
> +#include "bpf_misc.h"
>
> +__failure __msg("Unreleased reference")
> > +SEC("struct_ops/test_refcounted")
> > +int BPF_PROG(test_refcounted, int dummy,
> > +          struct task_struct *task)
> > +{
> > +     return 0;
> > +}
> > +
> > +SEC(".struct_ops.link")
> > +struct bpf_testmod_ops testmod_ref_acquire = {
> > +     .test_refcounted = (void *)test_refcounted,
> > +};
>
> [ I will stop here for today and will continue the rest tomorrow. ]
Yonghong Song Dec. 19, 2024, 3:40 a.m. UTC | #3 
On 12/13/24 3:29 PM, Amery Hung wrote:
> Test referenced kptr acquired through struct_ops argument tagged with
> "__ref". The success case checks whether 1) a reference to the correct
> type is acquired, and 2) the referenced kptr argument can be accessed in
> multiple paths as long as it hasn't been released. In the fail cases,
> we first confirm that a referenced kptr acquried through a struct_ops
> argument is not allowed to be leaked. Then, we make sure this new
> referenced kptr acquiring mechanism does not accidentally allow referenced
> kptrs to flow into global subprograms through their arguments.
>
> Signed-off-by: Amery Hung <amery.hung@bytedance.com>
> ---
>   .../selftests/bpf/bpf_testmod/bpf_testmod.c   |  7 ++
>   .../selftests/bpf/bpf_testmod/bpf_testmod.h   |  2 +
>   .../prog_tests/test_struct_ops_refcounted.c   | 58 ++++++++++++++++
>   .../bpf/progs/struct_ops_refcounted.c         | 67 +++++++++++++++++++
>   ...ruct_ops_refcounted_fail__global_subprog.c | 32 +++++++++
>   .../struct_ops_refcounted_fail__ref_leak.c    | 17 +++++
>   6 files changed, 183 insertions(+)
>   create mode 100644 tools/testing/selftests/bpf/prog_tests/test_struct_ops_refcounted.c
>   create mode 100644 tools/testing/selftests/bpf/progs/struct_ops_refcounted.c
>   create mode 100644 tools/testing/selftests/bpf/progs/struct_ops_refcounted_fail__global_subprog.c
>   create mode 100644 tools/testing/selftests/bpf/progs/struct_ops_refcounted_fail__ref_leak.c
>
> diff --git a/tools/testing/selftests/bpf/bpf_testmod/bpf_testmod.c b/tools/testing/selftests/bpf/bpf_testmod/bpf_testmod.c
> index 987d41af71d2..244234546ae2 100644
> --- a/tools/testing/selftests/bpf/bpf_testmod/bpf_testmod.c
> +++ b/tools/testing/selftests/bpf/bpf_testmod/bpf_testmod.c
> @@ -1135,10 +1135,17 @@ static int bpf_testmod_ops__test_maybe_null(int dummy,
>   	return 0;
>   }
>   
> +static int bpf_testmod_ops__test_refcounted(int dummy,
> +					    struct task_struct *task__ref)
> +{
> +	return 0;
> +}
> +
>   static struct bpf_testmod_ops __bpf_testmod_ops = {
>   	.test_1 = bpf_testmod_test_1,
>   	.test_2 = bpf_testmod_test_2,
>   	.test_maybe_null = bpf_testmod_ops__test_maybe_null,
> +	.test_refcounted = bpf_testmod_ops__test_refcounted,
>   };
>   
>   struct bpf_struct_ops bpf_bpf_testmod_ops = {
> diff --git a/tools/testing/selftests/bpf/bpf_testmod/bpf_testmod.h b/tools/testing/selftests/bpf/bpf_testmod/bpf_testmod.h
> index fb7dff47597a..0e31586c1353 100644
> --- a/tools/testing/selftests/bpf/bpf_testmod/bpf_testmod.h
> +++ b/tools/testing/selftests/bpf/bpf_testmod/bpf_testmod.h
> @@ -36,6 +36,8 @@ struct bpf_testmod_ops {
>   	/* Used to test nullable arguments. */
>   	int (*test_maybe_null)(int dummy, struct task_struct *task);
>   	int (*unsupported_ops)(void);
> +	/* Used to test ref_acquired arguments. */
> +	int (*test_refcounted)(int dummy, struct task_struct *task);
>   
>   	/* The following fields are used to test shadow copies. */
>   	char onebyte;
> diff --git a/tools/testing/selftests/bpf/prog_tests/test_struct_ops_refcounted.c b/tools/testing/selftests/bpf/prog_tests/test_struct_ops_refcounted.c
> new file mode 100644
> index 000000000000..976df951b700
> --- /dev/null
> +++ b/tools/testing/selftests/bpf/prog_tests/test_struct_ops_refcounted.c
> @@ -0,0 +1,58 @@
> +#include <test_progs.h>
> +
> +#include "struct_ops_refcounted.skel.h"
> +#include "struct_ops_refcounted_fail__ref_leak.skel.h"
> +#include "struct_ops_refcounted_fail__global_subprog.skel.h"
> +
> +/* Test that the verifier accepts a program that first acquires a referenced
> + * kptr through context and then releases the reference
> + */
> +static void refcounted(void)
> +{
> +	struct struct_ops_refcounted *skel;
> +
> +	skel = struct_ops_refcounted__open_and_load();
> +	if (!ASSERT_OK_PTR(skel, "struct_ops_module_open_and_load"))
> +		return;
> +
> +	struct_ops_refcounted__destroy(skel);
> +}
> +
> +/* Test that the verifier rejects a program that acquires a referenced
> + * kptr through context without releasing the reference
> + */
> +static void refcounted_fail__ref_leak(void)
> +{
> +	struct struct_ops_refcounted_fail__ref_leak *skel;
> +
> +	skel = struct_ops_refcounted_fail__ref_leak__open_and_load();
> +	if (ASSERT_ERR_PTR(skel, "struct_ops_module_fail__open_and_load"))
> +		return;
> +
> +	struct_ops_refcounted_fail__ref_leak__destroy(skel);
> +}
> +
> +/* Test that the verifier rejects a program that contains a global
> + * subprogram with referenced kptr arguments
> + */
> +static void refcounted_fail__global_subprog(void)
> +{
> +	struct struct_ops_refcounted_fail__global_subprog *skel;
> +
> +	skel = struct_ops_refcounted_fail__global_subprog__open_and_load();
> +	if (ASSERT_ERR_PTR(skel, "struct_ops_module_fail__open_and_load"))
> +		return;
> +
> +	struct_ops_refcounted_fail__global_subprog__destroy(skel);
> +}
> +
> +void test_struct_ops_refcounted(void)
> +{
> +	if (test__start_subtest("refcounted"))
> +		refcounted();
> +	if (test__start_subtest("refcounted_fail__ref_leak"))
> +		refcounted_fail__ref_leak();
> +	if (test__start_subtest("refcounted_fail__global_subprog"))
> +		refcounted_fail__global_subprog();
> +}
> +
> diff --git a/tools/testing/selftests/bpf/progs/struct_ops_refcounted.c b/tools/testing/selftests/bpf/progs/struct_ops_refcounted.c
> new file mode 100644
> index 000000000000..2c1326668b92
> --- /dev/null
> +++ b/tools/testing/selftests/bpf/progs/struct_ops_refcounted.c
> @@ -0,0 +1,67 @@
> +#include <vmlinux.h>
> +#include <bpf/bpf_tracing.h>
> +#include "../bpf_testmod/bpf_testmod.h"
> +#include "bpf_misc.h"
> +
> +char _license[] SEC("license") = "GPL";
> +
> +extern void bpf_task_release(struct task_struct *p) __ksym;
> +
> +/* This is a test BPF program that uses struct_ops to access a referenced
> + * kptr argument. This is a test for the verifier to ensure that it
> + * 1) recongnizes the task as a referenced object (i.e., ref_obj_id > 0), and
> + * 2) the same reference can be acquired from multiple paths as long as it
> + *    has not been released.
> + *
> + * test_refcounted() is equivalent to the C code below. It is written in assembly
> + * to avoid reads from task (i.e., getting referenced kptrs to task) being merged
> + * into single path by the compiler.
> + *
> + * int test_refcounted(int dummy, struct task_struct *task)
> + * {
> + *         if (dummy % 2)
> + *                 bpf_task_release(task);
> + *         else
> + *                 bpf_task_release(task);
> + *         return 0;
> + * }
> + */
> +SEC("struct_ops/test_refcounted")
> +int test_refcounted(unsigned long long *ctx)
> +{
> +	asm volatile ("					\
> +	/* r6 = dummy */				\
> +	r6 = *(u64 *)(r1 + 0x0);			\
> +	/* if (r6 & 0x1 != 0) */			\
> +	r6 &= 0x1;					\
> +	if r6 == 0 goto l0_%=;				\
> +	/* r1 = task */					\
> +	r1 = *(u64 *)(r1 + 0x8);			\
> +	call %[bpf_task_release];			\
> +	goto l1_%=;					\
> +l0_%=:	/* r1 = task */					\
> +	r1 = *(u64 *)(r1 + 0x8);			\
> +	call %[bpf_task_release];			\
> +l1_%=:	/* return 0 */					\
> +"	:
> +	: __imm(bpf_task_release)
> +	: __clobber_all);
> +	return 0;
> +}

You can use clang nomerge attribute to prevent bpf_task_release(task) merging. For example,

$ cat t.c
struct task_struct {
         int a;
         int b;
         int d[20];
};


__attribute__((nomerge)) extern void bpf_task_release(struct task_struct *task);

int test_refcounted(int dummy, struct task_struct *task)
{
         if (dummy % 2)
                 bpf_task_release(task);
         else
                 bpf_task_release(task);
         return 0;
}

$ clang --version
clang version 19.1.5 (https://github.com/llvm/llvm-project.git ab4b5a2db582958af1ee308a790cfdb42bd24720)
Target: x86_64-unknown-linux-gnu
Thread model: posix
InstalledDir: /home/yhs/work/llvm-project/llvm/build.19/Release/bin
$ clang --target=bpf -O2 -mcpu=v3 -S t.c
$ cat t.s
         .text
         .file   "t.c"
         .globl  test_refcounted                 # -- Begin function test_refcounted
         .p2align        3
         .type   test_refcounted,@function
test_refcounted:                        # @test_refcounted
# %bb.0:
         w1 &= 1
         if w1 == 0 goto LBB0_2
# %bb.1:
         r1 = r2
         call bpf_task_release
         goto LBB0_3
LBB0_2:
         r1 = r2
         call bpf_task_release
LBB0_3:
         w0 = 0
         exit
.Lfunc_end0:
         .size   test_refcounted, .Lfunc_end0-test_refcounted
                                         # -- End function
         .addrsig

> +
> +/* BTF FUNC records are not generated for kfuncs referenced
> + * from inline assembly. These records are necessary for
> + * libbpf to link the program. The function below is a hack
> + * to ensure that BTF FUNC records are generated.
> + */
> +void __btf_root(void)
> +{
> +	bpf_task_release(NULL);
> +}
> +
> +SEC(".struct_ops.link")
> +struct bpf_testmod_ops testmod_refcounted = {
> +	.test_refcounted = (void *)test_refcounted,
> +};
> +
> +
> diff --git a/tools/testing/selftests/bpf/progs/struct_ops_refcounted_fail__global_subprog.c b/tools/testing/selftests/bpf/progs/struct_ops_refcounted_fail__global_subprog.c
> new file mode 100644
> index 000000000000..c7e84e63b053
> --- /dev/null
> +++ b/tools/testing/selftests/bpf/progs/struct_ops_refcounted_fail__global_subprog.c
> @@ -0,0 +1,32 @@
> +#include <vmlinux.h>
> +#include <bpf/bpf_tracing.h>
> +#include "../bpf_testmod/bpf_testmod.h"
> +
> +char _license[] SEC("license") = "GPL";
> +
> +extern void bpf_task_release(struct task_struct *p) __ksym;
> +
> +__noinline int subprog_release(__u64 *ctx __arg_ctx)
> +{
> +	struct task_struct *task = (struct task_struct *)ctx[1];
> +	int dummy = (int)ctx[0];
> +
> +	bpf_task_release(task);
> +
> +	return dummy + 1;
> +}
> +
> +SEC("struct_ops/test_refcounted")
> +int test_refcounted(unsigned long long *ctx)
> +{
> +	struct task_struct *task = (struct task_struct *)ctx[1];
> +
> +	bpf_task_release(task);
> +
> +	return subprog_release(ctx);
> +}
> +
> +SEC(".struct_ops.link")
> +struct bpf_testmod_ops testmod_ref_acquire = {
> +	.test_refcounted = (void *)test_refcounted,
> +};
> diff --git a/tools/testing/selftests/bpf/progs/struct_ops_refcounted_fail__ref_leak.c b/tools/testing/selftests/bpf/progs/struct_ops_refcounted_fail__ref_leak.c
> new file mode 100644
> index 000000000000..6e82859eb187
> --- /dev/null
> +++ b/tools/testing/selftests/bpf/progs/struct_ops_refcounted_fail__ref_leak.c
> @@ -0,0 +1,17 @@
> +#include <vmlinux.h>
> +#include <bpf/bpf_tracing.h>
> +#include "../bpf_testmod/bpf_testmod.h"
> +
> +char _license[] SEC("license") = "GPL";
> +
> +SEC("struct_ops/test_refcounted")
> +int BPF_PROG(test_refcounted, int dummy,
> +	     struct task_struct *task)
> +{
> +	return 0;
> +}
> +
> +SEC(".struct_ops.link")
> +struct bpf_testmod_ops testmod_ref_acquire = {
> +	.test_refcounted = (void *)test_refcounted,
> +};
Amery Hung Dec. 19, 2024, 8:49 p.m. UTC | #4 
On Wed, Dec 18, 2024 at 7:41 PM Yonghong Song <yonghong.song@linux.dev> wrote:
>
>
>
>
> On 12/13/24 3:29 PM, Amery Hung wrote:
> > Test referenced kptr acquired through struct_ops argument tagged with
> > "__ref". The success case checks whether 1) a reference to the correct
> > type is acquired, and 2) the referenced kptr argument can be accessed in
> > multiple paths as long as it hasn't been released. In the fail cases,
> > we first confirm that a referenced kptr acquried through a struct_ops
> > argument is not allowed to be leaked. Then, we make sure this new
> > referenced kptr acquiring mechanism does not accidentally allow referenced
> > kptrs to flow into global subprograms through their arguments.
> >
> > Signed-off-by: Amery Hung <amery.hung@bytedance.com>
> > ---
> >   .../selftests/bpf/bpf_testmod/bpf_testmod.c   |  7 ++
> >   .../selftests/bpf/bpf_testmod/bpf_testmod.h   |  2 +
> >   .../prog_tests/test_struct_ops_refcounted.c   | 58 ++++++++++++++++
> >   .../bpf/progs/struct_ops_refcounted.c         | 67 +++++++++++++++++++
> >   ...ruct_ops_refcounted_fail__global_subprog.c | 32 +++++++++
> >   .../struct_ops_refcounted_fail__ref_leak.c    | 17 +++++
> >   6 files changed, 183 insertions(+)
> >   create mode 100644 tools/testing/selftests/bpf/prog_tests/test_struct_ops_refcounted.c
> >   create mode 100644 tools/testing/selftests/bpf/progs/struct_ops_refcounted.c
> >   create mode 100644 tools/testing/selftests/bpf/progs/struct_ops_refcounted_fail__global_subprog.c
> >   create mode 100644 tools/testing/selftests/bpf/progs/struct_ops_refcounted_fail__ref_leak.c
> >
> > diff --git a/tools/testing/selftests/bpf/bpf_testmod/bpf_testmod.c b/tools/testing/selftests/bpf/bpf_testmod/bpf_testmod.c
> > index 987d41af71d2..244234546ae2 100644
> > --- a/tools/testing/selftests/bpf/bpf_testmod/bpf_testmod.c
> > +++ b/tools/testing/selftests/bpf/bpf_testmod/bpf_testmod.c
> > @@ -1135,10 +1135,17 @@ static int bpf_testmod_ops__test_maybe_null(int dummy,
> >       return 0;
> >   }
> >
> > +static int bpf_testmod_ops__test_refcounted(int dummy,
> > +                                         struct task_struct *task__ref)
> > +{
> > +     return 0;
> > +}
> > +
> >   static struct bpf_testmod_ops __bpf_testmod_ops = {
> >       .test_1 = bpf_testmod_test_1,
> >       .test_2 = bpf_testmod_test_2,
> >       .test_maybe_null = bpf_testmod_ops__test_maybe_null,
> > +     .test_refcounted = bpf_testmod_ops__test_refcounted,
> >   };
> >
> >   struct bpf_struct_ops bpf_bpf_testmod_ops = {
> > diff --git a/tools/testing/selftests/bpf/bpf_testmod/bpf_testmod.h b/tools/testing/selftests/bpf/bpf_testmod/bpf_testmod.h
> > index fb7dff47597a..0e31586c1353 100644
> > --- a/tools/testing/selftests/bpf/bpf_testmod/bpf_testmod.h
> > +++ b/tools/testing/selftests/bpf/bpf_testmod/bpf_testmod.h
> > @@ -36,6 +36,8 @@ struct bpf_testmod_ops {
> >       /* Used to test nullable arguments. */
> >       int (*test_maybe_null)(int dummy, struct task_struct *task);
> >       int (*unsupported_ops)(void);
> > +     /* Used to test ref_acquired arguments. */
> > +     int (*test_refcounted)(int dummy, struct task_struct *task);
> >
> >       /* The following fields are used to test shadow copies. */
> >       char onebyte;
> > diff --git a/tools/testing/selftests/bpf/prog_tests/test_struct_ops_refcounted.c b/tools/testing/selftests/bpf/prog_tests/test_struct_ops_refcounted.c
> > new file mode 100644
> > index 000000000000..976df951b700
> > --- /dev/null
> > +++ b/tools/testing/selftests/bpf/prog_tests/test_struct_ops_refcounted.c
> > @@ -0,0 +1,58 @@
> > +#include <test_progs.h>
> > +
> > +#include "struct_ops_refcounted.skel.h"
> > +#include "struct_ops_refcounted_fail__ref_leak.skel.h"
> > +#include "struct_ops_refcounted_fail__global_subprog.skel.h"
> > +
> > +/* Test that the verifier accepts a program that first acquires a referenced
> > + * kptr through context and then releases the reference
> > + */
> > +static void refcounted(void)
> > +{
> > +     struct struct_ops_refcounted *skel;
> > +
> > +     skel = struct_ops_refcounted__open_and_load();
> > +     if (!ASSERT_OK_PTR(skel, "struct_ops_module_open_and_load"))
> > +             return;
> > +
> > +     struct_ops_refcounted__destroy(skel);
> > +}
> > +
> > +/* Test that the verifier rejects a program that acquires a referenced
> > + * kptr through context without releasing the reference
> > + */
> > +static void refcounted_fail__ref_leak(void)
> > +{
> > +     struct struct_ops_refcounted_fail__ref_leak *skel;
> > +
> > +     skel = struct_ops_refcounted_fail__ref_leak__open_and_load();
> > +     if (ASSERT_ERR_PTR(skel, "struct_ops_module_fail__open_and_load"))
> > +             return;
> > +
> > +     struct_ops_refcounted_fail__ref_leak__destroy(skel);
> > +}
> > +
> > +/* Test that the verifier rejects a program that contains a global
> > + * subprogram with referenced kptr arguments
> > + */
> > +static void refcounted_fail__global_subprog(void)
> > +{
> > +     struct struct_ops_refcounted_fail__global_subprog *skel;
> > +
> > +     skel = struct_ops_refcounted_fail__global_subprog__open_and_load();
> > +     if (ASSERT_ERR_PTR(skel, "struct_ops_module_fail__open_and_load"))
> > +             return;
> > +
> > +     struct_ops_refcounted_fail__global_subprog__destroy(skel);
> > +}
> > +
> > +void test_struct_ops_refcounted(void)
> > +{
> > +     if (test__start_subtest("refcounted"))
> > +             refcounted();
> > +     if (test__start_subtest("refcounted_fail__ref_leak"))
> > +             refcounted_fail__ref_leak();
> > +     if (test__start_subtest("refcounted_fail__global_subprog"))
> > +             refcounted_fail__global_subprog();
> > +}
> > +
> > diff --git a/tools/testing/selftests/bpf/progs/struct_ops_refcounted.c b/tools/testing/selftests/bpf/progs/struct_ops_refcounted.c
> > new file mode 100644
> > index 000000000000..2c1326668b92
> > --- /dev/null
> > +++ b/tools/testing/selftests/bpf/progs/struct_ops_refcounted.c
> > @@ -0,0 +1,67 @@
> > +#include <vmlinux.h>
> > +#include <bpf/bpf_tracing.h>
> > +#include "../bpf_testmod/bpf_testmod.h"
> > +#include "bpf_misc.h"
> > +
> > +char _license[] SEC("license") = "GPL";
> > +
> > +extern void bpf_task_release(struct task_struct *p) __ksym;
> > +
> > +/* This is a test BPF program that uses struct_ops to access a referenced
> > + * kptr argument. This is a test for the verifier to ensure that it
> > + * 1) recongnizes the task as a referenced object (i.e., ref_obj_id > 0), and
> > + * 2) the same reference can be acquired from multiple paths as long as it
> > + *    has not been released.
> > + *
> > + * test_refcounted() is equivalent to the C code below. It is written in assembly
> > + * to avoid reads from task (i.e., getting referenced kptrs to task) being merged
> > + * into single path by the compiler.
> > + *
> > + * int test_refcounted(int dummy, struct task_struct *task)
> > + * {
> > + *         if (dummy % 2)
> > + *                 bpf_task_release(task);
> > + *         else
> > + *                 bpf_task_release(task);
> > + *         return 0;
> > + * }
> > + */
> > +SEC("struct_ops/test_refcounted")
> > +int test_refcounted(unsigned long long *ctx)
> > +{
> > +     asm volatile ("                                 \
> > +     /* r6 = dummy */                                \
> > +     r6 = *(u64 *)(r1 + 0x0);                        \
> > +     /* if (r6 & 0x1 != 0) */                        \
> > +     r6 &= 0x1;                                      \
> > +     if r6 == 0 goto l0_%=;                          \
> > +     /* r1 = task */                                 \
> > +     r1 = *(u64 *)(r1 + 0x8);                        \
> > +     call %[bpf_task_release];                       \
> > +     goto l1_%=;                                     \
> > +l0_%=:       /* r1 = task */                                 \
> > +     r1 = *(u64 *)(r1 + 0x8);                        \
> > +     call %[bpf_task_release];                       \
> > +l1_%=:       /* return 0 */                                  \
> > +"    :
> > +     : __imm(bpf_task_release)
> > +     : __clobber_all);
> > +     return 0;
> > +}
>
> You can use clang nomerge attribute to prevent bpf_task_release(task) merging. For example,
>

Thanks for the info! That simplifies this test a lot. I will change it
in the next version.

> $ cat t.c
> struct task_struct {
>          int a;
>          int b;
>          int d[20];
> };
>
>
> __attribute__((nomerge)) extern void bpf_task_release(struct task_struct *task);
>
> int test_refcounted(int dummy, struct task_struct *task)
> {
>          if (dummy % 2)
>                  bpf_task_release(task);
>          else
>                  bpf_task_release(task);
>          return 0;
> }
>
> $ clang --version
> clang version 19.1.5 (https://github.com/llvm/llvm-project.git ab4b5a2db582958af1ee308a790cfdb42bd24720)
> Target: x86_64-unknown-linux-gnu
> Thread model: posix
> InstalledDir: /home/yhs/work/llvm-project/llvm/build.19/Release/bin
> $ clang --target=bpf -O2 -mcpu=v3 -S t.c
> $ cat t.s
>          .text
>          .file   "t.c"
>          .globl  test_refcounted                 # -- Begin function test_refcounted
>          .p2align        3
>          .type   test_refcounted,@function
> test_refcounted:                        # @test_refcounted
> # %bb.0:
>          w1 &= 1
>          if w1 == 0 goto LBB0_2
> # %bb.1:
>          r1 = r2
>          call bpf_task_release
>          goto LBB0_3
> LBB0_2:
>          r1 = r2
>          call bpf_task_release
> LBB0_3:
>          w0 = 0
>          exit
> .Lfunc_end0:
>          .size   test_refcounted, .Lfunc_end0-test_refcounted
>                                          # -- End function
>          .addrsig
>
> > +
> > +/* BTF FUNC records are not generated for kfuncs referenced
> > + * from inline assembly. These records are necessary for
> > + * libbpf to link the program. The function below is a hack
> > + * to ensure that BTF FUNC records are generated.
> > + */
> > +void __btf_root(void)
> > +{
> > +     bpf_task_release(NULL);
> > +}
> > +
> > +SEC(".struct_ops.link")
> > +struct bpf_testmod_ops testmod_refcounted = {
> > +     .test_refcounted = (void *)test_refcounted,
> > +};
> > +
> > +
> > diff --git a/tools/testing/selftests/bpf/progs/struct_ops_refcounted_fail__global_subprog.c b/tools/testing/selftests/bpf/progs/struct_ops_refcounted_fail__global_subprog.c
> > new file mode 100644
> > index 000000000000..c7e84e63b053
> > --- /dev/null
> > +++ b/tools/testing/selftests/bpf/progs/struct_ops_refcounted_fail__global_subprog.c
> > @@ -0,0 +1,32 @@
> > +#include <vmlinux.h>
> > +#include <bpf/bpf_tracing.h>
> > +#include "../bpf_testmod/bpf_testmod.h"
> > +
> > +char _license[] SEC("license") = "GPL";
> > +
> > +extern void bpf_task_release(struct task_struct *p) __ksym;
> > +
> > +__noinline int subprog_release(__u64 *ctx __arg_ctx)
> > +{
> > +     struct task_struct *task = (struct task_struct *)ctx[1];
> > +     int dummy = (int)ctx[0];
> > +
> > +     bpf_task_release(task);
> > +
> > +     return dummy + 1;
> > +}
> > +
> > +SEC("struct_ops/test_refcounted")
> > +int test_refcounted(unsigned long long *ctx)
> > +{
> > +     struct task_struct *task = (struct task_struct *)ctx[1];
> > +
> > +     bpf_task_release(task);
> > +
> > +     return subprog_release(ctx);
> > +}
> > +
> > +SEC(".struct_ops.link")
> > +struct bpf_testmod_ops testmod_ref_acquire = {
> > +     .test_refcounted = (void *)test_refcounted,
> > +};
> > diff --git a/tools/testing/selftests/bpf/progs/struct_ops_refcounted_fail__ref_leak.c b/tools/testing/selftests/bpf/progs/struct_ops_refcounted_fail__ref_leak.c
> > new file mode 100644
> > index 000000000000..6e82859eb187
> > --- /dev/null
> > +++ b/tools/testing/selftests/bpf/progs/struct_ops_refcounted_fail__ref_leak.c
> > @@ -0,0 +1,17 @@
> > +#include <vmlinux.h>
> > +#include <bpf/bpf_tracing.h>
> > +#include "../bpf_testmod/bpf_testmod.h"
> > +
> > +char _license[] SEC("license") = "GPL";
> > +
> > +SEC("struct_ops/test_refcounted")
> > +int BPF_PROG(test_refcounted, int dummy,
> > +          struct task_struct *task)
> > +{
> > +     return 0;
> > +}
> > +
> > +SEC(".struct_ops.link")
> > +struct bpf_testmod_ops testmod_ref_acquire = {
> > +     .test_refcounted = (void *)test_refcounted,
> > +};
>
diff mbox series

Patch

diff --git a/tools/testing/selftests/bpf/bpf_testmod/bpf_testmod.c b/tools/testing/selftests/bpf/bpf_testmod/bpf_testmod.c
index 987d41af71d2..244234546ae2 100644
--- a/tools/testing/selftests/bpf/bpf_testmod/bpf_testmod.c
+++ b/tools/testing/selftests/bpf/bpf_testmod/bpf_testmod.c
@@ -1135,10 +1135,17 @@  static int bpf_testmod_ops__test_maybe_null(int dummy,
 	return 0;
 }
 
+static int bpf_testmod_ops__test_refcounted(int dummy,
+					    struct task_struct *task__ref)
+{
+	return 0;
+}
+
 static struct bpf_testmod_ops __bpf_testmod_ops = {
 	.test_1 = bpf_testmod_test_1,
 	.test_2 = bpf_testmod_test_2,
 	.test_maybe_null = bpf_testmod_ops__test_maybe_null,
+	.test_refcounted = bpf_testmod_ops__test_refcounted,
 };
 
 struct bpf_struct_ops bpf_bpf_testmod_ops = {
diff --git a/tools/testing/selftests/bpf/bpf_testmod/bpf_testmod.h b/tools/testing/selftests/bpf/bpf_testmod/bpf_testmod.h
index fb7dff47597a..0e31586c1353 100644
--- a/tools/testing/selftests/bpf/bpf_testmod/bpf_testmod.h
+++ b/tools/testing/selftests/bpf/bpf_testmod/bpf_testmod.h
@@ -36,6 +36,8 @@  struct bpf_testmod_ops {
 	/* Used to test nullable arguments. */
 	int (*test_maybe_null)(int dummy, struct task_struct *task);
 	int (*unsupported_ops)(void);
+	/* Used to test ref_acquired arguments. */
+	int (*test_refcounted)(int dummy, struct task_struct *task);
 
 	/* The following fields are used to test shadow copies. */
 	char onebyte;
diff --git a/tools/testing/selftests/bpf/prog_tests/test_struct_ops_refcounted.c b/tools/testing/selftests/bpf/prog_tests/test_struct_ops_refcounted.c
new file mode 100644
index 000000000000..976df951b700
--- /dev/null
+++ b/tools/testing/selftests/bpf/prog_tests/test_struct_ops_refcounted.c
@@ -0,0 +1,58 @@ 
+#include <test_progs.h>
+
+#include "struct_ops_refcounted.skel.h"
+#include "struct_ops_refcounted_fail__ref_leak.skel.h"
+#include "struct_ops_refcounted_fail__global_subprog.skel.h"
+
+/* Test that the verifier accepts a program that first acquires a referenced
+ * kptr through context and then releases the reference
+ */
+static void refcounted(void)
+{
+	struct struct_ops_refcounted *skel;
+
+	skel = struct_ops_refcounted__open_and_load();
+	if (!ASSERT_OK_PTR(skel, "struct_ops_module_open_and_load"))
+		return;
+
+	struct_ops_refcounted__destroy(skel);
+}
+
+/* Test that the verifier rejects a program that acquires a referenced
+ * kptr through context without releasing the reference
+ */
+static void refcounted_fail__ref_leak(void)
+{
+	struct struct_ops_refcounted_fail__ref_leak *skel;
+
+	skel = struct_ops_refcounted_fail__ref_leak__open_and_load();
+	if (ASSERT_ERR_PTR(skel, "struct_ops_module_fail__open_and_load"))
+		return;
+
+	struct_ops_refcounted_fail__ref_leak__destroy(skel);
+}
+
+/* Test that the verifier rejects a program that contains a global
+ * subprogram with referenced kptr arguments
+ */
+static void refcounted_fail__global_subprog(void)
+{
+	struct struct_ops_refcounted_fail__global_subprog *skel;
+
+	skel = struct_ops_refcounted_fail__global_subprog__open_and_load();
+	if (ASSERT_ERR_PTR(skel, "struct_ops_module_fail__open_and_load"))
+		return;
+
+	struct_ops_refcounted_fail__global_subprog__destroy(skel);
+}
+
+void test_struct_ops_refcounted(void)
+{
+	if (test__start_subtest("refcounted"))
+		refcounted();
+	if (test__start_subtest("refcounted_fail__ref_leak"))
+		refcounted_fail__ref_leak();
+	if (test__start_subtest("refcounted_fail__global_subprog"))
+		refcounted_fail__global_subprog();
+}
+
diff --git a/tools/testing/selftests/bpf/progs/struct_ops_refcounted.c b/tools/testing/selftests/bpf/progs/struct_ops_refcounted.c
new file mode 100644
index 000000000000..2c1326668b92
--- /dev/null
+++ b/tools/testing/selftests/bpf/progs/struct_ops_refcounted.c
@@ -0,0 +1,67 @@ 
+#include <vmlinux.h>
+#include <bpf/bpf_tracing.h>
+#include "../bpf_testmod/bpf_testmod.h"
+#include "bpf_misc.h"
+
+char _license[] SEC("license") = "GPL";
+
+extern void bpf_task_release(struct task_struct *p) __ksym;
+
+/* This is a test BPF program that uses struct_ops to access a referenced
+ * kptr argument. This is a test for the verifier to ensure that it
+ * 1) recongnizes the task as a referenced object (i.e., ref_obj_id > 0), and
+ * 2) the same reference can be acquired from multiple paths as long as it
+ *    has not been released.
+ *
+ * test_refcounted() is equivalent to the C code below. It is written in assembly
+ * to avoid reads from task (i.e., getting referenced kptrs to task) being merged
+ * into single path by the compiler.
+ *
+ * int test_refcounted(int dummy, struct task_struct *task)
+ * {
+ *         if (dummy % 2)
+ *                 bpf_task_release(task);
+ *         else
+ *                 bpf_task_release(task);
+ *         return 0;
+ * }
+ */
+SEC("struct_ops/test_refcounted")
+int test_refcounted(unsigned long long *ctx)
+{
+	asm volatile ("					\
+	/* r6 = dummy */				\
+	r6 = *(u64 *)(r1 + 0x0);			\
+	/* if (r6 & 0x1 != 0) */			\
+	r6 &= 0x1;					\
+	if r6 == 0 goto l0_%=;				\
+	/* r1 = task */					\
+	r1 = *(u64 *)(r1 + 0x8);			\
+	call %[bpf_task_release];			\
+	goto l1_%=;					\
+l0_%=:	/* r1 = task */					\
+	r1 = *(u64 *)(r1 + 0x8);			\
+	call %[bpf_task_release];			\
+l1_%=:	/* return 0 */					\
+"	:
+	: __imm(bpf_task_release)
+	: __clobber_all);
+	return 0;
+}
+
+/* BTF FUNC records are not generated for kfuncs referenced
+ * from inline assembly. These records are necessary for
+ * libbpf to link the program. The function below is a hack
+ * to ensure that BTF FUNC records are generated.
+ */
+void __btf_root(void)
+{
+	bpf_task_release(NULL);
+}
+
+SEC(".struct_ops.link")
+struct bpf_testmod_ops testmod_refcounted = {
+	.test_refcounted = (void *)test_refcounted,
+};
+
+
diff --git a/tools/testing/selftests/bpf/progs/struct_ops_refcounted_fail__global_subprog.c b/tools/testing/selftests/bpf/progs/struct_ops_refcounted_fail__global_subprog.c
new file mode 100644
index 000000000000..c7e84e63b053
--- /dev/null
+++ b/tools/testing/selftests/bpf/progs/struct_ops_refcounted_fail__global_subprog.c
@@ -0,0 +1,32 @@ 
+#include <vmlinux.h>
+#include <bpf/bpf_tracing.h>
+#include "../bpf_testmod/bpf_testmod.h"
+
+char _license[] SEC("license") = "GPL";
+
+extern void bpf_task_release(struct task_struct *p) __ksym;
+
+__noinline int subprog_release(__u64 *ctx __arg_ctx)
+{
+	struct task_struct *task = (struct task_struct *)ctx[1];
+	int dummy = (int)ctx[0];
+
+	bpf_task_release(task);
+
+	return dummy + 1;
+}
+
+SEC("struct_ops/test_refcounted")
+int test_refcounted(unsigned long long *ctx)
+{
+	struct task_struct *task = (struct task_struct *)ctx[1];
+
+	bpf_task_release(task);
+
+	return subprog_release(ctx);
+}
+
+SEC(".struct_ops.link")
+struct bpf_testmod_ops testmod_ref_acquire = {
+	.test_refcounted = (void *)test_refcounted,
+};
diff --git a/tools/testing/selftests/bpf/progs/struct_ops_refcounted_fail__ref_leak.c b/tools/testing/selftests/bpf/progs/struct_ops_refcounted_fail__ref_leak.c
new file mode 100644
index 000000000000..6e82859eb187
--- /dev/null
+++ b/tools/testing/selftests/bpf/progs/struct_ops_refcounted_fail__ref_leak.c
@@ -0,0 +1,17 @@ 
+#include <vmlinux.h>
+#include <bpf/bpf_tracing.h>
+#include "../bpf_testmod/bpf_testmod.h"
+
+char _license[] SEC("license") = "GPL";
+
+SEC("struct_ops/test_refcounted")
+int BPF_PROG(test_refcounted, int dummy,
+	     struct task_struct *task)
+{
+	return 0;
+}
+
+SEC(".struct_ops.link")
+struct bpf_testmod_ops testmod_ref_acquire = {
+	.test_refcounted = (void *)test_refcounted,
+};