Thread (20 messages) 20 messages, 4 authors, 13d ago

Re: [PATCH v4 4/5] livepatch: Clear relocations when klp_apply_object_relocs() fails

flat view

From: Miroslav Benes <mbenes@suse.cz>
Date: 2026-09-18 14:05:19
Also in: lkml

On Fri, 18 Sep 2026, Miroslav Benes wrote:
On Tue, 8 Sep 2026, Petr Mladek wrote:
quoted
When a module is loaded, klp_module_coming() updates all enabled
livepatches. If an error occurs, it delegates cleanup to
klp_cleanup_module_patches_limited(). However, this cleanup loop skips
the partially updated patch, leaving any changes made prior to failure
unreverted.

One unhandled failure path occurs inside klp_apply_object_relocs(). On
architectures like x86_64, apply_relocate_add() performs a verification
step using memcmp() to check that memory contains the expected relocated
or zeroed value. If relocations left behind by a failed patch are not
cleared, subsequent patch operations or reloads can fail this validation.

Introduce klp_write_object_relocs_limited() to unwind and clear only the
relocations that were successfully applied before klp_write_object_relocs()
encountered an error.

There is no need to clear relocations for other objects in the failing
patch because klp_module_coming() operates strictly on the specific
module being loaded.
Hm, I spent some quite time on that and I am not sure if I deciphered 
everything correctly.

It seems to me that all error handling in those paths you are mentioning 
above is correct and the only problematic thing is that 
klp_apply_object_relocs() in klp_init_object_loaded() returns ret 
immediately which leaves partially applied relocations in case of an 
error. Following calls in klp_init_object_loaded() go through err: label 
where klp_free_object_loaded() is called which should be fine.

Is it correct?

Wouldn't be better to be somehow consistent and clean up right in the 
error path of klp_apply_object_relocs() call in klp_init_object_loaded() 
similarly to what is already there. After all we want to clean the object.

But I also see why you want to do it this way. It only seems more fragile 
to me.
And only now I realized that I checked with 5/5 already applied by 
mistake. The above still stands though and at least I reviewed 5/5 as 
well.

Miroslav
Keyboard shortcuts
hback out one level
jnext message in thread
kprevious message in thread
ldrill in
Escclose help / fold thread tree
?toggle this help