Unified patch management interface required about kpatch HOT 5 CLOSED

There is no way to describe/manage the dependency among patches. Wish to have a way to describe patch dependency (e.g. patch B requires the kernel with patch A). The kernel version dependency can be solved with module versioning.

Dependencies between patch modules don't really exist. Each patch module is only built against the original kernel, so patches don't know about each other.

If patch B needs patch A, the best way to handle that is to merge the diffs into a single cumulative patch. If patch A had already been loaded, all its functions would be replaced by patch B.

To avoid a kernel panic when failing to unpatch, patching/unpatching should be done via sysfs or ioctl which can return errors. module unloading is very critical one. We can lock the patch modules with increment refcount from kpatch module until the unpatching was done successfully.

We could increment the patch module's refcount during insmod. Then require the user to echo 1 > /sys/class/kpatch/patches/kpatch_foo/disable, which will disable the patch and decrement the ref count, or return an error if the activeness safety check failed. If it succeeds, the user can then rmmod.

We have no way to check or list-up what functions are already patched, and which patch module has the patched function.

I agree, we need to export this information in sysfs, so the user can type "kpatch list" to see which patches are applied.

from kpatch.

I've been thinking more about the patch dependency issue. I think we still have some problems with respect to supporting multiple patch modules.

As one example:

• Patch 1 changes the call signature of funcA, which results in funcA being modified, as well as its caller, funcB.
• Patch 2 changes funcB.

Now assume both modules have been loaded. Since patch 2 was compiled with no knowledge of patch 1, patch 2's funcB will call patch 1's funcA with the wrong arguments and will probably crash the kernel.

This type of situation can be prevented by making all patches be a superset of all previously applied patches. For example, if patch 2 is created with a merged diff that includes the source from both patch 1 and 2, then patch 2 will include funcA and funcB. Patch 2's funcA will be identical to patch 1's funcA but will replace it regardless. Patch 2's funcB will know about the new call interface to funcA (or would fail to compile otherwise), so problem solved.

But this solution also has problems. Consider another example:

• Patch 1 makes a change which results in gcc deciding to inline funcB (which was previously not inlined). funcA calls funcB, so patch 1 has a new funcA which inlines funcB.
• Patch 2 changes funcB and makes other changes. gcc decides that, even in combination with the diff from patch 1, funcB should not be inlined. So patch 2 has a new funcB.

If patch 1 is loaded before patch 2, then patch 1's funcA will still call the old (inlined) funcB, instead of patch 2's new funcB, which is unexpected behavior.

Another example where it fails:

• Patch 1 modifies funcA
• Patch 2 reverts funcA to its original state and modifies funcB

In this case, because patch 2 is a diff on top of patch 1's diff, the user might expect that they can revert patch 1. But there would be no changes to funcA in the patch 2 module, since patch 2's funcA is identical to the kernel's funcA. So the revert of funcA would not occur when loading patch 2 after patch 1. This might be a contrived example though.

So there are at least two possible solutions:

1. Only allow one patch module to be loaded at any time. If the patch module needs to be updated with a replacement, then we can atomically unload the old one and load the new one.
2. Or we could try to put some more intelligence in kpatch-build to try to detect dependencies between patch modules and always do the right thing. I haven't really given much thought to the feasibility of this option yet.

Sorry for the long comment, just wanted to put it out there before I head off to the Collab Summit tomorrow.

from kpatch.

Only allow one patch module to be loaded at any time. If the patch module needs to be updated with a replacement, then we can atomically unload the old one and load the new one.

I think the best option is a variation on this one. Give the user an option to replace one or more patch modules atomically.

from kpatch.

Changes coming soon that will fix some of these issues:

• safe kpatch unload: #139
• kpatch load --replace which will atomically replace all loaded modules with a new module. This will allow a safe patch module upgrade path, and will be the safest way to use kpatch. It allows you to have cumulative patch upgrades and never have more than one patch loaded at a time. PR coming soon.
• Providing a listing of which functions have been patched, and which modules have patched them is a good idea. We should open a separate issue for that, as it's much easier to track if we have one issue per "issue".

from kpatch.

• safe "kpatch unload" was fixed with #139.
• atomic "kpatch replace" was added with #160.
• ability to list which functions have been patched is coming very soon with #180.

from kpatch.