On Thu, Sep 5, 2024 at 12:56 AM Philo Lu <lulie@xxxxxxxxxxxxxxxxx> 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@xxxxxxxxxx>
> Signed-off-by: Philo Lu <lulie@xxxxxxxxxxxxxxxxx>
> ---
> kernel/bpf/btf.c | 13 +++++++++++++
> kernel/bpf/verifier.c | 36 +++++++++++++++++++++++++++++++++---
> 2 files changed, 46 insertions(+), 3 deletions(-)
>
> diff --git a/kernel/bpf/btf.c b/kernel/bpf/btf.c
> index 1e29281653c62..157f5e1247c81 100644
> --- a/kernel/bpf/btf.c
> +++ b/kernel/bpf/btf.c
> @@ -6385,6 +6385,16 @@ 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,
> + const struct btf_param *arg)
> +{
> + if (prog->type != BPF_PROG_TYPE_TRACING ||
> + prog->expected_attach_type != BPF_TRACE_RAW_TP)
> + return false;
> +
> + return btf_param_match_suffix(btf, arg, "__nullable");
why does this need to be BPF_TRACE_RAW_TP-specific logic? Are we
afraid that there might be "some_arg__nullable" argument name?..
> +}
> +
> int btf_ctx_arg_offset(const struct btf *btf, const struct btf_type *func_proto,
> u32 arg_no)
> {
[...]
> @@ -21923,10 +21929,34 @@ int bpf_check_attach_target(struct bpf_verifier_log *log,
> return -EINVAL;
> }
> tname += sizeof(prefix) - 1;
> - t = btf_type_by_id(btf, t->type);
> - if (!btf_type_is_ptr(t))
> - /* should never happen in valid vmlinux build */
> +
> + /* The func_proto of "btf_trace_##tname" is generated from typedef without argument
> + * names. Thus using bpf_raw_event_map to get argument names. For module, the module
> + * name is printed in "%ps" after the template function name, so use strsep to cut
> + * it off.
> + */
> + btp = bpf_get_raw_tracepoint(tname);
> + if (!btp)
> return -EINVAL;
there has to be bpf_put_raw_tracepoint() somewhere below
pw-bot: cr
> + sprintf(trace_symbol, "%ps", btp->bpf_func);
snprintf, but this is really a very round-about way of doing kallsym
lookup, no? Why not call kallsyms_lookup() directly?
> + fname = trace_symbol;
> + fname = strsep(&fname, " ");
> +
> + ret = btf_find_by_name_kind(btf, fname, BTF_KIND_FUNC);
> + if (ret < 0) {
> + bpf_log(log, "Cannot find btf of template %s, fall back to %s%s.\n",
> + fname, prefix, tname);
> + t = btf_type_by_id(btf, t->type);
> + if (!btf_type_is_ptr(t))
> + /* should never happen in valid vmlinux build */
> + return -EINVAL;
> + } else {
> + t = btf_type_by_id(btf, ret);
> + if (!btf_type_is_func(t))
> + /* should never happen in valid vmlinux build */
> + return -EINVAL;
> + }
> +
> t = btf_type_by_id(btf, t->type);
> if (!btf_type_is_func_proto(t))
> /* should never happen in valid vmlinux build */
> --
> 2.32.0.3.g01195cf9f
>
