On Wed, Jun 10, 2026 at 10:46 PM Petr Mladek <[email protected]> wrote: > > On Sun 2026-06-07 21:16:55, Yafang Shao wrote: > > Convert the replace attribute from a boolean to a u32 to function as a > > "replace set." A newly loaded livepatch will now atomically replace any > > existing patch belonging to the same set. There can only ever be one active > > livepatch for a given replace_set number. > > > > This change currently supports function replacement only. Livepatches that > > belong to different replace sets cannot modify the same function. If a new > > livepatch attempts to modify a function already modified by an older > > livepatch from a different replace_set, the loading of the new livepatch > > will be refused. > > > > Similarly, for the KLP state, livepatches belonging to different replace > > sets cannot use the same state ID. The system will refuse to load a new > > livepatch if it uses a state ID already in use by an older livepatch from > > a different replace_set. > > > > For the KLP shadow variable mechanism, developers must assign unique shadow > > IDs to livepatches that belong to different replace sets. > > > > Support for replace_set compatibility with KLP state and shadow variables > > will be implemented after Petr's KLP state transfer work is completed [0]. > > > > Other user-visible changes include: > > - The non-replace model is now deprecated > > - /sys/kernel/livepatch/livepatch_XXX/replace attribute is replaced by > > /sys/kernel/livepatch/livepatch_XXX/replace_set > > > > --- a/Documentation/ABI/testing/sysfs-kernel-livepatch > > +++ b/Documentation/ABI/testing/sysfs-kernel-livepatch > > @@ -47,13 +47,12 @@ Description: > > disabled when the feature is used. See > > Documentation/livepatch/livepatch.rst for more information. > > > > -What: /sys/kernel/livepatch/<patch>/replace > > +What: /sys/kernel/livepatch/<patch>/replace_set > > Date: Jun 2024 > > KernelVersion: 6.11.0 > > The Date and KernelVersion should be upsated as well. > It is a new attribute...
Sure > > > Contact: [email protected] > > Description: > > - An attribute which indicates whether the patch supports > > - atomic-replace. > > + An attribute to show the replace_set of this livepatch. > > > > What: /sys/kernel/livepatch/<patch>/stack_order > > Date: Jan 2025 > > diff --git a/Documentation/livepatch/cumulative-patches.rst > > b/Documentation/livepatch/cumulative-patches.rst > > index 1931f318976a..0361adb12f6d 100644 > > --- a/Documentation/livepatch/cumulative-patches.rst > > +++ b/Documentation/livepatch/cumulative-patches.rst > > @@ -17,18 +17,20 @@ from all older livepatches and completely replace them > > in one transition. > > We need to update also the introduction part above. I propose > the following text: > > <proposal> > Livepatches are used to fix kernel bugs. New fixes need to be added over time. > The fixes might be independent, but they might also depend on each other. This > brings a challenge of how to keep the livepatched system safe and consistent. > > Part of the solution is the "Atomic Replace" feature, which allows the kernel > to > atomically replace an existing livepatch with another one. These newer > livepatches are designed as "Cumulative Patches". They include all wanted > changes from all older livepatches and completely replace them in one > transition. > > The second part of the solution is the newly introduced ``replace_set`` > feature, > which allows the installation of multiple livepatches in parallel. A livepatch > will only replace other livepatches that share the same ``replace_set`` > number. > This might be used to fix independent problems separately, for example, the > livepatches might be prepared by separate teams focusing on particular > functionality or a subsystem. > > It should be emphasized that the preferred and most secure way is to always > use > the default ``replace_set = 0``. In this mode, any livepatch replaces any > other > livepatch, preventing any unexpected interactions between incompatible > livepatches. > </proposal> I will update it. > > > Usage > > ----- > > I would split the "Usage" section into two new sections, see below. > > > -The atomic replace can be enabled by setting "replace" flag in struct > > klp_patch, > > -for example:: > > +The "replace_set" attribute in ``struct klp_patch`` acts as a > > **replace_set**, > > +defining the scope of the replacement. By default, the replace_set is 0. > > + > > +For example:: > > > > static struct klp_patch patch = { > > .mod = THIS_MODULE, > > .objs = objs, > > - .replace = true, > > + .replace_set = 0, > > }; > > I would use the above text in a new section called "Replace Set": > > <proposal> > Replace Set > ----------- > > The "replace_set" attribute in ``struct klp_patch`` acts as a **replace_set**, > defining the scope of the replacement. By default, the replace_set is 0. > > For example:: > > static struct klp_patch patch = { > .mod = THIS_MODULE, > .objs = objs, > .replace_set = 0, > }; > > Any ``replace_set`` value might be associated with a set of livepatched > symbols, > callbacks, shadow variables, and state IDs. By definition, there can only ever > be one active livepatch for a given ``replace_set`` number. > > On the contrary, livepatches with a different ``replace_set`` number must not > modify the same function, or use the state with the same ID. Any attempt to > load an incompatible livepatch will be rejected by the kernel. > </proposal> > > And the rest would be in a new section called: > > Atomic Replace > -------------- > > > All processes are then migrated to use the code only from the new patch. > > -Once the transition is finished, all older patches are automatically > > -disabled. > > +Once the transition is finished, all older patches with the same replace > > +set are automatically disabled. Patches with different tags remain active. > > There always could be only one livepatch with the given replace set. > I would write something like: > > <proposal> > A livepatch with a given ``replace_set`` number is replaced by another > livepatch using the same number. All processes are migrated to use > the code only from the new patch. Once the transition is finished, > the older patch. Patches with different replace sets are not affected > and remain active. > </proposal> I will update it. > > > Ftrace handlers are transparently removed from functions that are no > > longer modified by the new cumulative patch. > > --- a/kernel/livepatch/core.c > > +++ b/kernel/livepatch/core.c > > @@ -600,6 +600,8 @@ static int klp_add_nops(struct klp_patch *patch) > > klp_for_each_object(old_patch, old_obj) { > > int err; > > > > + if (patch->replace_set != old_patch->replace_set) > > + continue; > > This should be checked on the klp_for_each_patch(old_patch) level. > The result is the same for each old_obj from the given old_patch... will update it. > > > err = klp_add_object_nops(patch, old_obj); > > if (err) > > return err; > > @@ -1174,6 +1176,8 @@ void klp_unpatch_replaced_patches(struct klp_patch > > *new_patch) > > if (old_patch == new_patch) > > return; > > > > Nit: Remove the empty line here. > > > + if (old_patch->replace_set != new_patch->replace_set) > > + continue; > > Nit: And put it here instead. To separate if/return/continue part from > the changes done for the eligible old_patch. Thanks for the code style improvements. > > > old_patch->enabled = false; > > klp_unpatch_objects(old_patch); > > } > > diff --git a/kernel/livepatch/state.c b/kernel/livepatch/state.c > > index 2565d039ade0..a1ac46637336 100644 > > --- a/kernel/livepatch/state.c > > +++ b/kernel/livepatch/state.c > > @@ -85,34 +85,65 @@ EXPORT_SYMBOL_GPL(klp_get_prev_state); > > > > /* Check if the patch is able to deal with the existing system state. */ > > static bool klp_is_state_compatible(struct klp_patch *patch, > > + struct klp_patch *old_patch, > > struct klp_state *old_state) > > { > > struct klp_state *state; > > > > state = klp_get_state(patch, old_state->id); > > + if (patch->replace_set == old_patch->replace_set) { > > + /* > > + * If the new livepatch shares a state set with an existing > > + * one, it must maintain compatibility with all states > > + * modified by the old patch. > > + */ > > + if (!state) > > + return false; > > + return state->version >= old_state->version; > > > > - /* A cumulative livepatch must handle all already modified states. */ > > - if (!state) > > - return !patch->replace; > > + } > > > > - return state->version >= old_state->version; > > + /* > > + * Two livepatches with a different "replace_set" must _not_ use > > + * the same "state->id. > > + */ > > + return !state; > > } > > > > -/* > > - * Check that the new livepatch will not break the existing system states. > > - * Cumulative patches must handle all already modified states. > > - * Non-cumulative patches can touch already modified states. > > - */ > > +/* Check that the new livepatch will not break the existing system states. > > */ > > bool klp_is_patch_compatible(struct klp_patch *patch) > > { > > + struct klp_object *obj, *old_obj; > > struct klp_patch *old_patch; > > struct klp_state *old_state; > > + struct klp_func *func; > > > > klp_for_each_patch(old_patch) { > > klp_for_each_state(old_patch, old_state) { > > - if (!klp_is_state_compatible(patch, old_state)) > > + if (!klp_is_state_compatible(patch, old_patch, > > old_state)) > > return false; > > } > > + > > + if (old_patch->replace_set == patch->replace_set) > > + continue; > > + > > + /* > > + * Refuse loading a livepatch which would want to modify a > > + * function which is already livepatched with the livepatch > > + * with another "replace_set". > > + */ > > + klp_for_each_object_static(patch, obj) { > > + klp_for_each_object(old_patch, old_obj) { > > + if (!!obj->name != !!old_obj->name) > > + continue; > > + if (obj->name && strcmp(obj->name, > > old_obj->name)) > > + continue; > > + klp_for_each_func_static(obj, func) { > > + if (klp_find_func(old_obj, func)) > > + return false; > > + } > > The combination of for_each_() and for_each_*_static() variants look > correct but it is a bit scarry. > > I think about calling klp_init_patch_early() before > klp_is_patch_compatible() so that we could use the non-static > for_each_*() variants even for the new patch. It would make the life > easier. But I do not have strong opinion about it. I will think about it. > > > + } > > Anyway, please, put the new check into a separate function, something > like: > > bool klp_has_function_conflict(struct klp_patch *patch, > struct klp_patch *old_patch); I will update it. > > > > + } > > } > > > > return true; > > The rest looks good. Thanks a lot for the review! -- Regards Yafang
