Message ID | 20250129205957.2457655-6-song@kernel.org (mailing list archive) |
---|---|
State | New |
Headers | show |
Series | Enable writing xattr from BPF programs | expand |
On Wed, Jan 29, 2025 at 1:00 PM Song Liu <song@kernel.org> wrote: > > btf_kfunc_id_set.remap can pick proper version of a kfunc for the calling > context. Use this logic to select bpf_dynptr_from_skb or > bpf_dynptr_from_skb_rdonly. This will make the verifier simpler. > > Unfortunately, btf_kfunc_id_set.remap cannot cover the DYNPTR_TYPE_SKB > logic in check_kfunc_args(). This can be addressed later. > > Signed-off-by: Song Liu <song@kernel.org> > --- > kernel/bpf/verifier.c | 25 ++++++---------------- > net/core/filter.c | 49 +++++++++++++++++++++++++++++++++++++++---- > 2 files changed, 51 insertions(+), 23 deletions(-) > > diff --git a/kernel/bpf/verifier.c b/kernel/bpf/verifier.c > index 2188b6674266..55e710e318e5 100644 > --- a/kernel/bpf/verifier.c > +++ b/kernel/bpf/verifier.c > @@ -11750,6 +11750,7 @@ enum special_kfunc_type { > KF_bpf_rbtree_add_impl, > KF_bpf_rbtree_first, > KF_bpf_dynptr_from_skb, > + KF_bpf_dynptr_from_skb_rdonly, > KF_bpf_dynptr_from_xdp, > KF_bpf_dynptr_slice, > KF_bpf_dynptr_slice_rdwr, > @@ -11785,6 +11786,7 @@ BTF_ID(func, bpf_rbtree_add_impl) > BTF_ID(func, bpf_rbtree_first) > #ifdef CONFIG_NET > BTF_ID(func, bpf_dynptr_from_skb) > +BTF_ID(func, bpf_dynptr_from_skb_rdonly) > BTF_ID(func, bpf_dynptr_from_xdp) > #endif > BTF_ID(func, bpf_dynptr_slice) > @@ -11816,10 +11818,12 @@ BTF_ID(func, bpf_rbtree_add_impl) > BTF_ID(func, bpf_rbtree_first) > #ifdef CONFIG_NET > BTF_ID(func, bpf_dynptr_from_skb) > +BTF_ID(func, bpf_dynptr_from_skb_rdonly) > BTF_ID(func, bpf_dynptr_from_xdp) > #else > BTF_ID_UNUSED > BTF_ID_UNUSED > +BTF_ID_UNUSED > #endif > BTF_ID(func, bpf_dynptr_slice) > BTF_ID(func, bpf_dynptr_slice_rdwr) > @@ -12741,7 +12745,8 @@ static int check_kfunc_args(struct bpf_verifier_env *env, struct bpf_kfunc_call_ > if (is_kfunc_arg_uninit(btf, &args[i])) > dynptr_arg_type |= MEM_UNINIT; > > - if (meta->func_id == special_kfunc_list[KF_bpf_dynptr_from_skb]) { > + if (meta->func_id == special_kfunc_list[KF_bpf_dynptr_from_skb] || > + meta->func_id == special_kfunc_list[KF_bpf_dynptr_from_skb_rdonly]) { > dynptr_arg_type |= DYNPTR_TYPE_SKB; > } else if (meta->func_id == special_kfunc_list[KF_bpf_dynptr_from_xdp]) { > dynptr_arg_type |= DYNPTR_TYPE_XDP; > @@ -20898,9 +20903,7 @@ static void specialize_kfunc(struct bpf_verifier_env *env, > u32 func_id, u16 offset, unsigned long *addr) > { > struct bpf_prog *prog = env->prog; > - bool seen_direct_write; > void *xdp_kfunc; > - bool is_rdonly; > > if (bpf_dev_bound_kfunc_id(func_id)) { > xdp_kfunc = bpf_dev_bound_resolve_kfunc(prog, func_id); > @@ -20910,22 +20913,6 @@ static void specialize_kfunc(struct bpf_verifier_env *env, > } > /* fallback to default kfunc when not supported by netdev */ > } > - > - if (offset) > - return; > - > - if (func_id == special_kfunc_list[KF_bpf_dynptr_from_skb]) { > - seen_direct_write = env->seen_direct_write; > - is_rdonly = !may_access_direct_pkt_data(env, NULL, BPF_WRITE); > - > - if (is_rdonly) > - *addr = (unsigned long)bpf_dynptr_from_skb_rdonly; > - > - /* restore env->seen_direct_write to its original value, since > - * may_access_direct_pkt_data mutates it > - */ > - env->seen_direct_write = seen_direct_write; > - } > } > > static void __fixup_collection_insert_kfunc(struct bpf_insn_aux_data *insn_aux, > diff --git a/net/core/filter.c b/net/core/filter.c > index 2ec162dd83c4..6416689e3976 100644 > --- a/net/core/filter.c > +++ b/net/core/filter.c > @@ -12062,10 +12062,8 @@ __bpf_kfunc int bpf_sk_assign_tcp_reqsk(struct __sk_buff *s, struct sock *sk, > #endif > } > > -__bpf_kfunc_end_defs(); > - > -int bpf_dynptr_from_skb_rdonly(struct __sk_buff *skb, u64 flags, > - struct bpf_dynptr *ptr__uninit) > +__bpf_kfunc int bpf_dynptr_from_skb_rdonly(struct __sk_buff *skb, u64 flags, > + struct bpf_dynptr *ptr__uninit) > { > struct bpf_dynptr_kern *ptr = (struct bpf_dynptr_kern *)ptr__uninit; > int err; > @@ -12079,10 +12077,16 @@ int bpf_dynptr_from_skb_rdonly(struct __sk_buff *skb, u64 flags, > return 0; > } > > +__bpf_kfunc_end_defs(); > + > BTF_KFUNCS_START(bpf_kfunc_check_set_skb) > BTF_ID_FLAGS(func, bpf_dynptr_from_skb, KF_TRUSTED_ARGS) > BTF_KFUNCS_END(bpf_kfunc_check_set_skb) > > +BTF_HIDDEN_KFUNCS_START(bpf_kfunc_check_hidden_set_skb) > +BTF_ID_FLAGS(func, bpf_dynptr_from_skb_rdonly, KF_TRUSTED_ARGS) > +BTF_KFUNCS_END(bpf_kfunc_check_hidden_set_skb) > + > BTF_KFUNCS_START(bpf_kfunc_check_set_xdp) > BTF_ID_FLAGS(func, bpf_dynptr_from_xdp) > BTF_KFUNCS_END(bpf_kfunc_check_set_xdp) > @@ -12095,9 +12099,46 @@ BTF_KFUNCS_START(bpf_kfunc_check_set_tcp_reqsk) > BTF_ID_FLAGS(func, bpf_sk_assign_tcp_reqsk, KF_TRUSTED_ARGS) > BTF_KFUNCS_END(bpf_kfunc_check_set_tcp_reqsk) > > +BTF_ID_LIST(bpf_dynptr_from_skb_list) > +BTF_ID(func, bpf_dynptr_from_skb) > +BTF_ID(func, bpf_dynptr_from_skb_rdonly) > + > +static u32 bpf_kfunc_set_skb_remap(const struct bpf_prog *prog, u32 kfunc_id) > +{ > + if (kfunc_id != bpf_dynptr_from_skb_list[0]) > + return 0; > + > + switch (resolve_prog_type(prog)) { > + /* Program types only with direct read access go here! */ > + case BPF_PROG_TYPE_LWT_IN: > + case BPF_PROG_TYPE_LWT_OUT: > + case BPF_PROG_TYPE_LWT_SEG6LOCAL: > + case BPF_PROG_TYPE_SK_REUSEPORT: > + case BPF_PROG_TYPE_FLOW_DISSECTOR: > + case BPF_PROG_TYPE_CGROUP_SKB: This copy pastes the logic from may_access_direct_pkt_data(), so any future change to that helper would need to update this one as well. > + return bpf_dynptr_from_skb_list[1]; The [0] and [1] stuff is quite error prone. > + > + /* Program types with direct read + write access go here! */ > + case BPF_PROG_TYPE_SCHED_CLS: > + case BPF_PROG_TYPE_SCHED_ACT: > + case BPF_PROG_TYPE_XDP: > + case BPF_PROG_TYPE_LWT_XMIT: > + case BPF_PROG_TYPE_SK_SKB: > + case BPF_PROG_TYPE_SK_MSG: > + case BPF_PROG_TYPE_CGROUP_SOCKOPT: > + return kfunc_id; > + > + default: > + break; > + } > + return bpf_dynptr_from_skb_list[1]; > +} > + > static const struct btf_kfunc_id_set bpf_kfunc_set_skb = { > .owner = THIS_MODULE, > .set = &bpf_kfunc_check_set_skb, > + .hidden_set = &bpf_kfunc_check_hidden_set_skb, If I'm reading it correctly the hidden_set serves no additional purpose. It splits the set into two, but patch 4 just adds them together. > + .remap = &bpf_kfunc_set_skb_remap, I'm not a fan of callbacks in general. The makes everything harder to follow. For all these reasons I don't like this approach. This "generality" doesn't make it cleaner or easier to extend. For the patch 6... just repeat what specialize_kfunc() currently does for dynptr ? pw-bot: cr
Hi Alexei, Thanks for the review! > On Jan 29, 2025, at 6:32 PM, Alexei Starovoitov <alexei.starovoitov@gmail.com> wrote: [...] >> >> +BTF_ID_LIST(bpf_dynptr_from_skb_list) >> +BTF_ID(func, bpf_dynptr_from_skb) >> +BTF_ID(func, bpf_dynptr_from_skb_rdonly) >> + >> +static u32 bpf_kfunc_set_skb_remap(const struct bpf_prog *prog, u32 kfunc_id) >> +{ >> + if (kfunc_id != bpf_dynptr_from_skb_list[0]) >> + return 0; >> + >> + switch (resolve_prog_type(prog)) { >> + /* Program types only with direct read access go here! */ >> + case BPF_PROG_TYPE_LWT_IN: >> + case BPF_PROG_TYPE_LWT_OUT: >> + case BPF_PROG_TYPE_LWT_SEG6LOCAL: >> + case BPF_PROG_TYPE_SK_REUSEPORT: >> + case BPF_PROG_TYPE_FLOW_DISSECTOR: >> + case BPF_PROG_TYPE_CGROUP_SKB: > > This copy pastes the logic from may_access_direct_pkt_data(), > so any future change to that helper would need to update > this one as well. We can probably improve this with some helpers/macros. > >> + return bpf_dynptr_from_skb_list[1]; > > The [0] and [1] stuff is quite error prone. > >> + >> + /* Program types with direct read + write access go here! */ >> + case BPF_PROG_TYPE_SCHED_CLS: >> + case BPF_PROG_TYPE_SCHED_ACT: >> + case BPF_PROG_TYPE_XDP: >> + case BPF_PROG_TYPE_LWT_XMIT: >> + case BPF_PROG_TYPE_SK_SKB: >> + case BPF_PROG_TYPE_SK_MSG: >> + case BPF_PROG_TYPE_CGROUP_SOCKOPT: >> + return kfunc_id; >> + >> + default: >> + break; >> + } >> + return bpf_dynptr_from_skb_list[1]; >> +} >> + >> static const struct btf_kfunc_id_set bpf_kfunc_set_skb = { >> .owner = THIS_MODULE, >> .set = &bpf_kfunc_check_set_skb, >> + .hidden_set = &bpf_kfunc_check_hidden_set_skb, > > If I'm reading it correctly the hidden_set serves no additional purpose. > It splits the set into two, but patch 4 just adds them together. hidden_set does not have BTF_SET8_KFUNCS, so pahole will not export these kfuncs to vmlinux.h. > >> + .remap = &bpf_kfunc_set_skb_remap, > > I'm not a fan of callbacks in general. > The makes everything harder to follow. This motivation here is to move polymorphism logic from verifier core to kfuncs owners. I guess we will need some callback to achieve this goal. Of course, we don't have to do it in this set. > For all these reasons I don't like this approach. > This "generality" doesn't make it cleaner or easier to extend. > For the patch 6... just repeat what specialize_kfunc() > currently does for dynptr ? Yes, specialize_kfunc() can handle this. But we will need to use d_inode_locked_hooks from 6/7 in specialize_kfunc(). It works, but it is not clean (to me). I will revise this set so that the polymorphism logic in handled in specialize_kfunc(). For longer term, maybe we should discuss "move some logic from verifier core to kfuncs" in the upcoming LSF/MM/BPF? Thanks, Song
On Thu, Jan 30, 2025 at 9:49 AM Song Liu <songliubraving@meta.com> wrote: > > Hi Alexei, > > Thanks for the review! > > > On Jan 29, 2025, at 6:32 PM, Alexei Starovoitov <alexei.starovoitov@gmail.com> wrote: > > [...] > > >> > >> +BTF_ID_LIST(bpf_dynptr_from_skb_list) > >> +BTF_ID(func, bpf_dynptr_from_skb) > >> +BTF_ID(func, bpf_dynptr_from_skb_rdonly) > >> + > >> +static u32 bpf_kfunc_set_skb_remap(const struct bpf_prog *prog, u32 kfunc_id) > >> +{ > >> + if (kfunc_id != bpf_dynptr_from_skb_list[0]) > >> + return 0; > >> + > >> + switch (resolve_prog_type(prog)) { > >> + /* Program types only with direct read access go here! */ > >> + case BPF_PROG_TYPE_LWT_IN: > >> + case BPF_PROG_TYPE_LWT_OUT: > >> + case BPF_PROG_TYPE_LWT_SEG6LOCAL: > >> + case BPF_PROG_TYPE_SK_REUSEPORT: > >> + case BPF_PROG_TYPE_FLOW_DISSECTOR: > >> + case BPF_PROG_TYPE_CGROUP_SKB: > > > > This copy pastes the logic from may_access_direct_pkt_data(), > > so any future change to that helper would need to update > > this one as well. > > We can probably improve this with some helpers/macros. > > > > >> + return bpf_dynptr_from_skb_list[1]; > > > > The [0] and [1] stuff is quite error prone. > > > >> + > >> + /* Program types with direct read + write access go here! */ > >> + case BPF_PROG_TYPE_SCHED_CLS: > >> + case BPF_PROG_TYPE_SCHED_ACT: > >> + case BPF_PROG_TYPE_XDP: > >> + case BPF_PROG_TYPE_LWT_XMIT: > >> + case BPF_PROG_TYPE_SK_SKB: > >> + case BPF_PROG_TYPE_SK_MSG: > >> + case BPF_PROG_TYPE_CGROUP_SOCKOPT: > >> + return kfunc_id; > >> + > >> + default: > >> + break; > >> + } > >> + return bpf_dynptr_from_skb_list[1]; > >> +} > >> + > >> static const struct btf_kfunc_id_set bpf_kfunc_set_skb = { > >> .owner = THIS_MODULE, > >> .set = &bpf_kfunc_check_set_skb, > >> + .hidden_set = &bpf_kfunc_check_hidden_set_skb, > > > > If I'm reading it correctly the hidden_set serves no additional purpose. > > It splits the set into two, but patch 4 just adds them together. > > hidden_set does not have BTF_SET8_KFUNCS, so pahole will not export > these kfuncs to vmlinux.h. > > > > >> + .remap = &bpf_kfunc_set_skb_remap, > > > > I'm not a fan of callbacks in general. > > The makes everything harder to follow. > > This motivation here is to move polymorphism logic from verifier > core to kfuncs owners. I guess we will need some callback to > achieve this goal. Of course, we don't have to do it in this set. > > > > For all these reasons I don't like this approach. > > This "generality" doesn't make it cleaner or easier to extend. > > For the patch 6... just repeat what specialize_kfunc() > > currently does for dynptr ? > > Yes, specialize_kfunc() can handle this. But we will need to use > d_inode_locked_hooks from 6/7 in specialize_kfunc(). It works, > but it is not clean (to me). I'm missing why that would be necessary to cross the layers so much. I guess the code will tell. Pls send an rfc to illustrate the unclean part. > I will revise this set so that the polymorphism logic in handled > in specialize_kfunc(). For longer term, maybe we should discuss > "move some logic from verifier core to kfuncs" in the upcoming > LSF/MM/BPF? imo such topic is too narrow and detail oriented. There is not much to gain from discussing it at lsfmm. email works well for such discussions.
> On Jan 30, 2025, at 12:23 PM, Alexei Starovoitov <alexei.starovoitov@gmail.com> wrote: [...] >>> For all these reasons I don't like this approach. >>> This "generality" doesn't make it cleaner or easier to extend. >>> For the patch 6... just repeat what specialize_kfunc() >>> currently does for dynptr ? >> >> Yes, specialize_kfunc() can handle this. But we will need to use >> d_inode_locked_hooks from 6/7 in specialize_kfunc(). It works, >> but it is not clean (to me). > > I'm missing why that would be necessary to cross the layers > so much. I guess the code will tell. > Pls send an rfc to illustrate the unclean part. The actual code is actually a lot cleaner than I thought. We just need to use the bpf_lsm_has_d_inode_locked() helper in verifier.c. Thanks, Song > >> I will revise this set so that the polymorphism logic in handled >> in specialize_kfunc(). For longer term, maybe we should discuss >> "move some logic from verifier core to kfuncs" in the upcoming >> LSF/MM/BPF? > > imo such topic is too narrow and detail oriented. > There is not much to gain from discussing it at lsfmm. > email works well for such discussions.
diff --git a/kernel/bpf/verifier.c b/kernel/bpf/verifier.c index 2188b6674266..55e710e318e5 100644 --- a/kernel/bpf/verifier.c +++ b/kernel/bpf/verifier.c @@ -11750,6 +11750,7 @@ enum special_kfunc_type { KF_bpf_rbtree_add_impl, KF_bpf_rbtree_first, KF_bpf_dynptr_from_skb, + KF_bpf_dynptr_from_skb_rdonly, KF_bpf_dynptr_from_xdp, KF_bpf_dynptr_slice, KF_bpf_dynptr_slice_rdwr, @@ -11785,6 +11786,7 @@ BTF_ID(func, bpf_rbtree_add_impl) BTF_ID(func, bpf_rbtree_first) #ifdef CONFIG_NET BTF_ID(func, bpf_dynptr_from_skb) +BTF_ID(func, bpf_dynptr_from_skb_rdonly) BTF_ID(func, bpf_dynptr_from_xdp) #endif BTF_ID(func, bpf_dynptr_slice) @@ -11816,10 +11818,12 @@ BTF_ID(func, bpf_rbtree_add_impl) BTF_ID(func, bpf_rbtree_first) #ifdef CONFIG_NET BTF_ID(func, bpf_dynptr_from_skb) +BTF_ID(func, bpf_dynptr_from_skb_rdonly) BTF_ID(func, bpf_dynptr_from_xdp) #else BTF_ID_UNUSED BTF_ID_UNUSED +BTF_ID_UNUSED #endif BTF_ID(func, bpf_dynptr_slice) BTF_ID(func, bpf_dynptr_slice_rdwr) @@ -12741,7 +12745,8 @@ static int check_kfunc_args(struct bpf_verifier_env *env, struct bpf_kfunc_call_ if (is_kfunc_arg_uninit(btf, &args[i])) dynptr_arg_type |= MEM_UNINIT; - if (meta->func_id == special_kfunc_list[KF_bpf_dynptr_from_skb]) { + if (meta->func_id == special_kfunc_list[KF_bpf_dynptr_from_skb] || + meta->func_id == special_kfunc_list[KF_bpf_dynptr_from_skb_rdonly]) { dynptr_arg_type |= DYNPTR_TYPE_SKB; } else if (meta->func_id == special_kfunc_list[KF_bpf_dynptr_from_xdp]) { dynptr_arg_type |= DYNPTR_TYPE_XDP; @@ -20898,9 +20903,7 @@ static void specialize_kfunc(struct bpf_verifier_env *env, u32 func_id, u16 offset, unsigned long *addr) { struct bpf_prog *prog = env->prog; - bool seen_direct_write; void *xdp_kfunc; - bool is_rdonly; if (bpf_dev_bound_kfunc_id(func_id)) { xdp_kfunc = bpf_dev_bound_resolve_kfunc(prog, func_id); @@ -20910,22 +20913,6 @@ static void specialize_kfunc(struct bpf_verifier_env *env, } /* fallback to default kfunc when not supported by netdev */ } - - if (offset) - return; - - if (func_id == special_kfunc_list[KF_bpf_dynptr_from_skb]) { - seen_direct_write = env->seen_direct_write; - is_rdonly = !may_access_direct_pkt_data(env, NULL, BPF_WRITE); - - if (is_rdonly) - *addr = (unsigned long)bpf_dynptr_from_skb_rdonly; - - /* restore env->seen_direct_write to its original value, since - * may_access_direct_pkt_data mutates it - */ - env->seen_direct_write = seen_direct_write; - } } static void __fixup_collection_insert_kfunc(struct bpf_insn_aux_data *insn_aux, diff --git a/net/core/filter.c b/net/core/filter.c index 2ec162dd83c4..6416689e3976 100644 --- a/net/core/filter.c +++ b/net/core/filter.c @@ -12062,10 +12062,8 @@ __bpf_kfunc int bpf_sk_assign_tcp_reqsk(struct __sk_buff *s, struct sock *sk, #endif } -__bpf_kfunc_end_defs(); - -int bpf_dynptr_from_skb_rdonly(struct __sk_buff *skb, u64 flags, - struct bpf_dynptr *ptr__uninit) +__bpf_kfunc int bpf_dynptr_from_skb_rdonly(struct __sk_buff *skb, u64 flags, + struct bpf_dynptr *ptr__uninit) { struct bpf_dynptr_kern *ptr = (struct bpf_dynptr_kern *)ptr__uninit; int err; @@ -12079,10 +12077,16 @@ int bpf_dynptr_from_skb_rdonly(struct __sk_buff *skb, u64 flags, return 0; } +__bpf_kfunc_end_defs(); + BTF_KFUNCS_START(bpf_kfunc_check_set_skb) BTF_ID_FLAGS(func, bpf_dynptr_from_skb, KF_TRUSTED_ARGS) BTF_KFUNCS_END(bpf_kfunc_check_set_skb) +BTF_HIDDEN_KFUNCS_START(bpf_kfunc_check_hidden_set_skb) +BTF_ID_FLAGS(func, bpf_dynptr_from_skb_rdonly, KF_TRUSTED_ARGS) +BTF_KFUNCS_END(bpf_kfunc_check_hidden_set_skb) + BTF_KFUNCS_START(bpf_kfunc_check_set_xdp) BTF_ID_FLAGS(func, bpf_dynptr_from_xdp) BTF_KFUNCS_END(bpf_kfunc_check_set_xdp) @@ -12095,9 +12099,46 @@ BTF_KFUNCS_START(bpf_kfunc_check_set_tcp_reqsk) BTF_ID_FLAGS(func, bpf_sk_assign_tcp_reqsk, KF_TRUSTED_ARGS) BTF_KFUNCS_END(bpf_kfunc_check_set_tcp_reqsk) +BTF_ID_LIST(bpf_dynptr_from_skb_list) +BTF_ID(func, bpf_dynptr_from_skb) +BTF_ID(func, bpf_dynptr_from_skb_rdonly) + +static u32 bpf_kfunc_set_skb_remap(const struct bpf_prog *prog, u32 kfunc_id) +{ + if (kfunc_id != bpf_dynptr_from_skb_list[0]) + return 0; + + switch (resolve_prog_type(prog)) { + /* Program types only with direct read access go here! */ + case BPF_PROG_TYPE_LWT_IN: + case BPF_PROG_TYPE_LWT_OUT: + case BPF_PROG_TYPE_LWT_SEG6LOCAL: + case BPF_PROG_TYPE_SK_REUSEPORT: + case BPF_PROG_TYPE_FLOW_DISSECTOR: + case BPF_PROG_TYPE_CGROUP_SKB: + return bpf_dynptr_from_skb_list[1]; + + /* Program types with direct read + write access go here! */ + case BPF_PROG_TYPE_SCHED_CLS: + case BPF_PROG_TYPE_SCHED_ACT: + case BPF_PROG_TYPE_XDP: + case BPF_PROG_TYPE_LWT_XMIT: + case BPF_PROG_TYPE_SK_SKB: + case BPF_PROG_TYPE_SK_MSG: + case BPF_PROG_TYPE_CGROUP_SOCKOPT: + return kfunc_id; + + default: + break; + } + return bpf_dynptr_from_skb_list[1]; +} + static const struct btf_kfunc_id_set bpf_kfunc_set_skb = { .owner = THIS_MODULE, .set = &bpf_kfunc_check_set_skb, + .hidden_set = &bpf_kfunc_check_hidden_set_skb, + .remap = &bpf_kfunc_set_skb_remap, }; static const struct btf_kfunc_id_set bpf_kfunc_set_xdp = {
btf_kfunc_id_set.remap can pick proper version of a kfunc for the calling context. Use this logic to select bpf_dynptr_from_skb or bpf_dynptr_from_skb_rdonly. This will make the verifier simpler. Unfortunately, btf_kfunc_id_set.remap cannot cover the DYNPTR_TYPE_SKB logic in check_kfunc_args(). This can be addressed later. Signed-off-by: Song Liu <song@kernel.org> --- kernel/bpf/verifier.c | 25 ++++++---------------- net/core/filter.c | 49 +++++++++++++++++++++++++++++++++++++++---- 2 files changed, 51 insertions(+), 23 deletions(-)