netdev - Re: [PATCH bpf-next v2 2/5] bpf: factor out helpers to check ctx access for BTF function

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] [thread-next>] [day] [month] [year] [list]

Message-ID: <20211020050058.wpafu63ffwh5zc2x@kafai-mbp.dhcp.thefacebook.com>
Date:   Tue, 19 Oct 2021 22:00:58 -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 2/5] bpf: factor out helpers to check ctx
  access for BTF function

On Sat, Oct 16, 2021 at 08:48:03PM +0800, Hou Tao wrote:
> Factor out two helpers to check the read access of ctx for BTF
> function. bpf_check_btf_func_arg_access() is used to check the
> read access to argument is valid, and bpf_check_btf_func_ctx_access()
> also checks whether the btf type of argument is valid besides
> the checking of arguments read. bpf_check_btf_func_ctx_access()
> will be used by the following patch.
> 
> Signed-off-by: Hou Tao <houtao1@...wei.com>
> ---
>  include/linux/bpf.h      | 27 +++++++++++++++++++++++++++
>  kernel/trace/bpf_trace.c | 16 ++--------------
>  net/ipv4/bpf_tcp_ca.c    |  9 +--------
>  3 files changed, 30 insertions(+), 22 deletions(-)
> 
> diff --git a/include/linux/bpf.h b/include/linux/bpf.h
> index b7c1e2bc93f7..b503306da2ab 100644
> --- a/include/linux/bpf.h
> +++ b/include/linux/bpf.h
> @@ -1648,6 +1648,33 @@ bool bpf_prog_test_check_kfunc_call(u32 kfunc_id, struct module *owner);
>  bool btf_ctx_access(int off, int size, enum bpf_access_type type,
>  		    const struct bpf_prog *prog,
>  		    struct bpf_insn_access_aux *info);
> +
> +/*
> + * The maximum number of BTF function arguments is MAX_BPF_FUNC_ARGS.
> + * And only aligned read is allowed.
> + */
> +static inline bool bpf_check_btf_func_arg_access(int off, int size,
> +						 enum bpf_access_type type)
Refactoring makes sense but naming could be hard to figure out here.

"_btf_func_arg" part is confusing with other btf_check_func_arg
functions in btf.c.  e.g. it is checking an arg of a bpf subprog or
checking a bpf_prog's ctx here?  The name sounds former but it is actually
the latter here (i.e. checking ctx).  It is a ctx with an array
of __u64 for tracing.  How about bpf_tracing_ctx_access()?

> +{
> +	if (off < 0 || off >= sizeof(__u64) * MAX_BPF_FUNC_ARGS)
> +		return false;
> +	if (type != BPF_READ)
> +		return false;
> +	if (off % size != 0)
> +		return false;
> +	return true;
> +}
> +
> +static inline bool bpf_check_btf_func_ctx_access(int off, int size,
> +						 enum bpf_access_type type,
> +						 const struct bpf_prog *prog,
> +						 struct bpf_insn_access_aux *info)
and may be bpf_tracing_btf_ctx_access() here?

> +{
> +	if (!bpf_check_btf_func_arg_access(off, size, type))
> +		return false;
> +	return btf_ctx_access(off, size, type, prog, info);
> +}