On Wed, Feb 21, 2024 at 8:25 AM Benjamin Tissoires <bentiss@xxxxxxxxxx> wrote: > > In this patch, bpf_timer_set_sleepable_cb() is functionally equivalent > to bpf_timer_set_callback(), to the exception that it enforces > the timer to be started with BPF_F_TIMER_SLEEPABLE. > > But given that bpf_timer_set_callback() is a helper when > bpf_timer_set_sleepable_cb() is a kfunc, we need to teach the verifier > about its attached callback. > Marking that callback as sleepable will be done in a separate patch > > Signed-off-by: Benjamin Tissoires <bentiss@xxxxxxxxxx> > > --- > > new in v3 (split from v2 02/10) > --- > kernel/bpf/helpers.c | 48 ++++++++++++++++++++++++++++++++++++++++++++++-- > kernel/bpf/verifier.c | 31 +++++++++++++++++++++++++++++-- > 2 files changed, 75 insertions(+), 4 deletions(-) > > diff --git a/kernel/bpf/helpers.c b/kernel/bpf/helpers.c > index f9add0abe40a..2c6dc3d0ffff 100644 > --- a/kernel/bpf/helpers.c > +++ b/kernel/bpf/helpers.c > @@ -1108,6 +1108,7 @@ struct bpf_hrtimer { > void __rcu *callback_fn; > void *value; > struct semaphore sleepable_lock; > + bool is_sleepable; > }; > > /* the actual struct hidden inside uapi struct bpf_timer */ > @@ -1270,8 +1271,8 @@ static const struct bpf_func_proto bpf_timer_init_proto = { > .arg3_type = ARG_ANYTHING, > }; > > -BPF_CALL_3(bpf_timer_set_callback, struct bpf_timer_kern *, timer, void *, callback_fn, > - struct bpf_prog_aux *, aux) > +static int __bpf_timer_set_callback(struct bpf_timer_kern *timer, void *callback_fn, > + struct bpf_prog_aux *aux, bool is_sleepable) > { > struct bpf_prog *prev, *prog = aux->prog; > struct bpf_hrtimer *t; > @@ -1311,12 +1312,19 @@ BPF_CALL_3(bpf_timer_set_callback, struct bpf_timer_kern *, timer, void *, callb > t->prog = prog; > } > rcu_assign_pointer(t->callback_fn, callback_fn); > + t->is_sleepable = is_sleepable; > up(&t->sleepable_lock); > out: > __bpf_spin_unlock_irqrestore(&timer->lock); > return ret; > } > > +BPF_CALL_3(bpf_timer_set_callback, struct bpf_timer_kern *, timer, void *, callback_fn, > + struct bpf_prog_aux *, aux) > +{ > + return __bpf_timer_set_callback(timer, callback_fn, aux, false); > +} > + > static const struct bpf_func_proto bpf_timer_set_callback_proto = { > .func = bpf_timer_set_callback, > .gpl_only = true, > @@ -1342,6 +1350,11 @@ BPF_CALL_3(bpf_timer_start, struct bpf_timer_kern *, timer, u64, nsecs, u64, fla > goto out; > } > > + if (t->is_sleepable && !(flags & BPF_F_TIMER_SLEEPABLE)) { > + ret = -EINVAL; > + goto out; > + } > + > if (flags & BPF_F_TIMER_ABS) > mode = HRTIMER_MODE_ABS_SOFT; > else > @@ -2606,6 +2619,36 @@ __bpf_kfunc void bpf_throw(u64 cookie) > WARN(1, "A call to BPF exception callback should never return\n"); > } > > +/** > + * bpf_timer_set_sleepable_cb() - Configure the timer to call %callback_fn > + * static function in a sleepable context. > + * @timer: The bpf_timer that needs to be configured > + * @callback_fn: a static bpf function > + * > + * @returns %0 on success. %-EINVAL if %timer was not initialized with > + * bpf_timer_init() earlier. %-EPERM if %timer is in a map that doesn't > + * have any user references. > + * The user space should either hold a file descriptor to a map with timers > + * or pin such map in bpffs. When map is unpinned or file descriptor is > + * closed all timers in the map will be cancelled and freed. > + * > + * This kfunc is equivalent to %bpf_timer_set_callback except that it tells > + * the verifier that the target callback is run in a sleepable context. > + */ > +__bpf_kfunc int bpf_timer_set_sleepable_cb(struct bpf_timer_kern *timer, > + int (callback_fn)(void *map, int *key, struct bpf_timer *timer)) > +{ > + struct bpf_throw_ctx ctx = {}; > + > + arch_bpf_stack_walk(bpf_stack_walker, &ctx); > + WARN_ON_ONCE(!ctx.aux); Sorry. Why such complexity? Please see how do_misc_fixups() handles BPF_FUNC_timer_set_callback.