On Apr 08 2024, bentiss@kernel.org wrote:
From: Kumar Kartikeya Dwivedi memxor@gmail.com
Introduce support for KF_ARG_PTR_TO_TIMER. The kfuncs will use bpf_timer as argument and that will be recognized as timer argument by verifier. bpf_timer_kern casting can happen inside kfunc, but using bpf_timer in argument makes life easier for users who work with non-kern type in BPF progs.
Fix up process_timer_func's meta argument usage (ignore if NULL) so that we can share the same checks for helpers and kfuncs. meta argument is only needed to ensure bpf_timer_init's timer and map arguments are coming from the same map (map_uid logic is necessary for correct inner-map handling).
No such concerns will be necessary for kfuncs as timer initialization happens using helpers, hence pass NULL to process_timer_func from kfunc argument handling code to ignore it.
Signed-off-by: Kumar Kartikeya Dwivedi memxor@gmail.com Signed-off-by: Benjamin Tissoires bentiss@kernel.org
changes in v6:
- used Kumar's version of the patch
- reverted `+BTF_ID(struct, bpf_timer_kern)`
My bad. While working on bpf_wq I realized I shouldn't have touched this part. See below:
changes in v5:
- also check for the reg offset
changes in v4:
- enforce KF_ARG_PTR_TO_TIMER to be of type PTR_TO_MAP_VALUE
new in v3 (split from v2 02/10)
kernel/bpf/verifier.c | 25 +++++++++++++++++++++++++ 1 file changed, 25 insertions(+)
diff --git a/kernel/bpf/verifier.c b/kernel/bpf/verifier.c index ca6cacf7b42f..ccfe9057d8dc 100644 --- a/kernel/bpf/verifier.c +++ b/kernel/bpf/verifier.c @@ -7568,12 +7568,16 @@ static int process_timer_func(struct bpf_verifier_env *env, int regno, val + reg->off, map->record->timer_off); return -EINVAL; }
- /* meta is only needed for bpf_timer_init to match timer and map */
- if (!meta)
if (meta->map_ptr) { verbose(env, "verifier bug. Two map pointers in a timer helper\n"); return -EFAULT; } meta->map_uid = reg->map_uid; meta->map_ptr = map;goto out;
+out: return 0; } @@ -10826,6 +10830,7 @@ enum { KF_ARG_LIST_NODE_ID, KF_ARG_RB_ROOT_ID, KF_ARG_RB_NODE_ID,
- KF_ARG_TIMER_ID,
}; BTF_ID_LIST(kf_arg_btf_ids) @@ -10834,6 +10839,7 @@ BTF_ID(struct, bpf_list_head) BTF_ID(struct, bpf_list_node) BTF_ID(struct, bpf_rb_root) BTF_ID(struct, bpf_rb_node) +BTF_ID(struct, bpf_timer_kern)
As Kumar originally put, this should be BTF_ID(struct, bpf_timer), and he explained everything in the commit message. I was just too dumb to understand it properly.
(Adding a comment here in case we want to extend bpf_timer API in the future, and so this patch will be useful).
Cheers, Benjamin
static bool __is_kfunc_ptr_arg_type(const struct btf *btf, const struct btf_param *arg, int type) @@ -10877,6 +10883,11 @@ static bool is_kfunc_arg_rbtree_node(const struct btf *btf, const struct btf_par return __is_kfunc_ptr_arg_type(btf, arg, KF_ARG_RB_NODE_ID); } +static bool is_kfunc_arg_timer(const struct btf *btf, const struct btf_param *arg) +{
- return __is_kfunc_ptr_arg_type(btf, arg, KF_ARG_TIMER_ID);
+}
static bool is_kfunc_arg_callback(struct bpf_verifier_env *env, const struct btf *btf, const struct btf_param *arg) { @@ -10946,6 +10957,7 @@ enum kfunc_ptr_arg_type { KF_ARG_PTR_TO_NULL, KF_ARG_PTR_TO_CONST_STR, KF_ARG_PTR_TO_MAP,
- KF_ARG_PTR_TO_TIMER,
}; enum special_kfunc_type { @@ -11102,6 +11114,9 @@ get_kfunc_ptr_arg_type(struct bpf_verifier_env *env, if (is_kfunc_arg_map(meta->btf, &args[argno])) return KF_ARG_PTR_TO_MAP;
- if (is_kfunc_arg_timer(meta->btf, &args[argno]))
return KF_ARG_PTR_TO_TIMER;
- if ((base_type(reg->type) == PTR_TO_BTF_ID || reg2btf_ids[base_type(reg->type)])) { if (!btf_type_is_struct(ref_t)) { verbose(env, "kernel function %s args#%d pointer type %s %s is not supported\n",
@@ -11735,6 +11750,7 @@ static int check_kfunc_args(struct bpf_verifier_env *env, struct bpf_kfunc_call_ case KF_ARG_PTR_TO_CALLBACK: case KF_ARG_PTR_TO_REFCOUNTED_KPTR: case KF_ARG_PTR_TO_CONST_STR:
default:case KF_ARG_PTR_TO_TIMER: /* Trusted by default */ break;
@@ -12021,6 +12037,15 @@ static int check_kfunc_args(struct bpf_verifier_env *env, struct bpf_kfunc_call_ if (ret) return ret; break;
case KF_ARG_PTR_TO_TIMER:
if (reg->type != PTR_TO_MAP_VALUE) {
verbose(env, "arg#%d doesn't point to a map value\n", i);
return -EINVAL;
}
ret = process_timer_func(env, regno, NULL);
if (ret < 0)
return ret;
} }break;
-- 2.44.0