From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received: (majordomo@vger.kernel.org) by vger.kernel.org via listexpand id S932384AbdHVJRP (ORCPT ); Tue, 22 Aug 2017 05:17:15 -0400 Received: from mx2.suse.de ([195.135.220.15]:43729 "EHLO mx1.suse.de" rhost-flags-OK-OK-OK-FAIL) by vger.kernel.org with ESMTP id S932205AbdHVJRN (ORCPT ); Tue, 22 Aug 2017 05:17:13 -0400 Date: Tue, 22 Aug 2017 11:17:11 +0200 From: Petr Mladek To: Joe Lawrence Cc: live-patching@vger.kernel.org, linux-kernel@vger.kernel.org, Josh Poimboeuf , Jessica Yu , Jiri Kosina , Miroslav Benes , Chris J Arges Subject: Re: [PATCH v3] livepatch: add (un)patch callbacks Message-ID: <20170822091711.GC30286@pathway.suse.cz> References: <1502911024-16143-1-git-send-email-joe.lawrence@redhat.com> <1502911024-16143-2-git-send-email-joe.lawrence@redhat.com> <20170818135816.GB25223@pathway.suse.cz> <20170821211858.n5wsweei7zew6bi5@redhat.com> MIME-Version: 1.0 Content-Type: text/plain; charset=us-ascii Content-Disposition: inline In-Reply-To: <20170821211858.n5wsweei7zew6bi5@redhat.com> User-Agent: Mutt/1.5.21 (2010-09-15) Sender: linux-kernel-owner@vger.kernel.org List-ID: X-Mailing-List: linux-kernel@vger.kernel.org On Mon 2017-08-21 17:18:58, Joe Lawrence wrote: > On Fri, Aug 18, 2017 at 03:58:16PM +0200, Petr Mladek wrote: > > On Wed 2017-08-16 15:17:04, Joe Lawrence wrote: > > > Provide livepatch modules a klp_object (un)patching notification > > > mechanism. Pre and post-(un)patch callbacks allow livepatch modules to > > > setup or synchronize changes that would be difficult to support in only > > > patched-or-unpatched code contexts. > > > > > > diff --git a/kernel/livepatch/core.c b/kernel/livepatch/core.c > > > index b9628e43c78f..ddb23e18a357 100644 > > > --- a/kernel/livepatch/core.c > > > +++ b/kernel/livepatch/core.c > > > @@ -878,6 +890,8 @@ int klp_module_coming(struct module *mod) > > > goto err; > > > } > > > > > > + klp_post_patch_callback(obj); > > > > This should be called only if (patch != klp_transition_patch). > > Otherwise, it would be called too early. > > Can you elaborate a bit on this scenario? When would the transition > patch (as I understand it, a livepatch not quite fully (un)patched) hit > the module coming/going notifier? Is it possible to load or unload a > module like this? I'd like to add this scenario to my test script if > possible. if (patch == klp_transition_patch) then the transition for this patch is still running. klp_post_patch_callback() should and is called at the end of the transition, see klp_complete_transition(). Note that klp_complete_transition() will see the object/module loaded because obj->mod is set in klp_module_coming(). The (up)patch transition might take a while. It might be even infinite if a process is blocked in a patched function. klp_mutex is available most of the time. Therefore it is perfectly fine to load/remove modules into/from the system and run their klp_module_coming()/going() callbacks when a livepatch patch transition is running. > > > + > > > break; > > > } > > > } > > > @@ -929,7 +943,10 @@ void klp_module_going(struct module *mod) > > > if (patch->enabled || patch == klp_transition_patch) { > > > pr_notice("reverting patch '%s' on unloading module '%s'\n", > > > patch->mod->name, obj->mod->name); > > > + > > > + klp_pre_unpatch_callback(obj); > > > > Also the pre_unpatch() callback should be called only > > if (patch != klp_transition_patch). Otherwise, it should have > > already been called. It is not the current case but see below. > > Ditto. This is related to the other comment where I and Josh suggested to call klp_pre_unpatch_callback() in __klp_disable_patch() before the transition started. It means that the callback has already been called for klp_transition_patch. Best Regards, Petr