lkml.org 
[lkml]   [2021]   [Dec]   [17]   [last100]   RSS Feed
Views: [wrap][no wrap]   [headers]  [forward] 
 
Messages in this thread
    /
    Date
    From
    SubjectRe: [PATCH] livepatch: Fix leak on klp_init_patch_early failure path
    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:
    > > 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.


    > > 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/
    >
    > 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

    Best Regards,
    Petr

    \
     
     \ /
      Last update: 2021-12-17 14:10    [W:6.574 / U:0.052 seconds]
    ©2003-2020 Jasper Spaans|hosted at Digital Ocean and TransIP|Read the blog|Advertise on this site