Re: [PATCH bpf-next v3 1/3] bpf: add destructive kfunc flag

[Artem Savkov]

On Mon, Aug 08, 2022 at 02:14:33PM +0200, Kumar Kartikeya Dwivedi wrote:
> On Mon, 8 Aug 2022 at 11:48, Artem Savkov <asavkov@xxxxxxxxxx> wrote:
> >
> > Add KF_DESTRUCTIVE flag for destructive functions. Functions with this
> > flag set will require CAP_SYS_BOOT capabilities.
> >
> > Signed-off-by: Artem Savkov <asavkov@xxxxxxxxxx>
> > ---
> >  include/linux/btf.h   | 1 +
> >  kernel/bpf/verifier.c | 5 +++++
> >  2 files changed, 6 insertions(+)
> >
> > diff --git a/include/linux/btf.h b/include/linux/btf.h
> > index cdb376d53238..51a0961c84e3 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_DESTRUCTIVE  (1 << 5) /* kfunc performs destructive actions */
> >
> 
> Please also document this flag in Documentation/bpf/kfuncs.rst.

Ok, will do.

> And maybe instead of KF_DESTRUCTIVE, it might be more apt to call this
> KF_CAP_SYS_BOOT. While it is true you had a destructive flag for
> programs being loaded earlier, so there was a mapping between the two
> UAPI and kfunc flags, what it has boiled down to is that this flag
> just requires CAP_SYS_BOOT (in addition to other capabilities) during
> load. So that name might express the intent a bit better. We might
> soon have similar flags encoding requirements of other capabilities on
> load.
> 
> The flag rename is just a suggestion, up to you.

This makes sense right now, but if going forward we'll add stricter
signing requirements or other prerequisites we'll either have to rename
the flag back, or add those as separate flags. I guess the decision here
depends on whether some of non-destructive bpf programs might ever require
CAP_SYS_BOOT capabilities or not.

-- 
 Artem