diff mbox series

[bpf,v2,2/3] bpf: Augment raw_tp arguments with PTR_MAYBE_NULL

Message ID 20241213175127.2084759-3-memxor@gmail.com (mailing list archive)
State Superseded
Delegated to: BPF
Headers show
Series Explicit raw_tp NULL arguments | expand

Checks

Context Check Description
bpf/vmtest-bpf-PR success PR summary
bpf/vmtest-bpf-VM_Test-0 success Logs for Lint
bpf/vmtest-bpf-VM_Test-1 success Logs for ShellCheck
bpf/vmtest-bpf-VM_Test-2 success Logs for Unittests
bpf/vmtest-bpf-VM_Test-3 success Logs for Validate matrix.py
bpf/vmtest-bpf-VM_Test-4 success Logs for aarch64-gcc / build / build for aarch64 with gcc
bpf/vmtest-bpf-VM_Test-5 success Logs for aarch64-gcc / build-release
bpf/vmtest-bpf-VM_Test-10 success Logs for aarch64-gcc / veristat-kernel
bpf/vmtest-bpf-VM_Test-9 success Logs for aarch64-gcc / test (test_verifier, false, 360) / test_verifier on aarch64 with gcc
bpf/vmtest-bpf-VM_Test-11 success Logs for aarch64-gcc / veristat-meta
bpf/vmtest-bpf-VM_Test-12 success Logs for s390x-gcc / build / build for s390x with gcc
bpf/vmtest-bpf-VM_Test-13 success Logs for s390x-gcc / build-release
bpf/vmtest-bpf-VM_Test-16 success Logs for s390x-gcc / test (test_verifier, false, 360) / test_verifier on s390x with gcc
bpf/vmtest-bpf-VM_Test-17 success Logs for s390x-gcc / veristat-kernel
bpf/vmtest-bpf-VM_Test-18 success Logs for s390x-gcc / veristat-meta
bpf/vmtest-bpf-VM_Test-19 success Logs for set-matrix
bpf/vmtest-bpf-VM_Test-20 success Logs for x86_64-gcc / build / build for x86_64 with gcc
bpf/vmtest-bpf-VM_Test-21 success Logs for x86_64-gcc / build-release
bpf/vmtest-bpf-VM_Test-27 success Logs for x86_64-gcc / test (test_verifier, false, 360) / test_verifier on x86_64 with gcc
bpf/vmtest-bpf-VM_Test-30 success Logs for x86_64-llvm-17 / build / build for x86_64 with llvm-17
bpf/vmtest-bpf-VM_Test-36 success Logs for x86_64-llvm-17 / veristat-kernel
bpf/vmtest-bpf-VM_Test-31 success Logs for x86_64-llvm-17 / build-release / build for x86_64 with llvm-17-O2
bpf/vmtest-bpf-VM_Test-37 success Logs for x86_64-llvm-17 / veristat-meta
bpf/vmtest-bpf-VM_Test-38 success Logs for x86_64-llvm-18 / build / build for x86_64 with llvm-18
bpf/vmtest-bpf-VM_Test-44 success Logs for x86_64-llvm-18 / test (test_verifier, false, 360) / test_verifier on x86_64 with llvm-18
bpf/vmtest-bpf-VM_Test-45 success Logs for x86_64-llvm-18 / veristat-kernel
bpf/vmtest-bpf-VM_Test-46 success Logs for x86_64-llvm-18 / veristat-meta
bpf/vmtest-bpf-VM_Test-6 success Logs for aarch64-gcc / test (test_maps, false, 360) / test_maps on aarch64 with gcc
bpf/vmtest-bpf-VM_Test-22 success Logs for x86_64-gcc / test (test_maps, false, 360) / test_maps on x86_64 with gcc
bpf/vmtest-bpf-VM_Test-23 success Logs for x86_64-gcc / test (test_progs, false, 360) / test_progs on x86_64 with gcc
bpf/vmtest-bpf-VM_Test-24 success Logs for x86_64-gcc / test (test_progs_no_alu32, false, 360) / test_progs_no_alu32 on x86_64 with gcc
bpf/vmtest-bpf-VM_Test-25 success Logs for x86_64-gcc / test (test_progs_no_alu32_parallel, true, 30) / test_progs_no_alu32_parallel on x86_64 with gcc
bpf/vmtest-bpf-VM_Test-26 success Logs for x86_64-gcc / test (test_progs_parallel, true, 30) / test_progs_parallel on x86_64 with gcc
bpf/vmtest-bpf-VM_Test-28 success Logs for x86_64-gcc / veristat-kernel / x86_64-gcc veristat_kernel
bpf/vmtest-bpf-VM_Test-29 success Logs for x86_64-gcc / veristat-meta / x86_64-gcc veristat_meta
bpf/vmtest-bpf-VM_Test-32 success Logs for x86_64-llvm-17 / test (test_maps, false, 360) / test_maps on x86_64 with llvm-17
bpf/vmtest-bpf-VM_Test-33 success Logs for x86_64-llvm-17 / test (test_progs, false, 360) / test_progs on x86_64 with llvm-17
bpf/vmtest-bpf-VM_Test-34 success Logs for x86_64-llvm-17 / test (test_progs_no_alu32, false, 360) / test_progs_no_alu32 on x86_64 with llvm-17
bpf/vmtest-bpf-VM_Test-35 success Logs for x86_64-llvm-17 / test (test_verifier, false, 360) / test_verifier on x86_64 with llvm-17
bpf/vmtest-bpf-VM_Test-39 success Logs for x86_64-llvm-18 / build-release / build for x86_64 with llvm-18-O2
bpf/vmtest-bpf-VM_Test-41 success Logs for x86_64-llvm-18 / test (test_progs, false, 360) / test_progs on x86_64 with llvm-18
bpf/vmtest-bpf-VM_Test-40 success Logs for x86_64-llvm-18 / test (test_maps, false, 360) / test_maps on x86_64 with llvm-18
bpf/vmtest-bpf-VM_Test-42 success Logs for x86_64-llvm-18 / test (test_progs_cpuv4, false, 360) / test_progs_cpuv4 on x86_64 with llvm-18
bpf/vmtest-bpf-VM_Test-43 success Logs for x86_64-llvm-18 / test (test_progs_no_alu32, false, 360) / test_progs_no_alu32 on x86_64 with llvm-18
bpf/vmtest-bpf-VM_Test-7 success Logs for aarch64-gcc / test (test_progs, false, 360) / test_progs on aarch64 with gcc
bpf/vmtest-bpf-VM_Test-8 success Logs for aarch64-gcc / test (test_progs_no_alu32, false, 360) / test_progs_no_alu32 on aarch64 with gcc
bpf/vmtest-bpf-VM_Test-14 success Logs for s390x-gcc / test (test_progs, false, 360) / test_progs on s390x with gcc
bpf/vmtest-bpf-VM_Test-15 success Logs for s390x-gcc / test (test_progs_no_alu32, false, 360) / test_progs_no_alu32 on s390x with gcc

