Re: [PATCH bpf-next v5 7/9] selftests/bpf: Test kptr arrays and kptrs in nested struct fields.

[Eduard Zingerman <eddyz87@gmail.com>]

On Thu, 2024-05-09 at 18:13 -0700, Kui-Feng Lee wrote:
> Make sure that BPF programs can declare global kptr arrays and kptr fields
> in struct types that is the type of a global variable or the type of a
> nested descendant field in a global variable.
> 
> An array with only one element is special case, that it treats the element
> like a non-array kptr field. Nested arrays are also tested to ensure they
> are handled properly.
> 
> Signed-off-by: Kui-Feng Lee <thinker.li@gmail.com>
> ---
>  .../selftests/bpf/prog_tests/cpumask.c        |   5 +
>  .../selftests/bpf/progs/cpumask_success.c     | 133 ++++++++++++++++++
>  2 files changed, 138 insertions(+)
> 
> diff --git a/tools/testing/selftests/bpf/prog_tests/cpumask.c b/tools/testing/selftests/bpf/prog_tests/cpumask.c
> index ecf89df78109..2570bd4b0cb2 100644
> --- a/tools/testing/selftests/bpf/prog_tests/cpumask.c
> +++ b/tools/testing/selftests/bpf/prog_tests/cpumask.c
> @@ -18,6 +18,11 @@ static const char * const cpumask_success_testcases[] = {
>  	"test_insert_leave",
>  	"test_insert_remove_release",
>  	"test_global_mask_rcu",
> +	"test_global_mask_array_one_rcu",
> +	"test_global_mask_array_rcu",
> +	"test_global_mask_array_l2_rcu",
> +	"test_global_mask_nested_rcu",
> +	"test_global_mask_nested_deep_rcu",
>  	"test_cpumask_weight",
>  };
>  
> diff --git a/tools/testing/selftests/bpf/progs/cpumask_success.c b/tools/testing/selftests/bpf/progs/cpumask_success.c
> index 7a1e64c6c065..0b6383fa9958 100644
> --- a/tools/testing/selftests/bpf/progs/cpumask_success.c
> +++ b/tools/testing/selftests/bpf/progs/cpumask_success.c
> @@ -12,6 +12,25 @@ char _license[] SEC("license") = "GPL";
>  
>  int pid, nr_cpus;
>  
> +struct kptr_nested {
> +	struct bpf_cpumask __kptr * mask;
> +};
> +
> +struct kptr_nested_mid {
> +	int dummy;
> +	struct kptr_nested m;
> +};
> +
> +struct kptr_nested_deep {
> +	struct kptr_nested_mid ptrs[2];
> +};

For the sake of completeness, would it be possible to create a test
case where there are several struct arrays following each other?
E.g. as below:

struct foo {
  ... __kptr *a;
  ... __kptr *b;
}

struct bar {
  ... __kptr *c;
}

struct {
  struct foo foos[3];
  struct bar bars[2];
}

Just to check that offset is propagated correctly.

Also, in the tests below you check that a pointer to some object could
be put into an array at different indexes. Tbh, I find it not very
interesting if we want to check that offsets are correct.
Would it be possible to create an array of object kptrs,
put specific references at specific indexes and somehow check which
object ended up where? (not necessarily 'bpf_cpumask').

