Thread (24 messages) 24 messages, 4 authors, 10d ago

Re: [PATCH v10 5/9] livepatch: Implement provides and obsoletes for scoped atomic replace

flat view

From: Yafang Shao <hidden>
Date: 2026-10-08 11:04:54

On Thu, Oct 8, 2026 at 6:30 PM Petr Mladek [off-list ref] wrote:
On Sun 2026-10-04 23:46:29, Yafang Shao wrote:
quoted
On Sat, Oct 3, 2026 at 1:06 AM Petr Mladek [off-list ref] wrote:
quoted
On Wed 2026-09-30 11:03:19, 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 changes made by
  the related livepatch. Where the changes are a set
  of modified objects, functions, and used callbacks,
  shadow variable ids, and state ids.

- 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 scopes.

A new livepatch atomically replaces any existing livepatch that
meets any of the following conditions:
1. It has the same provides id as the new patch,
2. Its provides id is listed in the new patch's obsoletes list, or
3. The new patch's provides id is listed in its obsoletes list
   (the obsoletes relationship is symmetric).

The obsoletes relationship is symmetric: two livepatches that
obsolete each other are mutually exclusive. Loading either one
atomically removes the other, so they can never be enabled at the
same time. This allows swapping between two independently prepared
livepatches that cover the same scope, in either direction.
Honestly, I am confused by the 3rd rule and by the last paragraph.
It is not compatible with my understanding of the provides/obsoletes
semantic.
Correct. In package management semantics, Obsoletes is not a symmetric
relationship.
quoted
In my mental model, the provides and obsoletes define which
way is supported. Let me show an example, let's have:

 Livepatch A:
   + provides: 1
   + obsoletes:

 Livepatch B:
   + provides: 2
   + obsoletes: 1

 Livepatch C:
   + provides: 3
   + obsoletes: 1,2

 Livepatch D:
   + provides: 2
   + obsoletes: 1,3

How is this supposed to work, please?

In my mental model:

  Livepatch A:
     + can be replaced by B, C, or D because they all obsolete 1
     + can _not_ replace B, C, D because A does not obsolete 2 and/or 3.

  Livepatch B:
    + can be replaced by C because C obsoletes both 1 and 2
    + can be replaced by D beause D provides 2 and obsoletes 1
    + can't replace C or D because B neither provides not obsoletes 3.

  Livepatch C:
    + can be replaced by D because D provides and obsoletes
       all ids which are proviced and obsoleted by D

    + can replace D from the same reasons.


OK, I understand that it might be practical to allow replacing
the patches just because they are "conflicting". As a kind
of revert/downgrade.
Right. This will let us easily roll back the buggy livepatch and
reinstall the old one as part of the rollout.
quoted
It might help to rename "obsoletes" to "conflicts". And just
say that the livepatch replaces livepatch with the same id
in provides and also all conflicting patches.

But then I see another problem. We have three types of conflicts:

  1. same id in provides/obsoletes/conflicts
  2. same livepatches function
  3. same livepatch state id

It is weird that we would silently replace livepatches with 1st
type conflict. But we would refuse to load the livepatch with
the 2nd and/or 3rd type conflict.

Note that the inconsistent handling of conflicts exists in
this patch even when the array is called "obsoletes". My proposal
to rename it to "conflicts" just helped me to see this
inconsistence.

Any option/idea? Yet another naming?
Perhaps it could use a better name, but I'm not good at naming things ;)
quoted
One more thing, the commit message does not describe that
the livepatches might conflict when livepatching the same
function or using the same state id. It is important part
of the semantic and must be documented.
Sure. I will update it.
quoted
Best Regards,
Petr

PS: I am sorry but I won't be able to finish the review before
    the Plumbers conference. But I am happy that I have found
    the above problem. We could discuss it at the conference
    and come up with some sane naming and semantic.
I can't be there in person, but I believe we'll come up with a
sensible name or semantic. I'll hold off until we reach a final
conclusion.
I am not sure if you saw the video from the the conference.
Song has shared the results with me offline. Thank you all so much!
It looked like we were going to nack this feature because
installing more livepatches on the system was considered
too dangerous.

In particular, it allows using livepatches provided by different
teams/vendors. It is almost impossible to prevent incompatible
changes and breaking the system. The check preventing livepatching
the same function or using the same state IDs is just the best
effort but it still allows many hidden incompatibilities.

We should rather discourage people installing more livepatches
in parallel.

Anyway, we had a follow-up discussion and agreed to accept this
feature in the end. We already allow to install more livepatches
in parallel. So this patchset is not adding any new danger.
Instead, it reduces the danger a bit without complicating the
code too much.

Also we understand that there might be some use cases for this
feature. And it might be safe when used carefully.


We also discussed the semantic of the obsolete field and we
would like to it simple. The livepatch with a provides ID should
replace a livepatch with another provides ID _only_ when the other
ID is mentioned in obsoletes.

By other words, please remove the symmetry and "support" for
a rollback. The symmetry causes some confusion as explained
before. I would summarize it by two reasons:

   + In the packaging world, the "obsoletes" behavior is not
     symmetric.

   + The symmetric behavior adds a "conflict" semantic. But
     this particular conflict is handled another way than
     the other conflicts (livepatches functions, state IDs).
     I mean replacing an already enabled livepatch vs.
     rejecting the new one.

In addition, we do not support the rollback in all situations,
for example, let's have:

Livepatch A:
  + provides: 1
  + obsoletes:

Livepatch B:
  + provides: 2
  + obsoletes:

Livepatch C:
  + provides: 3
  + obsoletes: 1,2

The meaning is:

  + A and B are independent livepatches.
  + C adds some dependency and has to replace both A and B.

Here a proper rollback from C to A+B would require loading both
A and B livepatches synchronously. It would require adding
special support into the module loader and the livepatch core code.
We do not want to add this complexity. Especially, because
the parallel installation of livepatches is considered a bad
practice.

A partial workaround might be to allow changing "obsoletes" via
a livepatch module parameter. It would allow to do the
rollback in 2 steps:

   modprobe obsoletes=3 livepatch_A
   modprobe livepatch_B

It is still not an atomic rollback. But it is better than
without the parameter which would require 3 steps:

   echo 0 > /sys/kernel/livepatch/livepatch_C/enabled
   modprobe livepatch_A
   modprobe livepatch_B
Thank you for the review! I had actually sent v11 before your email
arrived — you can find it here:

https://lore.kernel.org/live-patching/20261008095830.26308-1-laoar.shao@gmail.com/ (local)

Regarding your summary:
Summary:

We are open to accept this feature but please:

    + Remove the symmetric handling of obsolete flag.
Done in v11.
    + Properly document, in both the commit message and
      documentation, when the livepatches will be replaced
      and when they will conflict.
Done in v11.
    + Document the pitfalls with the rollback.
Not yet addressed — I'll cover this in the next revision.
    + Add a warning that installing more livepatches in parallel
      is dangerous, especially when they are provided by different
      teams or vendors. The built-in checks of possible conflicts
      do not prevent potential incompatibilities and breakages.
Not yet addressed — planned for the next revision as well.

Heh, I hope that I got it right.
If you have a chance, could you take a look at v11 and let me know if
there's anything else I should address?
Thanks again for your time and guidance!

-- 
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