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? 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/[email protected]?part=1
