On Tue 2026-09-08 12:25:48, [email protected] wrote: > Thank you for your contribution! Sashiko AI review found 1 potential issue(s) > to consider: > - [High] Partial relocations are not reverted when apply_relocate_add() fails > midway, leading to permanent state corruption. > -- > > commit 3f6df1b4ed699e839abbdc8919a6c7de5d1850c8 > Author: Petr Mladek <[email protected]> > > livepatch: Clean up klp_init_object_loaded() when fails > > When loading a module, klp_module_coming() updates all enabled patches. > If an error occurs, klp_cleanup_module_patches_limited() cleans up fully > processed patches, but skips the patch that failed midway. This commit > updates klp_free_object_loaded() to revert all and only changes made by > klp_init_object_loaded(). > > Closes: https://lore.kernel.org/r/[email protected] > Closes: https://lore.kernel.org/r/[email protected] > > > Next, klp_init_object_loaded() has to clear its own changes on > > failure. It just returns an error when relocations failed because > > they clear their own mess. It could call klp_free_object_loaded() > > in other situations because all relocations were done and other > > values are just cleared. > > [Severity: High] > Does this actually clear its own mess completely? Looking at how relocations > are rolled back in klp_write_object_relocs_limited(), which was introduced > earlier in this series: > > kernel/livepatch/core.c:klp_write_object_relocs_limited() { > ... > ret = klp_write_section_relocs(patch->mod, info->sechdrs, > info->secstrings, > patch->mod->core_kallsyms.strtab, > info->symndx, i, obj->name, > apply); > if (ret) { > if (apply) > klp_write_object_relocs_limited(patch, obj, > false, i); > return ret; > } > ... > } > > By passing i as the limit to the rollback function, the rollback loop: > > for (i = 1; i < limit; i++) { > > will terminate at i - 1. Doesn't this skip the rollback for the partially > applied section i? > > If apply_relocate_add succeeds on relocations 0 to k-1 in section i, but > fails on relocation k, these partial relocations remain permanently applied > in the livepatch module's text.
This is the same problem which was reported for the 4th patch. It would need to handled in the arch-code, namely in __write_relocate_add() in arch/x86/kernel/module.c. Best Regards, Petr > Subsequent attempts to load the target module will fail immediately at > relocation 0 due to non-zero target memory, converting a potentially transient > address-space layout failure into a permanent -ENOEXEC on retry.
