skb-backed dynptr writer kfuncs can mutate packet data, but the verifier
leaves checked direct packet pointers usable after kfunc calls.
The bpf_dynptr_write() helper already invalidates packet pointers
through clear_all_pkt_pointers(). Make skb dynptr writer kfuncs
follow the same rule.

Keep two verifier predicates for this. CFG analysis runs before register
states are available, so conservatively mark dynptr writer kfuncs as
packet-changing for subprogram summaries. The normal verifier path uses
the checked dynptr argument and invalidates only when the written dynptr
is, or may be, skb-backed.

Global subprogram dynptr arguments are prepared as unspecialized local
dynptr pointers, so treat CONST_PTR_TO_DYNPTR local dynptr writer
destinations as possibly packet-backed. This keeps packet pointer
invalidation sound both after global subprogram calls and inside global
subprogram bodies.

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 ++
 kernel/bpf/cfg.c             |  2 +-
 kernel/bpf/verifier.c        | 79 +++++++++++++++++++++++++++++++++++-
 3 files changed, 82 insertions(+), 2 deletions(-)

diff --git a/include/linux/bpf_verifier.h b/include/linux/bpf_verifier.h
index 39a851e690ec4..603f22220122e 100644
--- a/include/linux/bpf_verifier.h
+++ b/include/linux/bpf_verifier.h
@@ -1448,6 +1448,7 @@ struct bpf_kfunc_call_arg_meta {
        /* Out parameters */
        u8 release_regno;
        bool r0_rdonly;
+       bool pkt_dynptr_write;
        u32 ret_btf_id;
        u64 r0_size;
        u32 subprogno;
@@ -1502,6 +1503,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/kernel/bpf/cfg.c b/kernel/bpf/cfg.c
index 26d37066465f3..7ed6bdd986bc8 100644
--- a/kernel/bpf/cfg.c
+++ b/kernel/bpf/cfg.c
@@ -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 2abc79dbf281c..9c1e3cccbd6de 100644
--- a/kernel/bpf/verifier.c
+++ b/kernel/bpf/verifier.c
@@ -11016,6 +11016,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,
@@ -11096,6 +11106,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)
@@ -11227,9 +11258,52 @@ 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 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;
 }
 
 static enum kfunc_ptr_arg_type
@@ -12214,6 +12288,9 @@ static int check_kfunc_args(struct bpf_verifier_env 
*env, struct bpf_kfunc_call_
                                                  &meta->ref_obj, 
&meta->dynptr);
                        if (ret < 0)
                                return ret;
+                       if (is_kfunc_pkt_dynptr_writer(meta, i) &&
+                           dynptr_may_be_pkt_data(reg, meta->dynptr.type))
+                               meta->pkt_dynptr_write = true;
                        break;
                }
                case KF_ARG_PTR_TO_ITER:
-- 
2.34.1


Reply via email to