From: Martin KaFai Lau <martin.lau@linux•dev>
To: Philo Lu <lulie@linux•alibaba.com>
Cc: edumazet@google•com, rostedt@goodmis•org, mhiramat@kernel•org,
mathieu.desnoyers@efficios•com, ast@kernel•org,
daniel@iogearbox•net, andrii@kernel•org, eddyz87@gmail•com,
song@kernel•org, yonghong.song@linux•dev,
john.fastabend@gmail•com, kpsingh@kernel•org, sdf@fomichev•me,
haoluo@google•com, jolsa@kernel•org, davem@davemloft•net,
kuba@kernel•org, pabeni@redhat•com, mykolal@fb•com,
shuah@kernel•org, mcoquelin.stm32@gmail•com,
alexandre.torgue@foss•st.com, thinker.li@gmail•com,
juntong.deng@outlook•com, jrife@google•com,
alan.maguire@oracle•com, davemarchevsky@fb•com, dxu@dxuuu•xyz,
vmalik@redhat•com, cupertino.miranda@oracle•com,
mattbobrowski@google•com, xuanzhuo@linux•alibaba.com,
netdev@vger•kernel.org, linux-trace-kernel@vger•kernel.org,
bpf@vger•kernel.org
Subject: Re: [PATCH bpf-next v3 1/5] bpf: Support __nullable argument suffix for tp_btf
Date: Wed, 11 Sep 2024 10:30:27 -0700 [thread overview]
Message-ID: <c5974d0f-0636-470e-87ef-b2936d54cd87@linux.dev> (raw)
In-Reply-To: <20240911033719.91468-2-lulie@linux.alibaba.com>
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@kernel•org>
> Signed-off-by: Philo Lu <lulie@linux•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.
next prev parent reply other threads:[~2024-09-11 17:30 UTC|newest]
Thread overview: 9+ messages / expand[flat|nested] mbox.gz Atom feed top
2024-09-11 3:37 [PATCH bpf-next v3 0/5] bpf: Allow skb dynptr for tp_btf Philo Lu
2024-09-11 3:37 ` [PATCH bpf-next v3 1/5] bpf: Support __nullable argument suffix " Philo Lu
2024-09-11 17:30 ` Martin KaFai Lau [this message]
2024-09-11 3:37 ` [PATCH bpf-next v3 2/5] selftests/bpf: Add test for __nullable suffix in tp_btf Philo Lu
2024-09-11 3:37 ` [PATCH bpf-next v3 3/5] tcp: Use skb__nullable in trace_tcp_send_reset Philo Lu
2024-09-11 3:37 ` [PATCH bpf-next v3 4/5] bpf: Allow bpf_dynptr_from_skb() for tp_btf Philo Lu
2024-09-11 3:37 ` [PATCH bpf-next v3 5/5] selftests/bpf: Expand skb dynptr selftests " Philo Lu
2024-09-11 17:33 ` Martin KaFai Lau
2024-09-11 17:30 ` [PATCH bpf-next v3 0/5] bpf: Allow skb dynptr " patchwork-bot+netdevbpf
Reply instructions:
You may reply publicly to this message via plain-text email
using any one of the following methods:
* Save the following mbox file, import it into your mail client,
and reply-to-all from there: mbox
Avoid top-posting and favor interleaved quoting:
https://en.wikipedia.org/wiki/Posting_style#Interleaved_style
* Reply using the --to, --cc, and --in-reply-to
switches of git-send-email(1):
git send-email \
--in-reply-to=c5974d0f-0636-470e-87ef-b2936d54cd87@linux.dev \
--to=martin.lau@linux$(echo .)dev \
--cc=alan.maguire@oracle$(echo .)com \
--cc=alexandre.torgue@foss$(echo .)st.com \
--cc=andrii@kernel$(echo .)org \
--cc=ast@kernel$(echo .)org \
--cc=bpf@vger$(echo .)kernel.org \
--cc=cupertino.miranda@oracle$(echo .)com \
--cc=daniel@iogearbox$(echo .)net \
--cc=davem@davemloft$(echo .)net \
--cc=davemarchevsky@fb$(echo .)com \
--cc=dxu@dxuuu$(echo .)xyz \
--cc=eddyz87@gmail$(echo .)com \
--cc=edumazet@google$(echo .)com \
--cc=haoluo@google$(echo .)com \
--cc=john.fastabend@gmail$(echo .)com \
--cc=jolsa@kernel$(echo .)org \
--cc=jrife@google$(echo .)com \
--cc=juntong.deng@outlook$(echo .)com \
--cc=kpsingh@kernel$(echo .)org \
--cc=kuba@kernel$(echo .)org \
--cc=linux-trace-kernel@vger$(echo .)kernel.org \
--cc=lulie@linux$(echo .)alibaba.com \
--cc=mathieu.desnoyers@efficios$(echo .)com \
--cc=mattbobrowski@google$(echo .)com \
--cc=mcoquelin.stm32@gmail$(echo .)com \
--cc=mhiramat@kernel$(echo .)org \
--cc=mykolal@fb$(echo .)com \
--cc=netdev@vger$(echo .)kernel.org \
--cc=pabeni@redhat$(echo .)com \
--cc=rostedt@goodmis$(echo .)org \
--cc=sdf@fomichev$(echo .)me \
--cc=shuah@kernel$(echo .)org \
--cc=song@kernel$(echo .)org \
--cc=thinker.li@gmail$(echo .)com \
--cc=vmalik@redhat$(echo .)com \
--cc=xuanzhuo@linux$(echo .)alibaba.com \
--cc=yonghong.song@linux$(echo .)dev \
/path/to/YOUR_REPLY
https://kernel.org/pub/software/scm/git/docs/git-send-email.html
Be sure your reply has a Subject: header at the top and a blank line
before the message body.
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox