Re: [PATCH v4 5/5] livepatch: Clean up klp_init_object_loaded() when fails

[sashiko-bot@kernel.org]

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 <pmladek@suse.com>

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/20260823062313.1321B1F000E9@smtp.kernel.org
Closes: https://lore.kernel.org/r/20260830175608.4BABB1F000E9@smtp.kernel.org

> 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/20260908120325.299649-1-pmladek@suse.com?part=5