Commit Message

Kumar Kartikeya Dwivedi Dec. 13, 2024, 5:51 p.m. UTC 
Arguments to a raw tracepoint are tagged as trusted, which carries the
semantics that the pointer will be non-NULL.  However, in certain cases,
a raw tracepoint argument may end up being NULL. More context about this
issue is available in [0].

Thus, there is a discrepancy between the reality, that raw_tp arguments can
actually be NULL, and the verifier's knowledge, that they are never NULL,
causing explicit NULL check branch to be dead code eliminated.

A previous attempt [1], i.e. the second fixed commit, was made to
simulate symbolic execution as if in most accesses, the argument is a
non-NULL raw_tp, except for conditional jumps.  This tried to suppress
branch prediction while preserving compatibility, but surfaced issues
with production programs that were difficult to solve without increasing
verifier complexity. A more complete discussion of issues and fixes is
available at [2].

Fix this by maintaining an explicit list of tracepoints where the
arguments are known to be NULL, and mark the positional arguments as
PTR_MAYBE_NULL. Additionally, capture the tracepoints where arguments
are known to be ERR_PTR, and mark these arguments as scalar values to
prevent potential dereference.

Each hex digit is used to encode NULL-ness (0x1) or ERR_PTR-ness (0x2),
shifted by the zero-indexed argument number x 4. This can be represented
as follows:
1st arg: 0x1
2nd arg: 0x10
3rd arg: 0x100
... and so on (likewise for ERR_PTR case).

In the future, an automated pass will be used to produce such a list, or
insert __nullable annotations automatically for tracepoints. Each
compilation unit will be analyzed and results will be collated to find
whether a tracepoint pointer is definitely not null, maybe null, or an
unknown state where verifier conservatively marks it PTR_MAYBE_NULL.
A proof of concept of this tool from Eduard is available at [3].

