From: Miroslav Benes <mbenes@suse.cz>
To: Petr Mladek <pmladek@suse.com>
Cc: Harry Hsu <x90613@gmail.com>,
jpoimboe@kernel.org, joe.lawrence@redhat.com, jikos@kernel.org,
live-patching@vger.kernel.org, shuah@kernel.org,
song@kernel.org, linux-kernel@vger.kernel.org,
sashiko-bot@kernel.org
Subject: Re: [PATCH v4 4/5] livepatch: Clear relocations when klp_apply_object_relocs() fails
Date: Fri, 18 Sep 2026 16:05:09 +0200 (CEST) [thread overview]
Message-ID: <alpine.LSU.2.21.2609181604200.21788@pobox.suse.cz> (raw)
In-Reply-To: <alpine.LSU.2.21.2609181554590.21788@pobox.suse.cz>
On Fri, 18 Sep 2026, Miroslav Benes wrote:
> On Tue, 8 Sep 2026, Petr Mladek wrote:
>
> > 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
next prev parent reply other threads:[~2026-09-18 14:05 UTC|newest]
Thread overview: 20+ messages / expand[flat|nested] mbox.gz Atom feed top
2026-09-08 12:03 [PATCH v4 0/5] livepatch: Fail object initialization on duplicate patched function Petr Mladek
2026-09-08 12:03 ` [PATCH v4 1/5] " Petr Mladek
2026-09-08 12:17 ` sashiko-bot
2026-09-08 13:02 ` Petr Mladek
2026-09-18 12:54 ` Miroslav Benes
2026-09-30 14:11 ` [PATCH v4 1/3] " Harry Hsu
2026-10-01 4:32 ` [PATCH v4 1/5] " Harry Hsu
2026-09-08 12:03 ` [PATCH v4 2/5] selftests/livepatch: Test rejection of aliased symbols in one object Petr Mladek
2026-09-18 13:40 ` Miroslav Benes
2026-09-08 12:03 ` [PATCH v4 3/5] livepatch: Move code for updating livepatch object relocations Petr Mladek
2026-09-18 13:40 ` Miroslav Benes
2026-09-08 12:03 ` [PATCH v4 4/5] livepatch: Clear relocations when klp_apply_object_relocs() fails Petr Mladek
2026-09-08 12:18 ` sashiko-bot
2026-09-08 13:29 ` Petr Mladek
2026-09-18 13:59 ` Miroslav Benes
2026-09-18 14:05 ` Miroslav Benes [this message]
2026-09-08 12:03 ` [PATCH v4 5/5] livepatch: Clean up klp_init_object_loaded() when fails Petr Mladek
2026-09-08 12:25 ` sashiko-bot
2026-09-08 13:32 ` Petr Mladek
2026-09-18 14:07 ` Miroslav Benes
Reply instructions:
You may reply publicly to this message via plain-text email
using any one of the following methods:
* Save the following mbox file, import it into your mail client,
and reply-to-all from there: mbox
Avoid top-posting and favor interleaved quoting:
https://en.wikipedia.org/wiki/Posting_style#Interleaved_style
* Reply using the --to, --cc, and --in-reply-to
switches of git-send-email(1):
git send-email \
--in-reply-to=alpine.LSU.2.21.2609181604200.21788@pobox.suse.cz \
--to=mbenes@suse.cz \
--cc=jikos@kernel.org \
--cc=joe.lawrence@redhat.com \
--cc=jpoimboe@kernel.org \
--cc=linux-kernel@vger.kernel.org \
--cc=live-patching@vger.kernel.org \
--cc=pmladek@suse.com \
--cc=sashiko-bot@kernel.org \
--cc=shuah@kernel.org \
--cc=song@kernel.org \
--cc=x90613@gmail.com \
/path/to/YOUR_REPLY
https://kernel.org/pub/software/scm/git/docs/git-send-email.html
* If your mail client supports setting the In-Reply-To header
via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line
before the message body.
This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.