skb-backed dynptr writers can mutate packet data, but not all verifier
paths invalidate checked direct packet pointers after those writes.
bpf_dynptr_write() handles skb and skb-meta dynptrs in the normal helper
path, but global subprogram dynptr arguments are verified as
unspecialized local dynptr pointers. Treat CONST_PTR_TO_DYNPTR local
arguments as possibly packet-backed for packet pointer invalidation.
Global subprogram summaries are computed during CFG analysis before
register states exist. Add conservative static CFG predicates for
bpf_dynptr_write() and skb dynptr writer kfuncs so caller-side packet
pointers are invalidated after global calls that may write packet data.
Keep the normal verifier invalidation precise: helpers and kfuncs still
use the checked dynptr argument and only invalidate when the written
dynptr is, or may be, skb-backed. Source-only dynptr arguments remain
unchanged.
Fixes: b5964b968ac6 ("bpf: Add skb dynptrs")
Fixes: daec295a7094 ("bpf/helpers: Introduce bpf_dynptr_copy kfunc")
Fixes: a498ee7576de ("bpf: Implement dynptr copy kfuncs")
Fixes: 5fc5d8fded57 ("bpf: Add bpf_dynptr_memset() kfunc")
Signed-off-by: Yiyang Chen <[email protected]>
---
include/linux/bpf_verifier.h | 3 ++
include/linux/filter.h | 5 ++
kernel/bpf/cfg.c | 4 +-
kernel/bpf/verifier.c | 88 ++++++++++++++++++++++++++++++++++--
4 files changed, 95 insertions(+), 5 deletions(-)
diff --git a/include/linux/bpf_verifier.h b/include/linux/bpf_verifier.h
index 185b2aa43a420..b550e5a702081 100644
--- a/include/linux/bpf_verifier.h
+++ b/include/linux/bpf_verifier.h
@@ -1332,6 +1332,7 @@ struct bpf_kfunc_call_arg_meta {
u32 ref_obj_id;
u8 release_regno;
bool r0_rdonly;
+ bool pkt_dynptr_write;
u32 ret_btf_id;
u64 r0_size;
u32 subprogno;
@@ -1389,6 +1390,8 @@ static inline bool bpf_is_kfunc_sleepable(struct
bpf_kfunc_call_arg_meta *meta)
{
return meta->kfunc_flags & KF_SLEEPABLE;
}
+
+bool bpf_kfunc_may_change_pkt_data(struct bpf_kfunc_call_arg_meta *meta);
bool bpf_is_kfunc_pkt_changing(struct bpf_kfunc_call_arg_meta *meta);
struct bpf_iarray *bpf_iarray_realloc(struct bpf_iarray *old, size_t n_elem);
int bpf_copy_insn_array_uniq(struct bpf_map *map, u32 start, u32 end, u32
*off);
diff --git a/include/linux/filter.h b/include/linux/filter.h
index 88a241aac36a2..48485ca84d395 100644
--- a/include/linux/filter.h
+++ b/include/linux/filter.h
@@ -1172,6 +1172,11 @@ void arch_bpf_stack_walk(bool (*consume_fn)(void
*cookie, u64 ip, u64 sp, u64 bp
u64 arch_bpf_timed_may_goto(void);
u64 bpf_check_timed_may_goto(struct bpf_timed_may_goto *);
bool bpf_helper_changes_pkt_data(enum bpf_func_id func_id);
+static inline bool bpf_helper_may_change_pkt_data(enum bpf_func_id func_id)
+{
+ return bpf_helper_changes_pkt_data(func_id) ||
+ func_id == BPF_FUNC_dynptr_write;
+}
static inline bool bpf_dump_raw_ok(const struct cred *cred)
{
diff --git a/kernel/bpf/cfg.c b/kernel/bpf/cfg.c
index 26d37066465f3..54a2130f1e465 100644
--- a/kernel/bpf/cfg.c
+++ b/kernel/bpf/cfg.c
@@ -483,7 +483,7 @@ static int visit_insn(int t, struct bpf_verifier_env *env)
*/
if (ret == 0 && fp->might_sleep)
mark_subprog_might_sleep(env, t);
- if (bpf_helper_changes_pkt_data(insn->imm))
+ if (bpf_helper_may_change_pkt_data(insn->imm))
mark_subprog_changes_pkt_data(env, t);
if (insn->imm == BPF_FUNC_tail_call) {
ret = visit_abnormal_return_insn(env, t);
@@ -516,7 +516,7 @@ static int visit_insn(int t, struct bpf_verifier_env *env)
*/
if (ret == 0 && bpf_is_kfunc_sleepable(&meta))
mark_subprog_might_sleep(env, t);
- if (ret == 0 && bpf_is_kfunc_pkt_changing(&meta))
+ if (ret == 0 && bpf_kfunc_may_change_pkt_data(&meta))
mark_subprog_changes_pkt_data(env, t);
if (ret == 0 && bpf_is_throw_kfunc(insn))
mark_subprog_might_throw(env, t);
diff --git a/kernel/bpf/verifier.c b/kernel/bpf/verifier.c
index 7fb88e1cd7c4d..51468faa76a47 100644
--- a/kernel/bpf/verifier.c
+++ b/kernel/bpf/verifier.c
@@ -9155,6 +9155,23 @@ static void clear_all_pkt_pointers(struct
bpf_verifier_env *env)
}));
}
+static bool dynptr_type_pkt_data(enum bpf_dynptr_type type)
+{
+ return type == BPF_DYNPTR_TYPE_SKB ||
+ type == BPF_DYNPTR_TYPE_SKB_META;
+}
+
+static bool dynptr_may_be_pkt_data(const struct bpf_reg_state *reg,
+ enum bpf_dynptr_type type)
+{
+ if (dynptr_type_pkt_data(type))
+ return true;
+
+ /* Global subprog dynptr args are verified as unspecialized LOCAL. */
+ return reg->type == CONST_PTR_TO_DYNPTR &&
+ type == BPF_DYNPTR_TYPE_LOCAL;
+}
+
enum {
AT_PKT_END = -1,
BEYOND_PKT_END = -2,
@@ -10520,8 +10537,7 @@ static int check_helper_call(struct bpf_verifier_env
*env, struct bpf_insn *insn
if (dynptr_type == BPF_DYNPTR_TYPE_INVALID)
return -EFAULT;
- if (dynptr_type == BPF_DYNPTR_TYPE_SKB ||
- dynptr_type == BPF_DYNPTR_TYPE_SKB_META)
+ if (dynptr_may_be_pkt_data(reg, dynptr_type))
/* this will trigger clear_all_pkt_pointers(), which
will
* invalidate all dynptr slices associated with the skb
*/
@@ -11157,6 +11173,16 @@ enum special_kfunc_type {
KF_bpf_xdp_pull_data,
KF_bpf_dynptr_slice,
KF_bpf_dynptr_slice_rdwr,
+ KF_bpf_dynptr_copy,
+ KF_bpf_dynptr_memset,
+ KF_bpf_probe_read_user_dynptr,
+ KF_bpf_probe_read_kernel_dynptr,
+ KF_bpf_probe_read_user_str_dynptr,
+ KF_bpf_probe_read_kernel_str_dynptr,
+ KF_bpf_copy_from_user_dynptr,
+ KF_bpf_copy_from_user_str_dynptr,
+ KF_bpf_copy_from_user_task_dynptr,
+ KF_bpf_copy_from_user_task_str_dynptr,
KF_bpf_dynptr_clone,
KF_bpf_percpu_obj_new_impl,
KF_bpf_percpu_obj_new,
@@ -11232,6 +11258,27 @@ BTF_ID_UNUSED
#endif
BTF_ID(func, bpf_dynptr_slice)
BTF_ID(func, bpf_dynptr_slice_rdwr)
+BTF_ID(func, bpf_dynptr_copy)
+BTF_ID(func, bpf_dynptr_memset)
+#ifdef CONFIG_BPF_EVENTS
+BTF_ID(func, bpf_probe_read_user_dynptr)
+BTF_ID(func, bpf_probe_read_kernel_dynptr)
+BTF_ID(func, bpf_probe_read_user_str_dynptr)
+BTF_ID(func, bpf_probe_read_kernel_str_dynptr)
+BTF_ID(func, bpf_copy_from_user_dynptr)
+BTF_ID(func, bpf_copy_from_user_str_dynptr)
+BTF_ID(func, bpf_copy_from_user_task_dynptr)
+BTF_ID(func, bpf_copy_from_user_task_str_dynptr)
+#else
+BTF_ID_UNUSED
+BTF_ID_UNUSED
+BTF_ID_UNUSED
+BTF_ID_UNUSED
+BTF_ID_UNUSED
+BTF_ID_UNUSED
+BTF_ID_UNUSED
+BTF_ID_UNUSED
+#endif
BTF_ID(func, bpf_dynptr_clone)
BTF_ID(func, bpf_percpu_obj_new_impl)
BTF_ID(func, bpf_percpu_obj_new)
@@ -11362,9 +11409,35 @@ static bool is_kfunc_bpf_preempt_enable(struct
bpf_kfunc_call_arg_meta *meta)
return meta->func_id == special_kfunc_list[KF_bpf_preempt_enable];
}
+static bool is_kfunc_pkt_dynptr_writer(struct bpf_kfunc_call_arg_meta *meta,
u32 arg)
+{
+ u32 func_id = meta->func_id;
+
+ if (arg != 0)
+ return false;
+
+ return func_id == special_kfunc_list[KF_bpf_dynptr_copy] ||
+ func_id == special_kfunc_list[KF_bpf_dynptr_memset] ||
+ func_id == special_kfunc_list[KF_bpf_probe_read_user_dynptr] ||
+ func_id == special_kfunc_list[KF_bpf_probe_read_kernel_dynptr] ||
+ func_id == special_kfunc_list[KF_bpf_probe_read_user_str_dynptr]
||
+ func_id ==
special_kfunc_list[KF_bpf_probe_read_kernel_str_dynptr] ||
+ func_id == special_kfunc_list[KF_bpf_copy_from_user_dynptr] ||
+ func_id == special_kfunc_list[KF_bpf_copy_from_user_str_dynptr]
||
+ func_id == special_kfunc_list[KF_bpf_copy_from_user_task_dynptr]
||
+ func_id ==
special_kfunc_list[KF_bpf_copy_from_user_task_str_dynptr];
+}
+
+bool bpf_kfunc_may_change_pkt_data(struct bpf_kfunc_call_arg_meta *meta)
+{
+ return meta->func_id == special_kfunc_list[KF_bpf_xdp_pull_data] ||
+ is_kfunc_pkt_dynptr_writer(meta, 0);
+}
+
bool bpf_is_kfunc_pkt_changing(struct bpf_kfunc_call_arg_meta *meta)
{
- return meta->func_id == special_kfunc_list[KF_bpf_xdp_pull_data];
+ return meta->func_id == special_kfunc_list[KF_bpf_xdp_pull_data] ||
+ meta->pkt_dynptr_write;
}
static enum kfunc_ptr_arg_type
@@ -12327,6 +12400,15 @@ static int check_kfunc_args(struct bpf_verifier_env
*env, struct bpf_kfunc_call_
ret = process_dynptr_func(env, regno, insn_idx,
dynptr_arg_type, clone_ref_obj_id);
if (ret < 0)
return ret;
+ if (is_kfunc_pkt_dynptr_writer(meta, i)) {
+ enum bpf_dynptr_type type;
+
+ type = dynptr_get_type(env, reg);
+ if (type == BPF_DYNPTR_TYPE_INVALID)
+ return -EFAULT;
+ if (dynptr_may_be_pkt_data(reg, type))
+ meta->pkt_dynptr_write = true;
+ }
if (!(dynptr_arg_type & MEM_UNINIT)) {
int id = dynptr_id(env, reg);
--
2.34.1
