Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Jump to bottom

Handle possible NULL trusted raw_tp arguments #4596

Closed
wants to merge 4 commits into from
Closed

Handle possible NULL trusted raw_tp arguments #4596

wants to merge 4 commits into from

Conversation

kernel-patches-daemon-bpf-rc[bot]
Copy link

Pull request for series with
subject: Handle possible NULL trusted raw_tp arguments
version: 1
url: https://patchwork.kernel.org/project/netdevbpf/list/?series=905194

@kernel-patches-daemon-bpf-rc
Copy link
Author

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

@kernel-patches-daemon-bpf-rc
Copy link
Author

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

@kernel-patches-daemon-bpf-rc
Copy link
Author

Upstream branch: 4d99e50
series: https://patchwork.kernel.org/project/netdevbpf/list/?series=905194
version: 1

@kernel-patches-daemon-bpf-rc
Copy link
Author

Upstream branch: 77017b9
series: https://patchwork.kernel.org/project/netdevbpf/list/?series=905194
version: 1

@kernel-patches-daemon-bpf-rc
Copy link
Author

Upstream branch: 77017b9
series: https://patchwork.kernel.org/project/netdevbpf/list/?series=905792
version: 2

@kernel-patches-daemon-bpf-rc
Copy link
Author

Upstream branch: f2daa5a
series: https://patchwork.kernel.org/project/netdevbpf/list/?series=905792
version: 2

@kernel-patches-daemon-bpf-rc
Copy link
Author

Upstream branch: f2daa5a
series: https://patchwork.kernel.org/project/netdevbpf/list/?series=906140
version: 3

@kkdwivedi
bpf: Mark raw_tp arguments with PTR_MAYBE_NULL
d018e9c 
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 checks to be deleted, and accesses to such
pointers potentially crashing the kernel.

To fix this, mark raw_tp arguments as PTR_MAYBE_NULL, and then special
case the dereference and pointer arithmetic to permit it, and allow
passing them into helpers/kfuncs; these exceptions are made for raw_tp
programs only. Ensure that we don't do this when ref_obj_id > 0, as in
that case this is an acquired object and doesn't need such adjustment.

The reason we do mask_raw_tp_trusted_reg logic is because other will
recheck in places whether the register is a trusted_reg, and then
consider our register as untrusted when detecting the presence of the
PTR_MAYBE_NULL flag.

To allow safe dereference, we enable PROBE_MEM marking when we see loads
into trusted pointers with PTR_MAYBE_NULL.

While trusted raw_tp arguments can also be passed into helpers or kfuncs
where such broken assumption may cause issues, a future patch set will
tackle their case separately, as PTR_TO_BTF_ID (without PTR_TRUSTED) can
already be passed into helpers and causes similar problems. Thus, they
are left alone for now.

It is possible that these checks also permit passing non-raw_tp args
that are trusted PTR_TO_BTF_ID with null marking. In such a case,
allowing dereference when pointer is NULL expands allowed behavior, so
won't regress existing programs, and the case of passing these into
helpers is the same as above and will be dealt with later.

Also update the failure case in tp_btf_nullable selftest to capture the
new behavior, as the verifier will no longer cause an error when
directly dereference a raw tracepoint argument marked as __nullable.

  [0]: https://lore.kernel.org/bpf/[email protected]

Reviewed-by: Jiri Olsa <[email protected]>
Reported-by: Juri Lelli <[email protected]>
Tested-by: Juri Lelli <[email protected]>
Fixes: 3f00c52 ("bpf: Allow trusted pointers to be passed to KF_TRUSTED_ARGS kfuncs")
Signed-off-by: Kumar Kartikeya Dwivedi <[email protected]>
@kkdwivedi
selftests/bpf: Clean up open-coded gettid syscall invocations
5b11c58 
Availability of the gettid definition across glibc versions supported by
BPF selftests is not certain. Currently, all users in the tree open-code
syscall to gettid. Convert them to a common macro definition.

Reviewed-by: Jiri Olsa <[email protected]>
Signed-off-by: Kumar Kartikeya Dwivedi <[email protected]>
@kkdwivedi
selftests/bpf: Add tests for raw_tp null handling
37fd6b3 
Ensure that trusted PTR_TO_BTF_ID accesses perform PROBE_MEM handling in
raw_tp program. Without the previous fix, this selftest crashes the
kernel due to a NULL-pointer dereference. Also ensure that dead code
elimination does not kick in for checks on the pointer.

Reviewed-by: Jiri Olsa <[email protected]>
Signed-off-by: Kumar Kartikeya Dwivedi <[email protected]>
@kernel-patches-daemon-bpf-rc
Copy link
Author

Upstream branch: 9a78313
series: https://patchwork.kernel.org/project/netdevbpf/list/?series=906140
version: 3

@kernel-patches-daemon-bpf-rc
Copy link
Author

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

@kernel-patches-daemon-bpf-rc kernel-patches-daemon-bpf-rc bot deleted the series/905194=>bpf-next branch November 4, 2024 19:52
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers
No reviews
Assignees
No one assigned
Labels
accepted bpf-next V3-ci-fail V3
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.
1 participant
@kkdwivedi