Note that in case we don't find a specification in the raw_tp_null_args
array and the tracepoint belongs to a kernel module, we will
conservatively mark the arguments as PTR_MAYBE_NULL. This is because
unlike for in-tree modules, out-of-tree module tracepoints may pass NULL
freely to the tracepoint. We don't protect against such tracepoints
passing ERR_PTR (which is uncommon anyway), lest we mark all such
arguments as SCALAR_VALUE.

While we are it, let's adjust the test raw_tp_null to not perform
dereference of the skb->mark, as that won't be allowed anymore, and make
it more robust by using inline assembly to test the dead code
elimination behavior, which should still stay the same.

  [0]: https://lore.kernel.org/bpf/ZrCZS6nisraEqehw@jlelli-thinkpadt14gen4.remote.csb
  [1]: https://lore.kernel.org/all/20241104171959.2938862-1-memxor@gmail.com
  [2]: https://lore.kernel.org/bpf/20241206161053.809580-1-memxor@gmail.com
  [3]: https://github.com/eddyz87/llvm-project/tree/nullness-for-tracepoint-params

Reported-by: Juri Lelli <juri.lelli@redhat.com> # original bug
Reported-by: Manu Bretelle <chantra@meta.com> # bugs in masking fix
Fixes: 3f00c5239344 ("bpf: Allow trusted pointers to be passed to KF_TRUSTED_ARGS kfuncs")
Fixes: cb4158ce8ec8 ("bpf: Mark raw_tp arguments with PTR_MAYBE_NULL")
Co-developed-by: Jiri Olsa <jolsa@kernel.org>
Signed-off-by: Jiri Olsa <jolsa@kernel.org>
Signed-off-by: Kumar Kartikeya Dwivedi <memxor@gmail.com>
---
 kernel/bpf/btf.c                              | 136 ++++++++++++++++++
 .../testing/selftests/bpf/progs/raw_tp_null.c |  19 ++-
 2 files changed, 145 insertions(+), 10 deletions(-)

Comments

Eduard Zingerman Dec. 13, 2024, 8:05 p.m. UTC | #1 
On Fri, 2024-12-13 at 09:51 -0800, Kumar Kartikeya Dwivedi wrote:
> Arguments to a raw tracepoint are tagged as trusted, which carries the
> semantics that the pointer will be non-NULL.  However, in certain cases,
> a raw tracepoint argument may end up being NULL. More context about this
> issue is available in [0].
> 
> Thus, there is a discrepancy between the reality, that raw_tp arguments can
> actually be NULL, and the verifier's knowledge, that they are never NULL,
> causing explicit NULL check branch to be dead code eliminated.
> 
> A previous attempt [1], i.e. the second fixed commit, was made to
> simulate symbolic execution as if in most accesses, the argument is a
> non-NULL raw_tp, except for conditional jumps.  This tried to suppress
> branch prediction while preserving compatibility, but surfaced issues
> with production programs that were difficult to solve without increasing
> verifier complexity. A more complete discussion of issues and fixes is
> available at [2].
> 
> Fix this by maintaining an explicit list of tracepoints where the
> arguments are known to be NULL, and mark the positional arguments as
> PTR_MAYBE_NULL. Additionally, capture the tracepoints where arguments
> are known to be ERR_PTR, and mark these arguments as scalar values to
> prevent potential dereference.
> 
> Each hex digit is used to encode NULL-ness (0x1) or ERR_PTR-ness (0x2),
> shifted by the zero-indexed argument number x 4. This can be represented
> as follows:
> 1st arg: 0x1
> 2nd arg: 0x10
> 3rd arg: 0x100
> ... and so on (likewise for ERR_PTR case).
> 
> In the future, an automated pass will be used to produce such a list, or
> insert __nullable annotations automatically for tracepoints. Each
> compilation unit will be analyzed and results will be collated to find
> whether a tracepoint pointer is definitely not null, maybe null, or an
> unknown state where verifier conservatively marks it PTR_MAYBE_NULL.
> A proof of concept of this tool from Eduard is available at [3].
> 
> Note that in case we don't find a specification in the raw_tp_null_args
> array and the tracepoint belongs to a kernel module, we will
> conservatively mark the arguments as PTR_MAYBE_NULL. This is because
> unlike for in-tree modules, out-of-tree module tracepoints may pass NULL
> freely to the tracepoint. We don't protect against such tracepoints
> passing ERR_PTR (which is uncommon anyway), lest we mark all such
> arguments as SCALAR_VALUE.
> 
> While we are it, let's adjust the test raw_tp_null to not perform
> dereference of the skb->mark, as that won't be allowed anymore, and make
> it more robust by using inline assembly to test the dead code
> elimination behavior, which should still stay the same.
> 
>   [0]: https://lore.kernel.org/bpf/ZrCZS6nisraEqehw@jlelli-thinkpadt14gen4.remote.csb
>   [1]: https://lore.kernel.org/all/20241104171959.2938862-1-memxor@gmail.com
>   [2]: https://lore.kernel.org/bpf/20241206161053.809580-1-memxor@gmail.com
>   [3]: https://github.com/eddyz87/llvm-project/tree/nullness-for-tracepoint-params
> 
> Reported-by: Juri Lelli <juri.lelli@redhat.com> # original bug
> Reported-by: Manu Bretelle <chantra@meta.com> # bugs in masking fix
> Fixes: 3f00c5239344 ("bpf: Allow trusted pointers to be passed to KF_TRUSTED_ARGS kfuncs")
> Fixes: cb4158ce8ec8 ("bpf: Mark raw_tp arguments with PTR_MAYBE_NULL")
> Co-developed-by: Jiri Olsa <jolsa@kernel.org>
> Signed-off-by: Jiri Olsa <jolsa@kernel.org>
> Signed-off-by: Kumar Kartikeya Dwivedi <memxor@gmail.com>
> ---

