netdev - Re: [PATCH bpf-next v3 1/5] bpf: Support __nullable argument suffix for tp

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: <c5974d0f-0636-470e-87ef-b2936d54cd87@linux.dev>
Date: Wed, 11 Sep 2024 10:30:27 -0700
From: Martin KaFai Lau <martin.lau@...ux.dev>
To: Philo Lu <lulie@...ux.alibaba.com>
Cc: edumazet@...gle.com, rostedt@...dmis.org, mhiramat@...nel.org,
 mathieu.desnoyers@...icios.com, ast@...nel.org, daniel@...earbox.net,
 andrii@...nel.org, eddyz87@...il.com, song@...nel.org,
 yonghong.song@...ux.dev, john.fastabend@...il.com, kpsingh@...nel.org,
 sdf@...ichev.me, haoluo@...gle.com, jolsa@...nel.org, davem@...emloft.net,
 kuba@...nel.org, pabeni@...hat.com, mykolal@...com, shuah@...nel.org,
 mcoquelin.stm32@...il.com, alexandre.torgue@...s.st.com,
 thinker.li@...il.com, juntong.deng@...look.com, jrife@...gle.com,
 alan.maguire@...cle.com, davemarchevsky@...com, dxu@...uu.xyz,
 vmalik@...hat.com, cupertino.miranda@...cle.com, mattbobrowski@...gle.com,
 xuanzhuo@...ux.alibaba.com, netdev@...r.kernel.org,
 linux-trace-kernel@...r.kernel.org, bpf@...r.kernel.org
Subject: Re: [PATCH bpf-next v3 1/5] bpf: Support __nullable argument suffix
 for tp_btf

On 9/10/24 8:37 PM, Philo Lu wrote:
> Pointers passed to tp_btf were trusted to be valid, but some tracepoints
> do take NULL pointer as input, such as trace_tcp_send_reset(). Then the
> invalid memory access cannot be detected by verifier.
> 
> This patch fix it by add a suffix "__nullable" to the unreliable
> argument. The suffix is shown in btf, and PTR_MAYBE_NULL will be added
> to nullable arguments. Then users must check the pointer before use it.
> 
> A problem here is that we use "btf_trace_##call" to search func_proto.
> As it is a typedef, argument names as well as the suffix are not
> recorded. To solve this, I use bpf_raw_event_map to find
> "__bpf_trace##template" from "btf_trace_##call", and then we can see the
> suffix.
> 
> Suggested-by: Alexei Starovoitov <ast@...nel.org>
> Signed-off-by: Philo Lu <lulie@...ux.alibaba.com>
> ---
>   kernel/bpf/btf.c      |  9 +++++++++
>   kernel/bpf/verifier.c | 36 ++++++++++++++++++++++++++++++++----
>   2 files changed, 41 insertions(+), 4 deletions(-)
> 
> diff --git a/kernel/bpf/btf.c b/kernel/bpf/btf.c
> index 1e29281653c62..d1ea38d08f301 100644
> --- a/kernel/bpf/btf.c
> +++ b/kernel/bpf/btf.c
> @@ -6385,6 +6385,12 @@ static bool prog_args_trusted(const struct bpf_prog *prog)
>   	}
>   }
>   
> +static bool prog_arg_maybe_null(const struct bpf_prog *prog, const struct btf *btf,

The "prog" arg is not used, so the following nit...


> +				const struct btf_param *arg)
> +{
> +	return btf_param_match_suffix(btf, arg, "__nullable");
> +}
> +
>   int btf_ctx_arg_offset(const struct btf *btf, const struct btf_type *func_proto,
>   		       u32 arg_no)
>   {
> @@ -6554,6 +6560,9 @@ bool btf_ctx_access(int off, int size, enum bpf_access_type type,
>   	if (prog_args_trusted(prog))
>   		info->reg_type |= PTR_TRUSTED;
>   
> +	if (prog_arg_maybe_null(prog, btf, &args[arg]))

... I changed it to directly use
btf_param_match_suffix(btf, &args[arg], "__nullable"),

and removed the new prog_arg_maybe_null(). There are already a few 
is_kfunc_arg_{nullable, ...} helpers in verifier.c. Maybe we can consider 
refactoring them (if) there are more use cases like this in the future.