Re: [PATCH RFC bpf-next 2/4] bpf: support BPF ksym variables in kernel modules

[Alexei Starovoitov <alexei.starovoitov@gmail.com>]

On Thu, Dec 10, 2020 at 08:27:32PM -0800, Andrii Nakryiko wrote:
> During BPF program load time, verifier will resolve FD to BTF object and will
> take reference on BTF object itself and, for module BTFs, corresponding module
> as well, to make sure it won't be unloaded from under running BPF program. The
> mechanism used is similar to how bpf_prog keeps track of used bpf_maps.
...
> +
> +	/* if we reference variables from kernel module, bump its refcount */
> +	if (btf_is_module(btf)) {
> +		btf_mod->module = btf_try_get_module(btf);

Is it necessary to refcnt the module? Correct me if I'm wrong, but
for module's BTF we register a notifier. Then the module can be rmmod-ed
at any time and we will do btf_put() for corresponding BTF, but that BTF may
stay around because bpftool or something is looking at it.
Similarly when prog is attached to raw_tp in a module we currently do try_module_get(),
but is it really necessary ? When bpf is attached to a netdev the netdev can
be removed and the link will be dangling. May be it makes sense to do the same
with modules?  The raw_tp can become dangling after rmmod and the prog won't be
executed anymore. So hard coded address of a per-cpu var in a ksym will
be pointing to freed mod memory after rmmod, but that's ok, since that prog will
never execute.
On the other side if we envision a bpf prog attaching to a vmlinux function
and accessing per-cpu or normal ksym in some module it would need to inc refcnt
of that module, since we won't be able to guarantee that this prog will
not execute any more. So we cannot allow dangling memory addresses.
If latter is what we want to allow then we probably need a test case for it and
document the reasons for keeping modules pinned while progs access their data.
Since such pinning behavior is different from other bpf attaching cases where
underlying objects (like netdev and cgroup) can go away.