Re: [PATCH] bpf: Drop disabled LSM hooks from the sleepable set

[Mikko Ylinen]

On Sat, Jan 23, 2021 at 12:50:21AM +0100, KP Singh wrote:
> On Fri, Jan 22, 2021 at 11:33 PM KP Singh <kpsingh@xxxxxxxxxx> wrote:
> >
> > On Fri, Jan 22, 2021 at 1:32 PM Mikko Ylinen
> > <mikko.ylinen@xxxxxxxxxxxxxxx> wrote:
> > >
> > > Networking LSM hooks are conditionally enabled and when building the new
> > > sleepable BPF LSM hooks with the networking LSM hooks disabled, the
> > > following build error occurs:
> > >
> > > BTFIDS  vmlinux
> > > FAILED unresolved symbol bpf_lsm_socket_socketpair
> > >
> > > To fix the error, conditionally add the networking LSM hooks to the
> > > sleepable set.
> > >
> > > Fixes: 423f16108c9d8 ("bpf: Augment the set of sleepable LSM hooks")
> > > Signed-off-by: Mikko Ylinen <mikko.ylinen@xxxxxxxxxxxxxxx>
> >
> > Thanks!
> >
> > Acked-by: KP Singh <kpsingh@xxxxxxxxxx>
> 
> Btw, I was noticing that there's another hook that is surrounded by ifdefs:
> 
> diff --git a/kernel/bpf/bpf_lsm.c b/kernel/bpf/bpf_lsm.c
> index 70e5e0b6d69d..f7f7754e938d 100644
> --- a/kernel/bpf/bpf_lsm.c
> +++ b/kernel/bpf/bpf_lsm.c
> @@ -166,7 +166,11 @@ BTF_ID(func, bpf_lsm_inode_symlink)
>  BTF_ID(func, bpf_lsm_inode_unlink)
>  BTF_ID(func, bpf_lsm_kernel_module_request)
>  BTF_ID(func, bpf_lsm_kernfs_init_security)
> +
> +#ifdef CONFIG_KEYS
>  BTF_ID(func, bpf_lsm_key_free)
> +#endif
> +
>  BTF_ID(func, bpf_lsm_mmap_file)
>  BTF_ID(func, bpf_lsm_netlink_send)
>  BTF_ID(func, bpf_lsm_path_notify)
> 
> It would be great if you can also add this to your patch :)

Thanks for noticing! I cross-checked the sleepable set but somehow
missed this. Just posted v2.

> I guess the cleanest solution to never let this happen would be to
> incorporate this in
> lsm_hook_defs.h and mark hooks as SLEEPABLE and NON_SLEEPABLE with an
> extra parameter to the LSM_HOOK macro and then only generate the BTF IDs
> based on this macro parameter.

Agree, a way to get the set automatically created makes sense. But the
extra parameter to LSM_HOOK macro would be BPF specific, right?

-- Regards, Mikko