On Thu, Nov 5, 2020 at 10:44 PM Greg Kroah-Hartman
<gregkh@xxxxxxxxxxxxxxxxxxx> wrote:
>
> On Thu, Nov 05, 2020 at 09:51:09PM -0800, Andrii Nakryiko wrote:
> > Add kernel module listener that will load/validate and unload module BTF.
> > Module BTFs gets ID generated for them, which makes it possible to iterate
> > them with existing BTF iteration API. They are given their respective module's
> > names, which will get reported through GET_OBJ_INFO API. They are also marked
> > as in-kernel BTFs for tooling to distinguish them from user-provided BTFs.
> >
> > Also, similarly to vmlinux BTF, kernel module BTFs are exposed through
> > sysfs as /sys/kernel/btf/<module-name>. This is convenient for user-space
> > tools to inspect module BTF contents and dump their types with existing tools:
> >
> > [vmuser@archvm bpf]$ ls -la /sys/kernel/btf
> > total 0
> > drwxr-xr-x 2 root root 0 Nov 4 19:46 .
> > drwxr-xr-x 13 root root 0 Nov 4 19:46 ..
> >
> > ...
> >
> > -r--r--r-- 1 root root 888 Nov 4 19:46 irqbypass
> > -r--r--r-- 1 root root 100225 Nov 4 19:46 kvm
> > -r--r--r-- 1 root root 35401 Nov 4 19:46 kvm_intel
> > -r--r--r-- 1 root root 120 Nov 4 19:46 pcspkr
> > -r--r--r-- 1 root root 399 Nov 4 19:46 serio_raw
> > -r--r--r-- 1 root root 4094095 Nov 4 19:46 vmlinux
> >
> > Signed-off-by: Andrii Nakryiko <andrii@xxxxxxxxxx>
> > ---
> > Documentation/ABI/testing/sysfs-kernel-btf | 8 +
> > include/linux/bpf.h | 2 +
> > include/linux/module.h | 4 +
> > kernel/bpf/btf.c | 193 +++++++++++++++++++++
> > kernel/bpf/sysfs_btf.c | 2 +-
> > kernel/module.c | 32 ++++
> > 6 files changed, 240 insertions(+), 1 deletion(-)
> >
[...]
> > + if (IS_ENABLED(CONFIG_SYSFS)) {
> > + struct bin_attribute *attr;
> > +
> > + attr = kzalloc(sizeof(*attr), GFP_KERNEL);
> > + if (!attr) {
> > + WARN(1, "failed to register module [%s] BTF in sysfs\n", mod->name);
>
> kzalloc() will print errors on its own, no need to do this again. Also,
> for systems with panic-on-warn, you just crashed them, not nice :(
ah, pr_warn() is what I probably wanted here instead of WARN. I'll
just drop this, if kzalloc will log a warning anyways.
>
> > + goto out;
> > + }
> > +
> > + attr->attr.name = btf->name;
> > + attr->attr.mode = 0444;
> > + attr->size = btf->data_size;
> > + attr->private = btf;
> > + attr->read = btf_module_read;
> > +
> > + err = sysfs_create_bin_file(btf_kobj, attr);
>
> You forgot to call sysfs_bin_attr_init() to initialize your binary sysfs
> attribute. You'll only notice if you turn lockdep on.
Good catch, fixed. Also added CONFIG_DEBUG_LOCK_ALLOC to my config.
>
>
> > + if (err) {
> > + kfree(attr);
> > + WARN(1, "failed to register module [%s] BTF in sysfs: %d\n",
> > + mod->name, err);
>
> Again, just report the error and move on, don't crash systems.
>
Right, fixed.
> Other than these minor things, looks good to me, nice work!
>
Thanks!
> thanks,
>
> greg k-h
