Thread (19 messages) 19 messages, 6 authors, 2021-12-17

Re: [PATCH] livepatch: Fix leak on klp_init_patch_early failure path

From: Petr Mladek <pmladek@suse.com>
Date: 2021-12-17 13:10:23
Also in: live-patching, lkml

On Thu 2021-12-16 15:35:42, Greg Kroah-Hartman wrote:
On Thu, Dec 16, 2021 at 03:14:38PM +0100, Petr Mladek wrote:
quoted
There is the problem with kobject lifetime and module removal.
module is removed after mod->exit() callback finishes. But some
kobject release() callbacks might be delayed, especillay when
CONFIG_DEBUG_KOBJECT_RELEASE is enabled.
Ick, modules and kobjects, just say no :)
I will try to persuade you that it is not that uncommon scenario,
see below.

quoted
I have proposed there a solution where kobject_add_internal() takes reference
on the module. It would make sure that the module will stay in the
memory until the release callbacks is called, see
https://lore.kernel.org/all/Ya84O2%2FnYCyNb%2Ffp@alley/ (local)
I don't want to add module pointers to kobjects.

kobjects are data.  modules are code.  Module references control code
lifespans.  Kobject references control data lifespans.  They are
different, so let us never mix the two please.
I do not undestand this argument. The data are useless without the
code. The code is needed to remove the data. Therefore the code should
stay until the data are released.

I am talking about data using statically defined kobj_type in modules.

Yes, release callbacks are code, but if you really need to worry about
your release callback being unloaded, then just refuse to unload your
module until your data is all released!
This is exactly what I am proposing. If the kobject release callbacks
are defined in the module then the module should stay as long as
they are needed.

The huge majority of kobject users never touch them directly, they use
the driver model, which should not have this issue.  Only code that
wants to touch kobjects in the "raw" have problems like this, and if you
want to mess with them at that level, you can handle the release data
issue.
There seems to be 14 simple modules that define static strcut
kobj_type:

$> for file in `git grep "static struct kobj_type" | cut -d : -f1 | sort -u` ; \
       do grep -q "module_init" $file && echo $file ; \
   done
arch/sh/kernel/cpu/sh4/sq.c
drivers/block/pktcdvd.c
drivers/firmware/dmi-sysfs.c
drivers/firmware/efi/efivars.c
drivers/firmware/qemu_fw_cfg.c
drivers/net/bonding/bond_main.c
drivers/net/ethernet/ibm/ibmveth.c
drivers/parisc/pdc_stable.c
drivers/platform/x86/dell/dell-wmi-sysman/sysman.c
drivers/platform/x86/intel/uncore-frequency.c
drivers/platform/x86/uv_sysfs.c
drivers/uio/uio.c
kernel/livepatch/core.c
samples/kobject/kset-example.c


The other struct kobj_type are fined in 81 source files:

$> for file in `git grep "static struct kobj_type" | cut -d : -f1 | sort -u` ;  \
      do grep -q "module_init" $file || echo $file ; \
   done | wc -l
81


I believe that most of them might be compiled as modules as well.
There are many non-trivial drivers and file systes. From just a quick
look:

drivers/infiniband/hw/mlx4/sysfs.c
fs/btrfs/sysfs.c
fs/ext4/sysfs.c


Well, we should probably discuss this in the original thread
https://lore.kernel.org/r/20211129034509.2646872-3-ming.lei@redhat.com (local)

Best Regards,
Petr
Keyboard shortcuts
hback out one level
jnext message in thread
kprevious message in thread
ldrill in
Escclose help / fold thread tree
?toggle this help