On Wed, Aug 10, 2022 at 7:26 AM Benjamin Tissoires <benjamin.tissoires@xxxxxxxxxx> wrote: > > On Wed, Aug 10, 2022 at 3:44 PM Roberto Sassu <roberto.sassu@xxxxxxxxxx> wrote: > > > > > From: Jarkko Sakkinen [mailto:jarkko@xxxxxxxxxx] > > > Sent: Tuesday, August 9, 2022 6:55 PM > > > On Tue, Aug 09, 2022 at 03:45:54PM +0200, Roberto Sassu wrote: > > > > From: Benjamin Tissoires <benjamin.tissoires@xxxxxxxxxx> > > > > > > > > From: Benjamin Tissoires <benjamin.tissoires@xxxxxxxxxx> > > > > > > > > This allows to declare a kfunc as sleepable and prevents its use in > > > > a non sleepable program. > > > > > > Nit: "Declare a kfunc as sleepable and prevent its use in a > > > non-sleepable program." > > > > > > It's missing the part *how* the patch accomplishes its goals. > > > > I will add: > > > > If an eBPF program is going to call a kfunc declared as sleepable, > > eBPF will look at the eBPF program flags. If BPF_F_SLEEPABLE is > > not set, execution of that program is denied. > > All those changes are looking good to me. > > Thanks a lot for keeping pushing on this patch :) Multiple series other than the HID one started resending your patch once you dropped it, it's obviously needed :) Thanks for sending it in the first place :) > > Cheers, > Benjamin > > > > > Roberto > > > > > > Signed-off-by: Benjamin Tissoires <benjamin.tissoires@xxxxxxxxxx> > > > > Co-developed-by: Yosry Ahmed <yosryahmed@xxxxxxxxxx> > > > > Signed-off-by: Yosry Ahmed <yosryahmed@xxxxxxxxxx> > > > > Signed-off-by: Hao Luo <haoluo@xxxxxxxxxx> > > > > --- > > > > Documentation/bpf/kfuncs.rst | 6 ++++++ > > > > include/linux/btf.h | 1 + > > > > kernel/bpf/btf.c | 9 +++++++++ > > > > 3 files changed, 16 insertions(+) > > > > > > > > diff --git a/Documentation/bpf/kfuncs.rst b/Documentation/bpf/kfuncs.rst > > > > index c0b7dae6dbf5..c8b21de1c772 100644 > > > > --- a/Documentation/bpf/kfuncs.rst > > > > +++ b/Documentation/bpf/kfuncs.rst > > > > @@ -146,6 +146,12 @@ that operate (change some property, perform some > > > operation) on an object that > > > > was obtained using an acquire kfunc. Such kfuncs need an unchanged pointer > > > to > > > > ensure the integrity of the operation being performed on the expected object. > > > > > > > > +2.4.6 KF_SLEEPABLE flag > > > > +----------------------- > > > > + > > > > +The KF_SLEEPABLE flag is used for kfuncs that may sleep. Such kfuncs can > > > only > > > > +be called by sleepable BPF programs (BPF_F_SLEEPABLE). > > > > + > > > > 2.5 Registering the kfuncs > > > > -------------------------- > > > > > > > > diff --git a/include/linux/btf.h b/include/linux/btf.h > > > > index cdb376d53238..976cbdd2981f 100644 > > > > --- a/include/linux/btf.h > > > > +++ b/include/linux/btf.h > > > > @@ -49,6 +49,7 @@ > > > > * for this case. > > > > */ > > > > #define KF_TRUSTED_ARGS (1 << 4) /* kfunc only takes trusted pointer > > > arguments */ > > > > +#define KF_SLEEPABLE (1 << 5) /* kfunc may sleep */ > > > > > > > > struct btf; > > > > struct btf_member; > > > > diff --git a/kernel/bpf/btf.c b/kernel/bpf/btf.c > > > > index 7e64447659f3..d3e4c86b8fcd 100644 > > > > --- a/kernel/bpf/btf.c > > > > +++ b/kernel/bpf/btf.c > > > > @@ -6175,6 +6175,7 @@ static int btf_check_func_arg_match(struct > > > bpf_verifier_env *env, > > > > { > > > > enum bpf_prog_type prog_type = resolve_prog_type(env->prog); > > > > bool rel = false, kptr_get = false, trusted_arg = false; > > > > + bool sleepable = false; > > > > struct bpf_verifier_log *log = &env->log; > > > > u32 i, nargs, ref_id, ref_obj_id = 0; > > > > bool is_kfunc = btf_is_kernel(btf); > > > > @@ -6212,6 +6213,7 @@ static int btf_check_func_arg_match(struct > > > bpf_verifier_env *env, > > > > rel = kfunc_flags & KF_RELEASE; > > > > kptr_get = kfunc_flags & KF_KPTR_GET; > > > > trusted_arg = kfunc_flags & KF_TRUSTED_ARGS; > > > > + sleepable = kfunc_flags & KF_SLEEPABLE; > > > > } > > > > > > > > /* check that BTF function arguments match actual types that the > > > > @@ -6419,6 +6421,13 @@ static int btf_check_func_arg_match(struct > > > bpf_verifier_env *env, > > > > func_name); > > > > return -EINVAL; > > > > } > > > > + > > > > + if (sleepable && !env->prog->aux->sleepable) { > > > > + bpf_log(log, "kernel function %s is sleepable but the program is > > > not\n", > > > > + func_name); > > > > + return -EINVAL; > > > > + } > > > > + > > > > /* returns argument register number > 0 in case of reference release > > > kfunc */ > > > > return rel ? ref_regno : 0; > > > > } > > > > -- > > > > 2.25.1 > > > > > > > > > > BR, Jarkko > > >