> +
> +private(MASK) static struct bpf_cpumask __kptr * global_mask_array[2];
> +private(MASK) static struct bpf_cpumask __kptr * global_mask_array_l2[2][1];
> +private(MASK) static struct bpf_cpumask __kptr * global_mask_array_one[1];
> +private(MASK) static struct kptr_nested global_mask_nested[2];
> +private(MASK) static struct kptr_nested_deep global_mask_nested_deep;
> +
>  static bool is_test_task(void)
>  {
>  	int cur_pid = bpf_get_current_pid_tgid() >> 32;
> @@ -460,6 +479,120 @@ int BPF_PROG(test_global_mask_rcu, struct task_struct *task, u64 clone_flags)
>  	return 0;
>  }
>  
> +SEC("tp_btf/task_newtask")
> +int BPF_PROG(test_global_mask_array_one_rcu, struct task_struct *task, u64 clone_flags)
> +{
> +	struct bpf_cpumask *local, *prev;
> +
> +	if (!is_test_task())
> +		return 0;
> +
> +	/* Kptr arrays with one element are special cased, being treated
> +	 * just like a single pointer.
> +	 */
> +
> +	local = create_cpumask();
> +	if (!local)
> +		return 0;
> +
> +	prev = bpf_kptr_xchg(&global_mask_array_one[0], local);
> +	if (prev) {
> +		bpf_cpumask_release(prev);
> +		err = 3;
> +		return 0;
> +	}
> +
> +	bpf_rcu_read_lock();
> +	local = global_mask_array_one[0];
> +	if (!local) {
> +		err = 4;
> +		bpf_rcu_read_unlock();
> +		return 0;
> +	}
> +
> +	bpf_rcu_read_unlock();
> +
> +	return 0;
> +}
> +
> +static int _global_mask_array_rcu(struct bpf_cpumask **mask0,
> +				  struct bpf_cpumask **mask1)
> +{
> +	struct bpf_cpumask *local;
> +
> +	if (!is_test_task())
> +		return 0;
> +
> +	/* Check if two kptrs in the array work and independently */
> +
> +	local = create_cpumask();
> +	if (!local)
> +		return 0;
> +
> +	bpf_rcu_read_lock();
> +
> +	local = bpf_kptr_xchg(mask0, local);
> +	if (local) {
> +		err = 1;
> +		goto err_exit;
> +	}
> +
> +	/* [<mask 0>, NULL] */
> +	if (!*mask0 || *mask1) {
> +		err = 2;
> +		goto err_exit;
> +	}
> +
> +	local = create_cpumask();
> +	if (!local) {
> +		err = 9;
> +		goto err_exit;
> +	}
> +
> +	local = bpf_kptr_xchg(mask1, local);
> +	if (local) {
> +		err = 10;
> +		goto err_exit;
> +	}
> +
> +	/* [<mask 0>, <mask 1>] */
> +	if (!*mask0 || !*mask1 || *mask0 == *mask1) {
> +		err = 11;
> +		goto err_exit;
> +	}
> +
> +err_exit:
> +	if (local)
> +		bpf_cpumask_release(local);
> +	bpf_rcu_read_unlock();
> +	return 0;
> +}
> +
> +SEC("tp_btf/task_newtask")
> +int BPF_PROG(test_global_mask_array_rcu, struct task_struct *task, u64 clone_flags)
> +{
> +	return _global_mask_array_rcu(&global_mask_array[0], &global_mask_array[1]);
> +}
> +
> +SEC("tp_btf/task_newtask")
> +int BPF_PROG(test_global_mask_array_l2_rcu, struct task_struct *task, u64 clone_flags)
> +{
> +	return _global_mask_array_rcu(&global_mask_array_l2[0][0], &global_mask_array_l2[1][0]);
> +}
> +
> +SEC("tp_btf/task_newtask")
> +int BPF_PROG(test_global_mask_nested_rcu, struct task_struct *task, u64 clone_flags)
> +{
> +	return _global_mask_array_rcu(&global_mask_nested[0].mask, &global_mask_nested[1].mask);
> +}
> +
> +SEC("tp_btf/task_newtask")
> +int BPF_PROG(test_global_mask_nested_deep_rcu, struct task_struct *task, u64 clone_flags)
> +{
> +	return _global_mask_array_rcu(&global_mask_nested_deep.ptrs[0].m.mask,
> +				      &global_mask_nested_deep.ptrs[1].m.mask);
> +}
> +
>  SEC("tp_btf/task_newtask")
>  int BPF_PROG(test_cpumask_weight, struct task_struct *task, u64 clone_flags)
>  {