Re: [PATCH v9 06/10] bpf: Add bpf_lookup_*_key() and bpf_key_put() kfuncs

[Daniel Borkmann]

[...]
> > > +noinline __weak struct bpf_key *bpf_lookup_user_key(u32 serial, u64 flags)
> >
> > Why the need for noinline and the __weak here and below? (If indeed needed
> > this
> > should probably be explained in the commit desc.)
>
> Oh, I took from v3 of KP's patch set. It is gone in v5. Will do
> the same as well.
>
> > > +{
> > > +	key_ref_t key_ref;
> > > +	struct bpf_key *bkey;
> > > +
> > > +	/* Keep in sync with include/linux/key.h. */
> > > +	if (flags > (KEY_LOOKUP_PARTIAL << 1) - 1)
> >
> > Can't we just simplify and test flags &
> > ~(KEY_LOOKUP_CREATE|KEY_LOOKUP_PARTIAL)?
>
> I thought as if we have many flags.

I'd keep it simple for now, and if the actual need comes this can still be changed.

> > > +		return NULL;
> > > +
> > > +	/* Permission check is deferred until actual kfunc using the key. */
> > > +	key_ref = lookup_user_key(serial, flags, KEY_DEFER_PERM_CHECK);
> > > +	if (IS_ERR(key_ref))
> > > +		return NULL;
> > > +
> > > +	bkey = kmalloc(sizeof(*bkey), GFP_KERNEL);
> > > +	if (!bkey) {
> > > +		key_put(key_ref_to_ptr(key_ref));
> > > +		return bkey;
> >
> > nit: just return NULL probably cleaner
>
> Ok.