diff options
| -rw-r--r-- | include/linux/bpf_verifier.h | 4 | ||||
| -rw-r--r-- | kernel/bpf/verifier.c | 27 | ||||
| -rw-r--r-- | tools/testing/selftests/bpf/progs/percpu_alloc_fail.c | 59 | ||||
| -rw-r--r-- | tools/testing/selftests/bpf/progs/rcu_read_lock.c | 6 | ||||
| -rw-r--r-- | tools/testing/selftests/bpf/progs/refcounted_kptr.c | 61 | ||||
| -rw-r--r-- | tools/testing/selftests/bpf/progs/refcounted_kptr_fail.c | 150 |
6 files changed, 298 insertions, 9 deletions
diff --git a/include/linux/bpf_verifier.h b/include/linux/bpf_verifier.h index 1339c2f02..36b657978 100644 --- a/include/linux/bpf_verifier.h +++ b/include/linux/bpf_verifier.h @@ -1381,7 +1381,9 @@ static inline bool bpf_type_has_unsafe_modifiers(u32 type) static inline bool type_is_ptr_alloc_obj(u32 type) { - return base_type(type) == PTR_TO_BTF_ID && type_flag(type) & MEM_ALLOC; + return base_type(type) == PTR_TO_BTF_ID && + type_flag(type) & MEM_ALLOC && + !(type_flag(type) & PTR_UNTRUSTED); } static inline bool type_is_non_owning_ref(u32 type) diff --git a/kernel/bpf/verifier.c b/kernel/bpf/verifier.c index fde5d046b..9c6ad157a 100644 --- a/kernel/bpf/verifier.c +++ b/kernel/bpf/verifier.c @@ -4495,6 +4495,13 @@ static int map_kptr_match_type(struct bpf_verifier_env *env, if (type_flag(reg->type) & ~perm_flags) goto bad_type; + /* + * A BPF_KPTR_PERCPU field is read back as MEM_PERCPU, so the value + * stored in it must carry the same flag. + */ + if ((kptr_field->type == BPF_KPTR_PERCPU) != !!(reg->type & MEM_PERCPU)) + goto bad_type; + /* We need to verify reg->type and reg->btf, before accessing reg->btf */ reg_name = btf_type_name(reg->btf, reg->btf_id); @@ -6038,7 +6045,13 @@ static int check_ptr_to_btf_access(struct bpf_verifier_env *env, return -EACCES; } - if (type_is_alloc(reg->type) && !type_is_non_owning_ref(reg->type) && + /* + * A fault-prone allocated object may still be read through a + * BPF_PROBE_MEM load after its lifetime protection ends. Writes + * through such pointers were rejected above. + */ + if (type_is_alloc(reg->type) && !bpf_may_fault_on_deref(reg->type) && + !type_is_non_owning_ref(reg->type) && !(reg->type & MEM_RCU) && !reg_is_referenced(env, reg)) { verifier_bug(env, "allocated object must have a referenced id"); return -EFAULT; @@ -7416,10 +7429,14 @@ static int process_spin_lock(struct bpf_verifier_env *env, struct bpf_reg_state lock); return -EINVAL; } + /* + * Invalidate non-owning refs before RCU demotion clears their + * NON_OWN_REF flag. + */ + invalidate_non_owning_refs(env); + if (!in_rcu_cs(env)) invalidate_rcu_protected_refs(env); - - invalidate_non_owning_refs(env); } return 0; } @@ -9519,7 +9536,7 @@ static void invalidate_rcu_protected_refs(struct bpf_verifier_env *env) bpf_for_each_reg_in_vstate_mask(env->cur_state, state, reg, stack, clear_mask, ({ if (reg->type & MEM_RCU) { bpf_diag_mod_begin(env, reg, NULL, BPF_DIAG_MOD_WRITE); - reg->type &= ~(MEM_RCU | PTR_MAYBE_NULL); + reg->type &= ~(MEM_RCU | PTR_MAYBE_NULL | NON_OWN_REF); reg->type |= PTR_UNTRUSTED; bpf_diag_mod_end(env); } @@ -13178,7 +13195,7 @@ check_ok: bpf_diag_reg_type_plain(env, reg->type)); return -EINVAL; } - if (!type_is_non_owning_ref(reg->type)) + if (!type_is_non_owning_ref(reg->type) && reg_is_referenced(env, reg)) meta->arg_owning_ref = true; rec = reg_btf_record(reg); diff --git a/tools/testing/selftests/bpf/progs/percpu_alloc_fail.c b/tools/testing/selftests/bpf/progs/percpu_alloc_fail.c index 08379c3b6..3701f4ea5 100644 --- a/tools/testing/selftests/bpf/progs/percpu_alloc_fail.c +++ b/tools/testing/selftests/bpf/progs/percpu_alloc_fail.c @@ -33,6 +33,20 @@ struct { __type(value, struct elem); } array SEC(".maps"); +struct kernel_percpu_elem { + struct task_struct __percpu_kptr *task; +}; + +struct { + __uint(type, BPF_MAP_TYPE_ARRAY); + __uint(max_entries, 1); + __type(key, int); + __type(value, struct kernel_percpu_elem); +} kernel_percpu_array SEC(".maps"); + +struct task_struct *bpf_task_from_pid(s32 pid) __ksym; +void bpf_task_release(struct task_struct *p) __ksym; + long ret; SEC("?fentry/bpf_fentry_test1") @@ -137,6 +151,51 @@ int BPF_PROG(test_array_map_5) return 0; } +SEC("?syscall") +__failure __msg("invalid kptr access, R2 type=trusted_ptr_ expected=ptr_task_struct") +int reject_kernel_ptr_into_percpu_kptr(void *ctx) +{ + struct kernel_percpu_elem *e; + struct task_struct *p, *old; + int index = 0; + + e = bpf_map_lookup_elem(&kernel_percpu_array, &index); + if (!e) + return 0; + + p = bpf_task_from_pid(1); + if (!p) + return 0; + + old = bpf_kptr_xchg(&e->task, p); + if (old) + bpf_task_release(old); + return 0; +} + +SEC("?fentry.s/bpf_fentry_test1") +__failure __msg("invalid kptr access, R2 type=ptr_ expected=ptr_val_t") +int BPF_PROG(reject_plain_alloc_into_percpu_kptr) +{ + struct val_t __percpu_kptr *old; + struct val_t *p; + struct elem *e; + int index = 0; + + e = bpf_map_lookup_elem(&array, &index); + if (!e) + return 0; + + p = bpf_obj_new(struct val_t); + if (!p) + return 0; + + old = bpf_kptr_xchg(&e->pc, p); + if (old) + bpf_percpu_obj_drop(old); + return 0; +} + SEC("?fentry.s/bpf_fentry_test1") __failure __msg("bpf_percpu_obj_new type ID argument must be of a struct of scalars") int BPF_PROG(test_array_map_6) diff --git a/tools/testing/selftests/bpf/progs/rcu_read_lock.c b/tools/testing/selftests/bpf/progs/rcu_read_lock.c index 31d4081c3..cdb255add 100644 --- a/tools/testing/selftests/bpf/progs/rcu_read_lock.c +++ b/tools/testing/selftests/bpf/progs/rcu_read_lock.c @@ -592,9 +592,9 @@ int non_own_ref_untrusted_ld(void *ctx) } bpf_rcu_read_unlock(); /* - * The unlock leaves node as PTR_TO_BTF_ID | MEM_ALLOC | PTR_UNTRUSTED - * | NON_OWN_REF, and the load below has to get the BPF_PROBE_MEM - * rewrite for it, otherwise a bad address panics the kernel. + * The unlock leaves node as PTR_TO_BTF_ID | MEM_ALLOC | PTR_UNTRUSTED, + * and the load below has to get the BPF_PROBE_MEM rewrite for it, + * otherwise a bad address panics the kernel. */ non_own_ref_key = node->key; return 0; diff --git a/tools/testing/selftests/bpf/progs/refcounted_kptr.c b/tools/testing/selftests/bpf/progs/refcounted_kptr.c index 61906f480..cae00f7b0 100644 --- a/tools/testing/selftests/bpf/progs/refcounted_kptr.c +++ b/tools/testing/selftests/bpf/progs/refcounted_kptr.c @@ -23,6 +23,15 @@ struct map_value { struct node_data __kptr *node; }; +struct node_refcount_only { + long key; + struct bpf_refcount refcount; +}; + +struct map_value_refcount_only { + struct node_refcount_only __kptr *node; +}; + struct { __uint(type, BPF_MAP_TYPE_ARRAY); __type(key, int); @@ -30,6 +39,13 @@ struct { __uint(max_entries, 2); } stashed_nodes SEC(".maps"); +struct { + __uint(type, BPF_MAP_TYPE_ARRAY); + __type(key, int); + __type(value, struct map_value_refcount_only); + __uint(max_entries, 1); +} stashed_refcount_only SEC(".maps"); + struct node_acquire { long key; long data; @@ -832,6 +848,51 @@ long rbtree_refcounted_node_ref_escapes_owning_input(void *ctx) return 0; } +SEC("tc") +__success +long refcount_acquire_owning_input_no_null_check(void *ctx) +{ + struct node_refcount_only *n, *m; + + n = bpf_obj_new(typeof(*n)); + if (!n) + return 1; + + m = bpf_refcount_acquire(n); + bpf_obj_drop(m); + bpf_obj_drop(n); + + return 0; +} + +SEC("?syscall") +__success +long refcount_acquire_rcu_map_kptr_null_checked(void *ctx) +{ + struct map_value_refcount_only *mapval; + struct node_refcount_only *n, *m; + int idx = 0; + + mapval = bpf_map_lookup_elem(&stashed_refcount_only, &idx); + if (!mapval) + return 1; + + bpf_rcu_read_lock(); + n = mapval->node; + if (!n) { + bpf_rcu_read_unlock(); + return 2; + } + m = bpf_refcount_acquire(n); + bpf_rcu_read_unlock(); + + if (!m) + return 3; + bpf_obj_drop(m); + + return 0; +} + static long __stash_map_empty_xchg(struct node_data *n, int idx) { struct map_value *mapval = bpf_map_lookup_elem(&stashed_nodes, &idx); diff --git a/tools/testing/selftests/bpf/progs/refcounted_kptr_fail.c b/tools/testing/selftests/bpf/progs/refcounted_kptr_fail.c index eaaed0859..338e43822 100644 --- a/tools/testing/selftests/bpf/progs/refcounted_kptr_fail.c +++ b/tools/testing/selftests/bpf/progs/refcounted_kptr_fail.c @@ -19,6 +19,26 @@ struct node_refcounted { struct bpf_refcount refcount; }; +struct node_refcount_only { + long key; + struct bpf_refcount refcount; +}; + +struct map_value_refcount_only { + struct node_refcount_only __kptr *node; +}; + +struct rcu_graph_node { + struct bpf_rb_node node; + long data; +}; + +struct rcu_graph_node *just_here_because_btf_bug; + +struct map_value_rcu_graph { + struct rcu_graph_node __kptr *node; +}; + extern void bpf_rcu_read_lock(void) __ksym; extern void bpf_rcu_read_unlock(void) __ksym; @@ -27,6 +47,22 @@ private(A) struct bpf_spin_lock glock; private(A) struct bpf_rb_root groot __contains(node_acquire, node); private(B) struct bpf_spin_lock lock; private(B) struct bpf_list_head head __contains(node_refcounted, list); +private(C) struct bpf_spin_lock graph_lock; +private(C) struct bpf_rb_root graph_root __contains(rcu_graph_node, node); + +struct { + __uint(type, BPF_MAP_TYPE_ARRAY); + __type(key, int); + __type(value, struct map_value_refcount_only); + __uint(max_entries, 1); +} stashed_refcount_only SEC(".maps"); + +struct { + __uint(type, BPF_MAP_TYPE_ARRAY); + __type(key, int); + __type(value, struct map_value_rcu_graph); + __uint(max_entries, 1); +} stashed_rcu_graph SEC(".maps"); static bool less(struct bpf_rb_node *a, const struct bpf_rb_node *b) { @@ -89,6 +125,120 @@ long refcount_acquire_non_object(void *ctx) return bpf_refcount_acquire(ctx) != NULL; } +SEC("?syscall") +__failure __msg("Possibly NULL pointer passed to trusted R1") +long refcount_acquire_rcu_map_kptr_unchecked_drop(void *ctx) +{ + struct map_value_refcount_only *mapval; + struct node_refcount_only *tmp, *n, *m; + int idx = 0; + + /* Force Clang to emit complete BTF for struct node_refcount_only. */ + tmp = bpf_obj_new(typeof(*tmp)); + if (!tmp) + return 3; + bpf_obj_drop(tmp); + + mapval = bpf_map_lookup_elem(&stashed_refcount_only, &idx); + if (!mapval) + return 1; + + bpf_rcu_read_lock(); + n = mapval->node; + if (!n) { + bpf_rcu_read_unlock(); + return 2; + } + m = bpf_refcount_acquire(n); + bpf_rcu_read_unlock(); + + bpf_obj_drop(m); + + return 0; +} + +SEC("?syscall") +__failure +__msg("bpf_rbtree_remove can only take non-owning or refcounted " + "bpf_rb_node pointer") +long rbtree_remove_after_rcu_unlock(void *ctx) +{ + struct map_value_rcu_graph *mapval; + struct bpf_rb_node *rb_node; + struct rcu_graph_node *node; + int idx = 0; + + mapval = bpf_map_lookup_elem(&stashed_rcu_graph, &idx); + if (!mapval) + return 0; + + bpf_rcu_read_lock(); + node = mapval->node; + if (!node) { + bpf_rcu_read_unlock(); + return 0; + } + bpf_rcu_read_unlock(); + + bpf_spin_lock(&graph_lock); + rb_node = bpf_rbtree_remove(&graph_root, &node->node); + bpf_spin_unlock(&graph_lock); + if (rb_node) + bpf_obj_drop(container_of(rb_node, struct rcu_graph_node, node)); + + return 0; +} + +SEC("?syscall") +__failure __msg("R1 is neither owning or non-owning ref") +long refcount_acquire_after_rcu_unlock(void *ctx) +{ + struct map_value_refcount_only *mapval; + struct node_refcount_only *node, *ref; + int idx = 0; + + mapval = bpf_map_lookup_elem(&stashed_refcount_only, &idx); + if (!mapval) + return 0; + + bpf_rcu_read_lock(); + node = mapval->node; + if (!node) { + bpf_rcu_read_unlock(); + return 0; + } + bpf_rcu_read_unlock(); + + ref = bpf_refcount_acquire(node); + if (ref) + bpf_obj_drop(ref); + + return 0; +} + +SEC("?syscall") +__failure __msg("invalid mem access 'scalar'") +long graph_kptr_after_spin_unlock(void *ctx) +{ + struct map_value_rcu_graph *mapval; + struct rcu_graph_node *node; + int idx = 0; + + mapval = bpf_map_lookup_elem(&stashed_rcu_graph, &idx); + if (!mapval) + return 0; + + bpf_spin_lock(&graph_lock); + node = mapval->node; + if (!node) { + bpf_spin_unlock(&graph_lock); + return 0; + } + bpf_spin_unlock(&graph_lock); + + return node->data; +} + SEC("?tc") __failure __msg("Unreleased reference id=3 alloc_insn={{[0-9]+}}") long rbtree_refcounted_node_ref_escapes_owning_input(void *ctx) |