Skip to content

bpf: check_cond_jmp_op(): properly infer if register is null - #8852

Closed
kernel-patches-daemon-bpf-rc[bot] wants to merge 2 commits into
bpf-next_basefrom
series/1150064=>bpf-next
Closed

bpf: check_cond_jmp_op(): properly infer if register is null#8852
kernel-patches-daemon-bpf-rc[bot] wants to merge 2 commits into
bpf-next_basefrom
series/1150064=>bpf-next

Conversation

@kernel-patches-daemon-bpf-rc

Copy link
Copy Markdown

Pull request for series with
subject: bpf: check_cond_jmp_op(): properly infer if register is null
version: 1
url: https://patchwork.kernel.org/project/netdevbpf/list/?series=1150064

@kernel-patches-daemon-bpf-rc

Copy link
Copy Markdown
Author

Upstream branch: 669e4fa
series: https://patchwork.kernel.org/project/netdevbpf/list/?series=1150064
version: 1

@kernel-patches-daemon-bpf-rc

Copy link
Copy Markdown
Author

Upstream branch: a284ed4
series: https://patchwork.kernel.org/project/netdevbpf/list/?series=1150064
version: 1

@kernel-patches-daemon-bpf-rc

Copy link
Copy Markdown
Author

Upstream branch: 5e289c5
series: https://patchwork.kernel.org/project/netdevbpf/list/?series=1150064
version: 1

@kernel-patches-daemon-bpf-rc

Copy link
Copy Markdown
Author

Upstream branch: 5e289c5
series: https://patchwork.kernel.org/project/netdevbpf/list/?series=1150064
version: 1

Nicholas Carlini reported a bug when verifier can incorrectly infer
that a pointer is non-null. The bug occurs when two pointers are
compared and one of them has a type w/o PTR_MAYBE_NULL flag,
but which allows a value to be NULL at runtime.
Here is an example:

  // `a` is PTR_TO_MEM | MEM_RDONLY | PTR_UNTRUSTED
  // `a` is 0 at runtime.
  // `b` is PTR_TO_MAP_VALUE | PTR_MAYBE_NULL
  void *a = bpf_rdonly_cast(0, 0);
  int  *b = bpf_map_lookup_elem(...);

  if (a == b)
    *b = 42;  // verifier does not catch null pointer dereference

This happens because of a special case in check_cond_jmp_op(),
which attempts to strip PTR_MAYBE_NULL flags from pointer types,
when processing comparisons like `rA == rB`, if either rA or rB can't
be null.

The non-null property is derived based on the absence of
PTR_MAYBE_NULL flag on rA's or rB's type. But that is not sufficient
for types like PTR_TO_MEM, as in the example.

This patch replaces type_may_be_null() call with reg_not_null(),
which contains an allowlist of types for which absence of
PTR_MAYBE_NULL actually means that the value can't be NULL at runtime.

At the moment, the list in the reg_not_null() omits two types for
which PTR_MAYBE_NULL is applicable: PTR_TO_XDP_SOCK and PTR_TO_BUF.
In order to remain backward compatible, and assuming that only
comparison between pointers of the same type makes sense,
this commit extends reg_not_null(). W/o such an extension e.g.
verifier_jeq_infer_not_null/null_ptr_to_map_value fails.

reg_not_null() can be extended further, but I deem that out of scope
for the fix at hand. Explicit base_type(...) != PTR_TO_BTF_ID
checks in the check_cond_jmp_op() can be removed with migration to
reg_not_null(), but that is a behavioural change, as the special case
would start matching for PTR_TO_BTF_ID that is also is_trusted_reg().
I omit the behavioural change from this commit.

Fixes: befae75 ("bpf: propagate nullness information for reg to reg comparisons")
Suggested-by: Nicholas Carlini <npc@anthropic.com>
Signed-off-by: Eduard Zingerman <eddyz87@gmail.com>
A comparison between PTR_TO_MEM | MEM_RDONLY | PTR_UNTRUSTED and
PTR_TO_MAP_VALUE_OR_NULL should not infer that map pointer is not null.
A bug in check_cond_jmp_op() made such inference possible.

Signed-off-by: Eduard Zingerman <eddyz87@gmail.com>
@kernel-patches-daemon-bpf-rc

Copy link
Copy Markdown
Author

At least one diff in series https://patchwork.kernel.org/project/netdevbpf/list/?series=1150064 expired. Closing PR.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant