Thank you for your contribution! Sashiko AI review found 1 potential issue(s) to consider:
Pre-existing issues: - [High] Missing `codetag_unload_module(mod)` on `codetag_load_module()` failure permanently strands partially initialized codetags, causing a Use-After-Free when the freed module is subsequently accessed. -- commit 46e63afac7b776756489b1e2ad6cec3cf1e11513 Author: Hao Ge <[email protected]> module: fix lost error code from codetag_load_module() If codetag_load_module() fails, err is not set to reflect the failure and load_module() returns 0 after the module has been torn down. > diff --git a/kernel/module/main.c b/kernel/module/main.c > index d0e1e0bd2ad06..c1b34dc1e89ac 100644 > --- a/kernel/module/main.c > +++ b/kernel/module/main.c > @@ -3581,8 +3581,9 @@ static int load_module(struct load_info *info, const > char __user *uargs, > goto sysfs_cleanup; > } > > - if (codetag_load_module(mod)) > - goto sysfs_cleanup; > + err = codetag_load_module(mod); > + if (err) > + goto livepatch_cleanup; [Severity: High] This isn't a bug introduced by this patch, but missing codetag_unload_module() on codetag_load_module() failure permanently strands partially initialized codetags, causing a use-after-free when the freed module is subsequently accessed. If a module load is initiated, and codetag_load_module() iterates through registered codetag_types: lib/codetag.c:codetag_load_module() { ... ret = codetag_module_init(cttype, mod); if (ret) break; ... } If an early iteration succeeds but a subsequent one fails (e.g., kmalloc failure inside codetag_module_init()), codetag_load_module() breaks and returns the error without unwinding previously allocated cmod entries. load_module() receives the error, jumps to livepatch_cleanup and eventually calls module_deallocate(mod, info) to free the module memory. However, codetag_unload_module(mod) is never invoked on this error path, leaving cmod entries pointing to freed module memory inside the successfully initialized cttype->mod_idr structures. When subsequent codetag operations (like reading /proc/allocinfo) iterate over cttype->mod_idr, they will access the freed module memory. Should this error path invoke codetag_unload_module(mod) to clean up the partially registered state before tearing down and freeing the module memory? > > /* Get rid of temporary copy. */ > free_copy(info, flags); > @@ -3592,6 +3593,9 @@ static int load_module(struct load_info *info, const > char __user *uargs, > > return do_init_module(mod); > > + livepatch_cleanup: > + if (is_livepatch_module(mod)) > + free_module_elf(mod); > sysfs_cleanup: > mod_sysfs_teardown(mod); > coming_cleanup: -- Sashiko AI review ยท https://sashiko.dev/#/patchset/[email protected]?part=1
