netdev - Re: [PATCH bpf-next v2 4/5] bpf: hook .test_run for struct

lists.openwall.net		lists / announce owl-users owl-dev john-users john-dev passwdqc-users yescrypt popa3d-users / oss-security kernel-hardening musl sabotage tlsify passwords / crypt-dev xvendor / Bugtraq Full-Disclosure linux-kernel linux-netdev linux-ext4 linux-hardening linux-cve-announce PHC
Open Source and information security mailing list archives

Hash Suite: Windows password security audit tool. GUI, reports in PDF.

[<prev] [next>] [<thread-prev] [day] [month] [year] [list]

Message-ID: <20211020052412.zmzb4w3pipem6obj@kafai-mbp.dhcp.thefacebook.com>
Date:   Tue, 19 Oct 2021 22:24:12 -0700
From:   Martin KaFai Lau <kafai@...com>
To:     Hou Tao <houtao1@...wei.com>
CC:     Alexei Starovoitov <ast@...nel.org>, Yonghong Song <yhs@...com>,
        Daniel Borkmann <daniel@...earbox.net>,
        Andrii Nakryiko <andrii@...nel.org>, <netdev@...r.kernel.org>,
        <bpf@...r.kernel.org>
Subject: Re: [PATCH bpf-next v2 4/5] bpf: hook .test_run for struct_ops
 program

On Sat, Oct 16, 2021 at 08:48:05PM +0800, Hou Tao wrote:
> bpf_struct_ops_test_run() will be used to run struct_ops program
> from bpf_dummy_ops and now its main purpose is to test the handling
> of return value and multiple arguments.
> 
> Signed-off-by: Hou Tao <houtao1@...wei.com>
> ---
>  kernel/bpf/bpf_struct_ops.c | 17 +++++++++++++++++
>  1 file changed, 17 insertions(+)
> 
> diff --git a/kernel/bpf/bpf_struct_ops.c b/kernel/bpf/bpf_struct_ops.c
> index 44be101f2562..ceedc9f0f786 100644
> --- a/kernel/bpf/bpf_struct_ops.c
> +++ b/kernel/bpf/bpf_struct_ops.c
> @@ -11,6 +11,9 @@
>  #include <linux/refcount.h>
>  #include <linux/mutex.h>
>  
> +static int bpf_struct_ops_test_run(struct bpf_prog *prog,
> +				   const union bpf_attr *kattr,
> +				   union bpf_attr __user *uattr);
>  enum bpf_struct_ops_state {
>  	BPF_STRUCT_OPS_STATE_INIT,
>  	BPF_STRUCT_OPS_STATE_INUSE,
> @@ -93,6 +96,7 @@ const struct bpf_verifier_ops bpf_struct_ops_verifier_ops = {
>  };
>  
>  const struct bpf_prog_ops bpf_struct_ops_prog_ops = {
> +	.test_run = bpf_struct_ops_test_run,
>  };
>  
>  static const struct btf_type *module_type;
> @@ -667,3 +671,16 @@ void bpf_struct_ops_put(const void *kdata)
>  		call_rcu(&st_map->rcu, bpf_struct_ops_put_rcu);
>  	}
>  }
> +
> +static int bpf_struct_ops_test_run(struct bpf_prog *prog,
> +				   const union bpf_attr *kattr,
> +				   union bpf_attr __user *uattr)
> +{
> +	const struct bpf_struct_ops *st_ops;
> +
> +	st_ops = bpf_struct_ops_find(prog->aux->attach_btf_id);
Checking bpf_bpf_dummy_ops.type_id == prog->aux->attach_btf_id is as good?
then the bpf_struct_ops_find() should not be needed.

> +	if (st_ops != &bpf_bpf_dummy_ops)
> +		return -EOPNOTSUPP;
> +
> +	return bpf_dummy_struct_ops_test_run(prog, kattr, uattr);
The function bpf_dummy_struct_ops_test_run() is available under
CONFIG_NET.

How about checking the attach_btf_id in bpf_dummy_struct_ops_test_run().
and then rename s/bpf_dummy_struct_ops_test_run/bpf_struct_ops_test_run/.

and do this in bpf_struct_ops_prog_ops:

const struct bpf_prog_ops bpf_struct_ops_prog_ops = {
#ifdef CONFIG_NET
	.test_run = bpf_struct_ops_test_run,
#endif
};

Take a look at some test_run in bpf_trace.c as examples.