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. 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. -- Sashiko AI review ยท https://sashiko.dev/#/patchset/[email protected]?part=5
