mirror of
https://github.com/torvalds/linux.git
synced 2026-11-07 06:17:02 +01:00
Merge branch 'bpf-destroy-callee-local-dynptrs-on-subprog-return'
Xu Yunxiang says:
====================
bpf: Destroy callee-local dynptrs on subprog return
A dynptr data slice is valid only while the specific dynptr that produced
it remains valid. Apply normal dynptr stack-slot teardown before a callee
frame disappears, so its descendants are invalidated and its last
release-capable referenced dynptr cannot be lost.
Patch 1 adds a helper to destroy dynptrs in an inclusive range of stack
slots and reuses it for initialization, variable-offset writes and
subprogram returns. The callback diagnostic expectation changes in that
same commit. Patch 2 tests callee-local slices, last-holder rejection and
caller-owned slice compatibility.
Changes in v6, following Amery's review:
- Replace destroy_dynptrs_on_func_exit() with the generic
destroy_dynptrs_in_stack_slots() and use it at all three call sites.
- Add the requested blank line before return-time teardown.
- Clarify callee-local clone comments and rename the rejection test to
callee_dynptr_slice_invalid_after_return().
- Rebase onto bpf-next ea9358e127.
v5:
https://lore.kernel.org/r/20260920210438.345847-1-xyx2021@mail.ustc.edu.cn
Review:
https://lore.kernel.org/r/CAMB2axMmrTb+UZ84UD48MwMtXbk1s9bWtreU3AOfjzU0fPT0BA@mail.gmail.com
https://lore.kernel.org/r/CAMB2axOEZjRV1K4Yxb4=1f2jvPHiD2rVy-+byXbvs3G0qat63g@mail.gmail.com
Validation on the rebased candidate with a matching bpf_testmod:
- W=1 verifier, full kernel and modules builds passed.
- Changed BPF objects and required fixtures compiled strictly.
- dynptr: 2/137 passed; 0 skipped.
- file_reader: 1/8 passed; 0 skipped.
- ns_bpf_qdisc/invalid_dynptr_returned_slice: 1/1 passed; 0 skipped.
- global_func: 3/29 passed; 3 skipped.
- cb_refs: 1/4 passed; 0 skipped.
- subprogs: 5/34 passed; 0 skipped.
- verifier_var_off: 1/24 passed; 1 skipped.
No selected test failed. The VM used panic_on_warn and panic_on_oops;
no WARN_ON report, Oops or panic was found. Unprivileged BPF was
enabled only in the isolated guest; the kernel printed the expected
Spectre-v2 notice when enabling that sysctl.
The broad test_progs build used BPF_STRICT_BUILD=0 for unselected
objects requiring unavailable kernel features. The changed objects
and required fixtures were also compiled in strict mode. Annotated
lifetime tests check loading and diagnostics. The unfiltered full
suite, sanitizer and architecture matrices, and new unmodified-base
differential probes were not run in this rebase validation.
Three 512-byte stack-limit variants skip on this 2 KiB-stack kernel;
their large-stack counterparts passed. ctx_arg_rewrite skips because
the kernel supports native context-argument tags.
Please queue the verifier fix for stable after it reaches the BPF tree.
====================
Link: https://patch.msgid.link/20260927120422.1042107-1-xyx2021@mail.ustc.edu.cn
Signed-off-by: Kumar Kartikeya Dwivedi <memxor@gmail.com>
This commit is contained in:
commit
e1d84a37cb
|
|
@ -11,6 +11,7 @@
|
|||
#include "bpf_qdisc_fail__invalid_dynptr.skel.h"
|
||||
#include "bpf_qdisc_fail__invalid_dynptr_slice.skel.h"
|
||||
#include "bpf_qdisc_fail__invalid_dynptr_cross_frame.skel.h"
|
||||
#include "bpf_qdisc_fail__invalid_dynptr_returned_slice.skel.h"
|
||||
#include "bpf_qdisc_fail__untrusted_write.skel.h"
|
||||
#include "bpf_qdisc_dynptr_use_after_invalidate_clone.skel.h"
|
||||
|
||||
|
|
@ -230,6 +231,7 @@ void test_ns_bpf_qdisc(void)
|
|||
test_incompl_ops();
|
||||
RUN_TESTS(bpf_qdisc_fail__invalid_dynptr);
|
||||
RUN_TESTS(bpf_qdisc_fail__invalid_dynptr_cross_frame);
|
||||
RUN_TESTS(bpf_qdisc_fail__invalid_dynptr_returned_slice);
|
||||
RUN_TESTS(bpf_qdisc_fail__invalid_dynptr_slice);
|
||||
RUN_TESTS(bpf_qdisc_fail__untrusted_write);
|
||||
RUN_TESTS(bpf_qdisc_dynptr_use_after_invalidate_clone);
|
||||
|
|
|
|||
|
|
@ -0,0 +1,76 @@
|
|||
// SPDX-License-Identifier: GPL-2.0
|
||||
|
||||
#include <vmlinux.h>
|
||||
#include "bpf_experimental.h"
|
||||
#include "bpf_qdisc_common.h"
|
||||
#include "bpf_misc.h"
|
||||
|
||||
char _license[] SEC("license") = "GPL";
|
||||
|
||||
int proto;
|
||||
|
||||
static __noinline struct ethhdr *slice_in_subprog(struct sk_buff *skb)
|
||||
{
|
||||
struct bpf_dynptr ptr;
|
||||
|
||||
bpf_dynptr_from_skb((struct __sk_buff *)skb, 0, &ptr);
|
||||
return bpf_dynptr_slice(&ptr, 0, NULL, sizeof(struct ethhdr));
|
||||
}
|
||||
|
||||
SEC("struct_ops")
|
||||
__failure __msg("invalid mem access 'scalar'")
|
||||
int BPF_PROG(invalid_dynptr_returned_slice, struct sk_buff *skb,
|
||||
struct Qdisc *sch, struct bpf_sk_buff_ptr *to_free)
|
||||
{
|
||||
struct ethhdr *hdr;
|
||||
|
||||
hdr = slice_in_subprog(skb);
|
||||
if (!hdr) {
|
||||
bpf_qdisc_skb_drop(skb, to_free);
|
||||
return NET_XMIT_DROP;
|
||||
}
|
||||
|
||||
/* this should fail */
|
||||
proto = hdr->h_proto;
|
||||
|
||||
bpf_qdisc_skb_drop(skb, to_free);
|
||||
|
||||
return NET_XMIT_DROP;
|
||||
}
|
||||
|
||||
SEC("struct_ops")
|
||||
__auxiliary
|
||||
struct sk_buff *BPF_PROG(bpf_qdisc_test_dequeue, struct Qdisc *sch)
|
||||
{
|
||||
return NULL;
|
||||
}
|
||||
|
||||
SEC("struct_ops")
|
||||
__auxiliary
|
||||
int BPF_PROG(bpf_qdisc_test_init, struct Qdisc *sch, struct nlattr *opt,
|
||||
struct netlink_ext_ack *extack)
|
||||
{
|
||||
return 0;
|
||||
}
|
||||
|
||||
SEC("struct_ops")
|
||||
__auxiliary
|
||||
void BPF_PROG(bpf_qdisc_test_reset, struct Qdisc *sch)
|
||||
{
|
||||
}
|
||||
|
||||
SEC("struct_ops")
|
||||
__auxiliary
|
||||
void BPF_PROG(bpf_qdisc_test_destroy, struct Qdisc *sch)
|
||||
{
|
||||
}
|
||||
|
||||
SEC(".struct_ops")
|
||||
struct Qdisc_ops test = {
|
||||
.enqueue = (void *)invalid_dynptr_returned_slice,
|
||||
.dequeue = (void *)bpf_qdisc_test_dequeue,
|
||||
.init = (void *)bpf_qdisc_test_init,
|
||||
.reset = (void *)bpf_qdisc_test_reset,
|
||||
.destroy = (void *)bpf_qdisc_test_destroy,
|
||||
.id = "bpf_qdisc_test",
|
||||
};
|
||||
|
|
@ -1895,6 +1895,101 @@ int clone_invalidate4(void *ctx)
|
|||
return 0;
|
||||
}
|
||||
|
||||
static __noinline void clone_slice_in_subprog(struct bpf_dynptr *ptr, int **data)
|
||||
{
|
||||
struct bpf_dynptr clone;
|
||||
|
||||
bpf_dynptr_clone(ptr, &clone);
|
||||
*data = bpf_dynptr_data(&clone, 0, sizeof(val));
|
||||
}
|
||||
|
||||
static __noinline void caller_slice_in_subprog(struct bpf_dynptr *ptr, int **data)
|
||||
{
|
||||
struct bpf_dynptr clone;
|
||||
|
||||
*data = bpf_dynptr_data(ptr, 0, sizeof(val));
|
||||
bpf_dynptr_clone(ptr, &clone);
|
||||
}
|
||||
|
||||
static __noinline void reserve_dynptr_in_subprog(void)
|
||||
{
|
||||
struct bpf_dynptr ptr;
|
||||
|
||||
bpf_ringbuf_reserve_dynptr(&ringbuf, val, 0, &ptr);
|
||||
}
|
||||
|
||||
/* A subprogram cannot lose the last dynptr that can release a resource. */
|
||||
SEC("?raw_tp")
|
||||
__failure __msg("cannot overwrite referenced dynptr")
|
||||
int referenced_dynptr_lost_on_subprog_return(void *ctx)
|
||||
{
|
||||
reserve_dynptr_in_subprog();
|
||||
|
||||
return 0;
|
||||
}
|
||||
|
||||
/*
|
||||
* Destroying a callee-local clone on return must not invalidate a slice whose
|
||||
* source dynptr belongs to the caller.
|
||||
*/
|
||||
SEC("?raw_tp")
|
||||
__success
|
||||
int caller_dynptr_slice_across_subprog_valid(void *ctx)
|
||||
{
|
||||
struct bpf_dynptr ptr;
|
||||
int *data = NULL;
|
||||
|
||||
bpf_ringbuf_reserve_dynptr(&ringbuf, val, 0, &ptr);
|
||||
caller_slice_in_subprog(&ptr, &data);
|
||||
if (data)
|
||||
*data = 123;
|
||||
bpf_ringbuf_submit_dynptr(&ptr, 0);
|
||||
|
||||
return 0;
|
||||
}
|
||||
|
||||
/*
|
||||
* A slice derived from a callee-local clone is invalid after the subprogram
|
||||
* returns.
|
||||
*/
|
||||
SEC("?raw_tp")
|
||||
__failure __msg("invalid mem access 'scalar'")
|
||||
int callee_dynptr_slice_invalid_after_return(void *ctx)
|
||||
{
|
||||
struct bpf_dynptr ptr;
|
||||
int *data = NULL;
|
||||
|
||||
bpf_ringbuf_reserve_dynptr(&ringbuf, val, 0, &ptr);
|
||||
clone_slice_in_subprog(&ptr, &data);
|
||||
if (data)
|
||||
/* this should fail */
|
||||
*data = 123;
|
||||
bpf_ringbuf_submit_dynptr(&ptr, 0);
|
||||
|
||||
return 0;
|
||||
}
|
||||
|
||||
/*
|
||||
* A slice from a caller-owned dynptr survives the subprogram return, but
|
||||
* releasing the shared reservation must invalidate it.
|
||||
*/
|
||||
SEC("?raw_tp")
|
||||
__failure __msg("invalid mem access 'scalar'")
|
||||
int caller_dynptr_slice_release_after_subprog_invalid(void *ctx)
|
||||
{
|
||||
struct bpf_dynptr ptr;
|
||||
int *data = NULL;
|
||||
|
||||
bpf_ringbuf_reserve_dynptr(&ringbuf, val, 0, &ptr);
|
||||
caller_slice_in_subprog(&ptr, &data);
|
||||
bpf_ringbuf_submit_dynptr(&ptr, 0);
|
||||
if (data)
|
||||
/* this should fail */
|
||||
*data = 123;
|
||||
|
||||
return 0;
|
||||
}
|
||||
|
||||
/* Invalidating a dynptr should invalidate any data slices
|
||||
* of its parent
|
||||
*/
|
||||
|
|
|
|||
Loading…
Reference in New Issue
Block a user