Thread (53 messages) flat view 53 messages, 6 authors, 10d ago

Re: Replace rules: was: Re: [PATCH v7 for-next 3/8] livepatch: Implement replace set for scoped atomic replace

From: Yafang Shao <hidden>
Date: 2026-09-02 11:51:06

On Wed, Sep 2, 2026 at 5:40 PM Yafang Shao [off-list ref] wrote:
On Wed, Sep 2, 2026 at 3:31 PM Petr Mladek [off-list ref] wrote:
quoted
On Tue 2026-08-25 19:46:36, Yafang Shao wrote:
quoted
The current bool replace flag is too coarse: it is either all or
nothing. A livepatch with .replace=true replaces ALL existing
livepatches, which is safe but inflexible. There is no way to have
multiple independent livepatch sets coexist on the same system.

Replace it with a more flexible model using two new fields in
struct klp_patch:

- provides: an unsigned int id identifying the patch replace set.
  By default (provides=0), any livepatch replaces any other livepatch.

- obsoletes: an optional array of unsigned int ids specifying
  additional provides ids to be replaced. This allows a new patch
  to explicitly obsolete patches from different replace sets.
quoted
--- a/include/linux/livepatch.h
+++ b/include/linux/livepatch.h
@@ -123,7 +123,8 @@ struct klp_state {
  * @mod:     reference to the live patch module
  * @objs:    object entries for kernel objects to be patched
  * @states:  system states that can get modified
- * @replace: replace all actively used patches
+ * @provides:        only one active livepatch per id
+ * @obsoletes:       replace given livepatch id(s)
  * @list:    list node for global list of actively used patches
  * @kobj:    kobject for sysfs resources
  * @obj_list:        dynamic list of the object entries
@@ -137,7 +138,9 @@ struct klp_patch {
      struct module *mod;
      struct klp_object *objs;
      struct klp_state *states;
-     bool replace;
+     unsigned int provides;
+     unsigned int *obsoletes;
+     unsigned int nr_obsoletes;
This is a different sematic in compare with the other arrays.
I guess that you wanted to allow obsoleting livepatch with '0' ID.
right.
quoted
But '0' is special. It obsoletes anything. Maybe, we could
make it even more special and say that it can't obsoleted.
Then we would be able to use it as the trailing element
in the array...

I do not have strong opinion about this. It is just an idea.
Making '0' special is good for backward compatibility, but it
complicates usage for users. Therefore, I prefer not to treat '0' as a
special case.
quoted

Another question:

Should we allow to enable a livepatch when its provides id
is obsoleted by a currently enabled livepatch?
Good question.
I believe it's best to refuse to load it, as doing otherwise might
introduce potential issues. I will update this rule in the next
version.
quoted
For example, let's have:

  + Livepatch A: provides:1
  + Livepatch B: provides:2, obsoletes:1

Now, two scenarios:

  1. Livepatch A can be replaced by livepatch B. This is easy.
  2. Can livepatch B get replaced by livepatch A?
No, I don't believe B should be replaced by A. In this case, if B is
already enabled, A should fail to load.
This brings up another question regarding large server fleets. In our
production environment, if a new livepatch introduces a regression, we
always roll back to the old version. For example, if A is the old
version and B is the new one, we will roll back to A if B causes
issues. In that case, if we refuse to load A while B is already
loaded, we can't roll back. (In practice, though, we haven't rolled
back a single livepatch after rolling out 40 versions on our 6.1.y
stable kernel.)
However, this isn't an unfixable issue.  In the future, I plan to
introduce dynamical provides IDs and obsoletes IDs at load time,
allowing us to change IDs on demand
quoted
The current code would allow to replace B with A when there is
_no_ real conflict in the livaptched objects, functions, and states.

But does it make sense?

Reasoning: Livepatch B obsoleted the livepatch A for a reason.
        It sounds like they should not be enabled at the same time.


Special case:

Should we allow to install a livepatch with non-zero provides
when a livepatch with zero provides is installed.

For example, let's have:

  + Livepatch A: provides:1
  + Livepatch B: provides:0

Now, two scenarios:

  1. Livepatch A can be replaced by livepatch B. This is easy.
  2. Can livepatch A be installed in parallel with B?

Reasoning: The livepatch B replaces everything because it wants
        to be the only installed livepatch. It sounds weird
        to "break" it by installing A in parallel later again.
So, let's just not treat '0' as a special case?

--
Regards
Yafang


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