Re: [PATCH v2] params: serialize lookup_or_create_module_kobject()

[Andrew Morton <akpm@linux-foundation.org>]

On Fri, 18 Sep 2026 10:07:12 +0000 Jiakai Xu <xujiakai24@mails.ucas.ac.cn> wrote:

> lookup_or_create_module_kobject() first looks up the module kobject with
> kset_find_obj() and, if not found, creates a new one with
> kobject_init_and_add().  The function is called at runtime from
> module_add_driver() since commit f95bbfe18512 ("drivers: base: handle
> module_kobject creation"), which means two concurrent driver
> registrations for the same built-in module name can both miss the
> lookup and race to create the same kobject.
> 
> The loser of the race gets -EEXIST from kobject_init_and_add() and its
> kobject is removed from module_kset by kobject_add_internal() before
> the failure is reported.  The error path then calls kobject_put(),
> which invokes module_kobj_release(), but that only completes
> ->kobj_completion and never frees the dynamically allocated
> module_kobject, leaking it (96 bytes) along with the object having been
> detached from the kset.
> 
> This is triggerable by unprivileged users, e.g. by concurrently issuing
> the RAW_IOCTL_INIT ioctl of the raw-gadget driver, which registers the
> "raw_gadget" driver on the gadget bus:
> 
>   sysfs: cannot create duplicate filename '/module/raw_gadget'
>   ...
>   Adding module 'raw_gadget' to sysfs failed (-17), the system may be
>   unstable.
>   ...
>   unreferenced object 0xffff8880188dacc0 (size 96):
>     backtrace:
>       lookup_or_create_module_kobject+0x47/0x100
>       module_add_driver+0x73/0x1b0
>       bus_add_driver+0x1d9/0x340
>       driver_register+0xde/0x170
> 
> Fix it by serializing the lookup and the creation with a mutex, so that
> the second caller finds the kobject created by the first one instead of
> racing with it.

Thanks.  AI-based review said a bunch of things (below), but this fix
is considered sound.  I'll make that Fixes: change and add a couple of
Petr's changes while applying.

> Crash report (raw_report0) of the syzkaller crash this patch fixes:
> ---
> BUG: memory leak
> unreferenced object 0xffff8880188dacc0 (size 96):
>   comm "syz.3.569", pid 12998, jiffies 4294992047
>   hex dump (first 32 bytes):
>     12 4e fe 86 ff ff ff ff c8 ac 8d 18 80 88 ff ff  .N..............
>     c8 ac 8d 18 80 88 ff ff 00 00 00 00 00 00 00 00  ................
>   backtrace (crc 8dccd85e):
>     kmemleak_alloc_recursive home/zzzrrll/tmp/kf_src/linux-7.3-rc2/include/linux/kmemleak.h:44 [inline]
>     slab_post_alloc_hook home/zzzrrll/tmp/kf_src/linux-7.3-rc2/mm/slub.c:4696 [inline]
>     slab_alloc_node home/zzzrrll/tmp/kf_src/linux-7.3-rc2/mm/slub.c:4996 [inline]
>     __kmalloc_cache_noprof+0x1bf/0x420 home/zzzrrll/tmp/kf_src/linux-7.3-rc2/mm/slub.c:5559

It would be nice to have a Reported-by: and a Closes: for the syscaller
report, but I wasn't able to find the original.


: The mutex fixes the race between two callers of this helper, but the
: changelog needs Petr's corrections: USB_RAW_IOCTL_RUN does the
: registration, and the device node defaults to root-only access.
: 
: Petr is also right about the remaining leak. Other failures from
: kobject_init_and_add() or sysfs_create_file() still reach a release
: callback which never frees the allocated module_kobject. That needs
: a separate fix.
: 
: Please keep f95bbfe18512 as the Fixes commit and drop the redundant
: 7c76c813cfc4 tag. Also remove the v6.15+ restriction: the introducing
: change was backported, including to 6.12.28.
: 
: I don't think the module-removal answer is correct. mod_sysfs_init()
: does not take this mutex, and module_patient_check_exists() checks
: the loadable module list, not the built-in objects.
: 
: There is a conditional lifetime problem here. If a separately loaded
: module already owns the same name before the built-in driver's first
: registration, the lookup can return that module's embedded mkobj.
: module_add_driver() drops the lookup reference before using and
: caching the pointer, so unloading that module can leave a stale
: pointer. This requires privileged loading of a same-named module;
: it isn't an ordinary unprivileged raw-gadget operation.
: 
: That predates this patch, but deserves a separate follow-up. I checked
: the source and a bounded model, not a running-kernel reproducer.
: Successful kobject_get_unless_zero() protects the object while the
: reference is held; there isn't a generic kobject-core gap there.