Tbh, I think we should have fixed the bug in what is currently in the
tree and avoid revert. Anyways, the code looks good to me.

Reviewed-by: Eduard Zingerman <eddyz87@gmail.com>

[...]

> @@ -6597,6 +6693,39 @@ bool btf_ctx_access(int off, int size, enum bpf_access_type type,
>  	if (btf_param_match_suffix(btf, &args[arg], "__nullable"))
>  		info->reg_type |= PTR_MAYBE_NULL;
>  
> +	if (prog->expected_attach_type == BPF_TRACE_RAW_TP) {
> +		struct btf *btf = prog->aux->attach_btf;
> +		const struct btf_type *t;
> +		const char *tname;
> +
> +		/* BTF lookups cannot fail, return false on error */
> +		t = btf_type_by_id(btf, prog->aux->attach_btf_id);
> +		if (!t)
> +			return false;
> +		tname = btf_name_by_offset(btf, t->name_off);
> +		if (!tname)
> +			return false;
> +		/* Checked by bpf_check_attach_target */
> +		tname += sizeof("bpf_trace_") - 1;

Nit: bpf_check_attach_target uses "btf_trace_" prefix.

> +		for (i = 0; i < ARRAY_SIZE(raw_tp_null_args); i++) {
> +			/* Is this a func with potential NULL args? */
> +			if (strcmp(tname, raw_tp_null_args[i].func))
> +				continue;
> +			if (raw_tp_null_args[i].mask & (0x1 << (arg * 4)))
> +				info->reg_type |= PTR_MAYBE_NULL;
> +			/* Is the current arg IS_ERR? */
> +			if (raw_tp_null_args[i].mask & (0x2 << (arg * 4)))
> +				ptr_err_raw_tp = true;
> +			break;
> +		}
> +		/* If we don't know NULL-ness specification and the tracepoint
> +		 * is coming from a loadable module, be conservative and mark
> +		 * argument as PTR_MAYBE_NULL.
> +		 */
> +		if (i == ARRAY_SIZE(raw_tp_null_args) && btf_is_module(btf))
> +			info->reg_type |= PTR_MAYBE_NULL;
> +	}
> +
>  	if (tgt_prog) {
>  		enum bpf_prog_type tgt_type;
>  
> @@ -6641,6 +6770,13 @@ bool btf_ctx_access(int off, int size, enum bpf_access_type type,
>  	bpf_log(log, "func '%s' arg%d has btf_id %d type %s '%s'\n",
>  		tname, arg, info->btf_id, btf_type_str(t),
>  		__btf_name_by_offset(btf, t->name_off));
> +
> +	/* Perform all checks on the validity of type for this argument, but if
> +	 * we know it can be IS_ERR at runtime, scrub pointer type and mark as
> +	 * scalar.
> +	 */
> +	if (ptr_err_raw_tp)
> +		info->reg_type = SCALAR_VALUE;

Nit: the log line above would be a bit confusing if 'ptr_err_raw_tp' would be true.
     maybe add an additional line here, saying that verifier overrides BTF type?

>  	return true;
>  }
>  EXPORT_SYMBOL_GPL(btf_ctx_access);

[...]
Kumar Kartikeya Dwivedi Dec. 13, 2024, 10:19 p.m. UTC | #2 
On Fri, 13 Dec 2024 at 21:06, Eduard Zingerman <eddyz87@gmail.com> wrote:
>
> On Fri, 2024-12-13 at 09:51 -0800, Kumar Kartikeya Dwivedi wrote:
> > Arguments to a raw tracepoint are tagged as trusted, which carries the
> > semantics that the pointer will be non-NULL.  However, in certain cases,
> > a raw tracepoint argument may end up being NULL. More context about this
> > issue is available in [0].
> >
> > Thus, there is a discrepancy between the reality, that raw_tp arguments can
> > actually be NULL, and the verifier's knowledge, that they are never NULL,
> > causing explicit NULL check branch to be dead code eliminated.
> >
> > A previous attempt [1], i.e. the second fixed commit, was made to
> > simulate symbolic execution as if in most accesses, the argument is a
> > non-NULL raw_tp, except for conditional jumps.  This tried to suppress
> > branch prediction while preserving compatibility, but surfaced issues
> > with production programs that were difficult to solve without increasing
> > verifier complexity. A more complete discussion of issues and fixes is
> > available at [2].
> >
> > Fix this by maintaining an explicit list of tracepoints where the
> > arguments are known to be NULL, and mark the positional arguments as
> > PTR_MAYBE_NULL. Additionally, capture the tracepoints where arguments
> > are known to be ERR_PTR, and mark these arguments as scalar values to
> > prevent potential dereference.
> >
> > Each hex digit is used to encode NULL-ness (0x1) or ERR_PTR-ness (0x2),
> > shifted by the zero-indexed argument number x 4. This can be represented
> > as follows:
> > 1st arg: 0x1
> > 2nd arg: 0x10
> > 3rd arg: 0x100
> > ... and so on (likewise for ERR_PTR case).
> >
> > In the future, an automated pass will be used to produce such a list, or
> > insert __nullable annotations automatically for tracepoints. Each
> > compilation unit will be analyzed and results will be collated to find
> > whether a tracepoint pointer is definitely not null, maybe null, or an
> > unknown state where verifier conservatively marks it PTR_MAYBE_NULL.
> > A proof of concept of this tool from Eduard is available at [3].
> >
> > Note that in case we don't find a specification in the raw_tp_null_args
> > array and the tracepoint belongs to a kernel module, we will
> > conservatively mark the arguments as PTR_MAYBE_NULL. This is because
> > unlike for in-tree modules, out-of-tree module tracepoints may pass NULL
> > freely to the tracepoint. We don't protect against such tracepoints
> > passing ERR_PTR (which is uncommon anyway), lest we mark all such
> > arguments as SCALAR_VALUE.
> >
> > While we are it, let's adjust the test raw_tp_null to not perform
> > dereference of the skb->mark, as that won't be allowed anymore, and make
> > it more robust by using inline assembly to test the dead code
> > elimination behavior, which should still stay the same.
> >
> >   [0]: https://lore.kernel.org/bpf/ZrCZS6nisraEqehw@jlelli-thinkpadt14gen4.remote.csb
> >   [1]: https://lore.kernel.org/all/20241104171959.2938862-1-memxor@gmail.com
> >   [2]: https://lore.kernel.org/bpf/20241206161053.809580-1-memxor@gmail.com
> >   [3]: https://github.com/eddyz87/llvm-project/tree/nullness-for-tracepoint-params
> >
> > Reported-by: Juri Lelli <juri.lelli@redhat.com> # original bug
> > Reported-by: Manu Bretelle <chantra@meta.com> # bugs in masking fix
> > Fixes: 3f00c5239344 ("bpf: Allow trusted pointers to be passed to KF_TRUSTED_ARGS kfuncs")
> > Fixes: cb4158ce8ec8 ("bpf: Mark raw_tp arguments with PTR_MAYBE_NULL")
> > Co-developed-by: Jiri Olsa <jolsa@kernel.org>
> > Signed-off-by: Jiri Olsa <jolsa@kernel.org>
> > Signed-off-by: Kumar Kartikeya Dwivedi <memxor@gmail.com>
> > ---
>
> Tbh, I think we should have fixed the bug in what is currently in the
> tree and avoid revert. Anyways, the code looks good to me.
>
> Reviewed-by: Eduard Zingerman <eddyz87@gmail.com>
>

Thanks for the review, I have addressed the nits and resent v3.

> [...]
>
> > @@ -6597,6 +6693,39 @@ bool btf_ctx_access(int off, int size, enum bpf_access_type type,
> >       if (btf_param_match_suffix(btf, &args[arg], "__nullable"))
> >               info->reg_type |= PTR_MAYBE_NULL;
> >
> > +     if (prog->expected_attach_type == BPF_TRACE_RAW_TP) {
> > +             struct btf *btf = prog->aux->attach_btf;
> > +             const struct btf_type *t;
> > +             const char *tname;
> > +
> > +             /* BTF lookups cannot fail, return false on error */
> > +             t = btf_type_by_id(btf, prog->aux->attach_btf_id);
> > +             if (!t)
> > +                     return false;
> > +             tname = btf_name_by_offset(btf, t->name_off);
> > +             if (!tname)
> > +                     return false;
> > +             /* Checked by bpf_check_attach_target */
> > +             tname += sizeof("bpf_trace_") - 1;
>
> Nit: bpf_check_attach_target uses "btf_trace_" prefix.
>
> > +             for (i = 0; i < ARRAY_SIZE(raw_tp_null_args); i++) {
> > +                     /* Is this a func with potential NULL args? */
> > +                     if (strcmp(tname, raw_tp_null_args[i].func))
> > +                             continue;
> > +                     if (raw_tp_null_args[i].mask & (0x1 << (arg * 4)))
> > +                             info->reg_type |= PTR_MAYBE_NULL;
> > +                     /* Is the current arg IS_ERR? */
> > +                     if (raw_tp_null_args[i].mask & (0x2 << (arg * 4)))
> > +                             ptr_err_raw_tp = true;
> > +                     break;
> > +             }
> > +             /* If we don't know NULL-ness specification and the tracepoint
> > +              * is coming from a loadable module, be conservative and mark
> > +              * argument as PTR_MAYBE_NULL.
> > +              */
> > +             if (i == ARRAY_SIZE(raw_tp_null_args) && btf_is_module(btf))
> > +                     info->reg_type |= PTR_MAYBE_NULL;
> > +     }
> > +
> >       if (tgt_prog) {
> >               enum bpf_prog_type tgt_type;
> >
> > @@ -6641,6 +6770,13 @@ bool btf_ctx_access(int off, int size, enum bpf_access_type type,
> >       bpf_log(log, "func '%s' arg%d has btf_id %d type %s '%s'\n",
> >               tname, arg, info->btf_id, btf_type_str(t),
> >               __btf_name_by_offset(btf, t->name_off));
> > +
> > +     /* Perform all checks on the validity of type for this argument, but if
> > +      * we know it can be IS_ERR at runtime, scrub pointer type and mark as
> > +      * scalar.
> > +      */
> > +     if (ptr_err_raw_tp)
> > +             info->reg_type = SCALAR_VALUE;
>
> Nit: the log line above would be a bit confusing if 'ptr_err_raw_tp' would be true.
>      maybe add an additional line here, saying that verifier overrides BTF type?
>
> >       return true;
> >  }
> >  EXPORT_SYMBOL_GPL(btf_ctx_access);
>
> [...]
>
diff mbox series

Patch

diff --git a/kernel/bpf/btf.c b/kernel/bpf/btf.c
index c4aa304028ce..999423077de4 100644
--- a/kernel/bpf/btf.c
+++ b/kernel/bpf/btf.c
@@ -6439,6 +6439,101 @@  int btf_ctx_arg_offset(const struct btf *btf, const struct btf_type *func_proto,
 	return off;
 }
 
+struct bpf_raw_tp_null_args {
+	const char *func;
+	u64 mask;
+};
+
+static const struct bpf_raw_tp_null_args raw_tp_null_args[] = {
+	/* sched */
+	{ "sched_pi_setprio", 0x10 },
+	/* ... from sched_numa_pair_template event class */
+	{ "sched_stick_numa", 0x100 },
+	{ "sched_swap_numa", 0x100 },
+	/* afs */
+	{ "afs_make_fs_call", 0x10 },
+	{ "afs_make_fs_calli", 0x10 },
+	{ "afs_make_fs_call1", 0x10 },
+	{ "afs_make_fs_call2", 0x10 },
+	{ "afs_protocol_error", 0x1 },
+	{ "afs_flock_ev", 0x10 },
+	/* cachefiles */
+	{ "cachefiles_lookup", 0x1 | 0x200 },
+	{ "cachefiles_unlink", 0x1 },
+	{ "cachefiles_rename", 0x1 },
+	{ "cachefiles_prep_read", 0x1 },
+	{ "cachefiles_mark_active", 0x1 },
+	{ "cachefiles_mark_failed", 0x1 },
+	{ "cachefiles_mark_inactive", 0x1 },
+	{ "cachefiles_vfs_error", 0x1 },
+	{ "cachefiles_io_error", 0x1 },
+	{ "cachefiles_ondemand_open", 0x1 },
+	{ "cachefiles_ondemand_copen", 0x1 },
+	{ "cachefiles_ondemand_close", 0x1 },
+	{ "cachefiles_ondemand_read", 0x1 },
+	{ "cachefiles_ondemand_cread", 0x1 },
+	{ "cachefiles_ondemand_fd_write", 0x1 },
+	{ "cachefiles_ondemand_fd_release", 0x1 },
+	/* ext4, from ext4__mballoc event class */
+	{ "ext4_mballoc_discard", 0x10 },
+	{ "ext4_mballoc_free", 0x10 },
+	/* fib */
+	{ "fib_table_lookup", 0x100 },
+	/* filelock */
+	/* ... from filelock_lock event class */
+	{ "posix_lock_inode", 0x10 },
+	{ "fcntl_setlk", 0x10 },
+	{ "locks_remove_posix", 0x10 },
+	{ "flock_lock_inode", 0x10 },
+	/* ... from filelock_lease event class */
+	{ "break_lease_noblock", 0x10 },
+	{ "break_lease_block", 0x10 },
+	{ "break_lease_unblock", 0x10 },
+	{ "generic_delete_lease", 0x10 },
+	{ "time_out_leases", 0x10 },
+	/* host1x */
+	{ "host1x_cdma_push_gather", 0x10000 },
+	/* huge_memory */
+	{ "mm_khugepaged_scan_pmd", 0x10 },
+	{ "mm_collapse_huge_page_isolate", 0x1 },
+	{ "mm_khugepaged_scan_file", 0x10 },
+	{ "mm_khugepaged_collapse_file", 0x10 },
+	/* kmem */
+	{ "mm_page_alloc", 0x1 },
+	{ "mm_page_pcpu_drain", 0x1 },
+	/* .. from mm_page event class */
+	{ "mm_page_alloc_zone_locked", 0x1 },
+	/* netfs */
+	{ "netfs_failure", 0x10 },
+	/* power */
+	{ "device_pm_callback_start", 0x10 },
+	/* qdisc */
+	{ "qdisc_dequeue", 0x1000 },
+	/* rxrpc */
+	{ "rxrpc_recvdata", 0x1 },
+	{ "rxrpc_resend", 0x10 },
+	/* sunrpc */
+	{ "xs_stream_read_data", 0x1 },
+	/* ... from xprt_cong_event event class */
+	{ "xprt_reserve_cong", 0x10 },
+	{ "xprt_release_cong", 0x10 },
+	{ "xprt_get_cong", 0x10 },
+	{ "xprt_put_cong", 0x10 },
+	/* tcp */
+	{ "tcp_send_reset", 0x11 },
+	/* tegra_apb_dma */
+	{ "tegra_dma_tx_status", 0x100 },
+	/* timer_migration */
+	{ "tmigr_update_events", 0x1 },
+	/* writeback, from writeback_folio_template event class */
+	{ "writeback_dirty_folio", 0x10 },
+	{ "folio_wait_writeback", 0x10 },
+	/* rdma */
+	{ "mr_integ_alloc", 0x2000 },
+	/* bpf_testmod */
+	{ "bpf_testmod_test_read", 0x0 },
+};
+
 bool btf_ctx_access(int off, int size, enum bpf_access_type type,
 		    const struct bpf_prog *prog,
 		    struct bpf_insn_access_aux *info)
@@ -6449,6 +6544,7 @@  bool btf_ctx_access(int off, int size, enum bpf_access_type type,
 	const char *tname = prog->aux->attach_func_name;
 	struct bpf_verifier_log *log = info->log;
 	const struct btf_param *args;
+	bool ptr_err_raw_tp = false;
 	const char *tag_value;
 	u32 nr_args, arg;
 	int i, ret;
@@ -6597,6 +6693,39 @@  bool btf_ctx_access(int off, int size, enum bpf_access_type type,
 	if (btf_param_match_suffix(btf, &args[arg], "__nullable"))
 		info->reg_type |= PTR_MAYBE_NULL;
 
+	if (prog->expected_attach_type == BPF_TRACE_RAW_TP) {
+		struct btf *btf = prog->aux->attach_btf;
+		const struct btf_type *t;
+		const char *tname;
+
+		/* BTF lookups cannot fail, return false on error */
+		t = btf_type_by_id(btf, prog->aux->attach_btf_id);
+		if (!t)
+			return false;
+		tname = btf_name_by_offset(btf, t->name_off);
+		if (!tname)
+			return false;
+		/* Checked by bpf_check_attach_target */
+		tname += sizeof("bpf_trace_") - 1;
+		for (i = 0; i < ARRAY_SIZE(raw_tp_null_args); i++) {
+			/* Is this a func with potential NULL args? */
+			if (strcmp(tname, raw_tp_null_args[i].func))
+				continue;
+			if (raw_tp_null_args[i].mask & (0x1 << (arg * 4)))
+				info->reg_type |= PTR_MAYBE_NULL;
+			/* Is the current arg IS_ERR? */
+			if (raw_tp_null_args[i].mask & (0x2 << (arg * 4)))
+				ptr_err_raw_tp = true;
+			break;
+		}
+		/* If we don't know NULL-ness specification and the tracepoint
+		 * is coming from a loadable module, be conservative and mark
+		 * argument as PTR_MAYBE_NULL.
+		 */
+		if (i == ARRAY_SIZE(raw_tp_null_args) && btf_is_module(btf))
+			info->reg_type |= PTR_MAYBE_NULL;
+	}
+
 	if (tgt_prog) {
 		enum bpf_prog_type tgt_type;
 
@@ -6641,6 +6770,13 @@  bool btf_ctx_access(int off, int size, enum bpf_access_type type,
 	bpf_log(log, "func '%s' arg%d has btf_id %d type %s '%s'\n",
 		tname, arg, info->btf_id, btf_type_str(t),
 		__btf_name_by_offset(btf, t->name_off));
+
+	/* Perform all checks on the validity of type for this argument, but if
+	 * we know it can be IS_ERR at runtime, scrub pointer type and mark as
+	 * scalar.
+	 */
+	if (ptr_err_raw_tp)
+		info->reg_type = SCALAR_VALUE;
 	return true;
 }
 EXPORT_SYMBOL_GPL(btf_ctx_access);
diff --git a/tools/testing/selftests/bpf/progs/raw_tp_null.c b/tools/testing/selftests/bpf/progs/raw_tp_null.c
index 457f34c151e3..5927054b6dd9 100644
--- a/tools/testing/selftests/bpf/progs/raw_tp_null.c
+++ b/tools/testing/selftests/bpf/progs/raw_tp_null.c
@@ -3,6 +3,7 @@ 
 
 #include <vmlinux.h>
 #include <bpf/bpf_tracing.h>
+#include "bpf_misc.h"
 
 char _license[] SEC("license") = "GPL";
 
@@ -17,16 +18,14 @@  int BPF_PROG(test_raw_tp_null, struct sk_buff *skb)
 	if (task->pid != tid)
 		return 0;
 
-	i = i + skb->mark + 1;
-	/* The compiler may move the NULL check before this deref, which causes
-	 * the load to fail as deref of scalar. Prevent that by using a barrier.
+	/* If dead code elimination kicks in, the increment +=2 will be
+	 * removed. For raw_tp programs attaching to tracepoints in kernel
+	 * modules, we mark input arguments as PTR_MAYBE_NULL, so branch
+	 * prediction should never kick in.
 	 */
-	barrier();
-	/* If dead code elimination kicks in, the increment below will
-	 * be removed. For raw_tp programs, we mark input arguments as
-	 * PTR_MAYBE_NULL, so branch prediction should never kick in.
-	 */
-	if (!skb)
-		i += 2;
+	asm volatile ("%[i] += 1; if %[ctx] != 0 goto +1; %[i] += 2;"
+			: [i]"+r"(i)
+			: [ctx]"r"(skb)
+			: "memory");
 	return 0;
 }