Commit cb4158ce8ec8 ("bpf: Mark raw_tp arguments with PTR_MAYBE_NULL")
began marking raw_tp arguments as PTR_MAYBE_NULL, which caused the
values of these pointers in the branch where they are seen to be NULL to
be marked as scalar zero.
In comparison, raw_tp using programs which did the NULL check never
explored the NULL branch, hence successor instructions following the
NULL check always saw a non-NULL raw_tp pointer and performed memory
accesses on it.
To preserve compatibility, we need to leave a NULL raw_tp arg as is,
such that it can be allowed to be dereferenced. Otherwise, the verifer
will notice the dereference of a zero scalar in the path following the
NULL branch and reject existing programs.
This can allow programs that do bogus things with a NULL pointer to be
able to access memory (with PROBE_MEM) correctly, for instance a program
only having:
if (!skb) __builtin_memcpy(dst, &skb->mark, sizeof(skb->mark));
However, such programs would also not fail on earlier kernels, since the
verifier would simply never explore this branch. Now, it will permit
behavior where such memory is accessed and explore the branch.
The correct way to fix this is to simply introduce the right annotations
per-tracepoint, and remove the masking/unmasking hack, until then the
raw_tp stop-gap allows programs to continue passing without deleting
code that at runtime can cause safety violations.
An implication of this fix, which follows from the way the raw_tp fixes
were implemented, is that all PTR_MAYBE_NULL trusted PTR_TO_BTF_ID are
engulfed by these checks, and PROBE_MEM will apply to all of them, incl.
those coming from helpers with KF_ACQUIRE returning maybe null trusted
pointers. This NULL tagging after this commit will be sticky. Compared
to a solution which only specially tagged raw_tp args with a different
special maybe null tag (like PTR_SOFT_NULL), it's a consequence of
overloading PTR_MAYBE_NULL with this meaning.
Fixes: cb4158ce8ec8 ("bpf: Mark raw_tp arguments with PTR_MAYBE_NULL")
Reported-by: Manu Bretelle <chantra@xxxxxxxx>
Signed-off-by: Kumar Kartikeya Dwivedi <memxor@xxxxxxxxx>
---
kernel/bpf/verifier.c | 6 ++++++
1 file changed, 6 insertions(+)
diff --git a/kernel/bpf/verifier.c b/kernel/bpf/verifier.c
index 82f40d63ad7b..556fb609d4a4 100644
--- a/kernel/bpf/verifier.c
+++ b/kernel/bpf/verifier.c
@@ -15365,6 +15365,12 @@ static void mark_ptr_or_null_reg(struct bpf_verifier_env *env,
return;
if (is_null) {
+ /* We never mark a raw_tp trusted pointer as scalar, to
+ * preserve backwards compatibility, instead just leave
+ * it as is.
+ */
+ if (mask_raw_tp_reg_cond(env, reg))
+ return;
reg->type = SCALAR_VALUE;
/* We don't need id and ref_obj_id from this point
* onwards anymore, thus we should better reset it,
--
2.43.5
