Re: [PATCH v2] selftests/livepatch: fix resource leak in test_klp_syscall init error path

[sashiko-bot@kernel.org]

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 <qirui.001@bytedance.com>

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?

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?

Should kobject_del(klp_kobj) be called prior to kobject_put() to
synchronously remove the sysfs entries?

> +
> +	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?

-- 
Sashiko AI review · https://sashiko.dev/#/patchset/20260604083208.1071428-1-qirui.001@bytedance.com?part=1