On Thu 2026-06-04 08:42:13, [email protected] wrote:
> Thank you for your contribution! Sashiko AI review found 1 potential issue(s)
> to consider:
>
> Pre-existing issues:
> - [High] Relying exclusively on kobject_put() to clean up sysfs attributes
> backed by module memory introduces a use-after-free window during module
> unload if CONFIG_DEBUG_KOBJECT_RELEASE is enabled.
> --
>
> commit 89541aa9ecd8e356eaf4884fa2b111bc605a7933
> Author: Rui Qi <[email protected]>
>
> selftests/livepatch: fix resource leak in test_klp_syscall init error path
>
> This commit attempts to fix a resource leak in livepatch_init() by calling
> kobject_put() on the error path if klp_enable_patch() fails.
>
> > --- a/tools/testing/selftests/livepatch/test_modules/test_klp_syscall.c
> > +++ b/tools/testing/selftests/livepatch/test_modules/test_klp_syscall.c
> [ ... ]
> > @@ -98,7 +98,11 @@ static int livepatch_init(void)
> > */
> > npids = npids_pending;
> >
> > - return klp_enable_patch(&patch);
> > + ret = klp_enable_patch(&patch);
> > + if (ret)
> > + kobject_put(klp_kobj);
>
> [Severity: High]
> This isn't a bug introduced by this patch, but it replicates a pre-existing
> issue into the error path.
>
> Does relying exclusively on kobject_put() here create a use-after-free window
> during module unload?
Sigh, I always get lost when looking at kobject-related API.
> If CONFIG_DEBUG_KOBJECT_RELEASE is enabled, kobject_put() drops the refcount
> but schedules kobject_cleanup() (and thus sysfs_remove_dir()) to run
> asynchronously via a workqueue after a 1 to 4 second delay. Because klp_attr
> and its show callback are allocated in the module's memory, returning an error
> here causes the module loader to free that memory immediately.
>
> During this delay, the sysfs file remains fully accessible. If a user reads
> this file, would sysfs_kf_seq_show() dereference the attribute and callback
> that now point to unmapped module memory?
This sounds correct.
> Should kobject_del(klp_kobj) be called prior to kobject_put() to
> synchronously remove the sysfs entries?
I am not sure if this would be a complete solution. I do not see
anywhere that kobject_del() would be synchronous. I think that
the file still might stay opened.
> > +
> > + return ret;
> > }
> >
> > static void livepatch_exit(void)
> > {
> > kobject_put(klp_kobj);
> > }
>
> [Severity: High]
> This is a pre-existing issue, but the exact same vulnerability exists here in
> the unmodified module exit path where kobject_put() is the only cleanup
> mechanism before the module unloads.
>
> Could we also add kobject_del(klp_kobj) before kobject_put() in
> livepatch_exit() to ensure sysfs entries are synchronously removed before
> module memory is freed?
I would ignore this. The same code (just kobject_put()) is used in
samples/kobject/kobject-example.c which is supposed to show how
the API should be used.
It is just a test module. The interface is used only by the selftest.
I believe that this is a problem of the kobject API and should
be fixed there.
Best Regards,
Petr
