On 06/30/2017 01:03 PM, Josh Poimboeuf wrote:
On Fri, Jun 30, 2017 at 12:40:12PM -0400, Jason Baron wrote:
On 06/30/2017 11:49 AM, Josh Poimboeuf wrote:
On Wed, Jun 28, 2017 at 04:36:13PM -0500, Josh Poimboeuf wrote:
On Wed, Jun 28, 2017 at 04:34:18PM -0500, Josh Poimboeuf wrote:
On Wed, Jun 28, 2017 at 04:25:19PM -0500, Josh Poimboeuf wrote:
On Wed, Jun 28, 2017 at 02:02:09PM -0400, Jason Baron wrote:
Hi,
If I have kernel source S, apply a kpatch module A, but then want to
'revert' parts of A, by applying a cumulative patch B, such that some
functions are no longer different from source kernel S, how do I do that?
It seems like there are a few options:
1) remove or disable module A, then load module B. This leaves an
un-patched window and rmmod isn't possible without reliable backtrace.
I'm also not sure this works with the kpatch 'stacking'.
2) modify patch B such that any functions that are initially unchanged
from S, are in fact modified but modified in an acceptable way.
3) maybe the kernel can understand that are not added to or modified by
a cumulative patch should be reverted back to their original state?
#3 seems to me to be the cleanest...but maybe I'm missing something...
Hi Jason,
We don't have a mechanism for reverting patches, so #2 will probably be
your best bet.
Sorry, that was both poorly worded and incorrect, let me try again :-)
I meant to say we don't have a mechanism for atomically replacing the
entire contents of patch A with patch B.
But I was mistaken. Technically we do have a "replace" mechanism. It
was deprecated, see https://github.com/dynup/kpatch/issues/456 for more
details. But it should work in theory if you load the patch module
manually with insmod and the 'replace=1' argument. We had some issues
with it though, so #2 might be a safer bet.
And I should note that the replace only works with kpatch.ko, not with
livepatch. We may add replace support to livepatch eventually.
FYI, I'm proposing a new KPATCH_REVERT_FUNCTION macro which makes this a
little easier, so you can use the macro instead of making a trivial
change to the function:
https://github.com/dynup/kpatch/pull/715
I have to explicitly mark functions as revert, right?
I'm wondering though if the function is inlined then I have to go and mark
all the places where it was inlined as KPATCH_REVERT_FUNCTION()? And further
there may be other changes in some of those functions and not others.
Yes, there are still manual steps involved:
1) compile v1 and note the list of changed functions
2) compile v2 (without KPATCH_REVERT_FUNCTION macro) and note the
list of changed functions
3) for those functions which changed in v1 and did not change in v2,
annotate them with KPATCH_REVERT_FUNCTION
We should document those steps in the patch author guide as part of the
pull request.
So it seems like it would be nice to calculate this based on the previous
set of loaded modules, or from the state of currently patched kernel.
I agree it would be nice to have some way to automate it. I don't think
using the state of the currently patched kernel would work in most
cases, because when building kpatches for a distro, the kernel isn't
typically patched.
The thought is that the core kernel can know that its loading cumulative
patches and thus when it loads a new livepatch module it can revert any
functions that are not explicitly patched by the new livepatch module.
Would that make sense?
Thanks,
-Jason
Maybe we could allow the user to pass in any previously built modules on
the command-line, and then kpatch-build could automatically do the same
work as KPATCH_REVERT_FUNCTION. I opened an issue to track that idea:
https://github.com/dynup/kpatch/issues/716
We could either do that, or get atomic replace working properly. But
either way it's going to be a bigger piece of work, so in the meantime
we can have KPATCH_REVERT_FUNCTION and its associated manual steps.
_______________________________________________
kpatch mailing list
[email protected]
https://www.redhat.com/mailman/listinfo/kpatch
