On Fri, Jun 30, 2017 at 03:19:44PM -0400, Jason Baron wrote: > 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?
Right, and that was basically the intent with the (deprecated) "replace" feature. It reverts the old patch when it applies the new patch. -- Josh _______________________________________________ kpatch mailing list [email protected] https://www.redhat.com/mailman/listinfo/kpatch
