From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received: (majordomo@vger.kernel.org) by vger.kernel.org via listexpand id S1754952AbcH3PQB (ORCPT ); Tue, 30 Aug 2016 11:16:01 -0400 Received: from mail-oi0-f50.google.com ([209.85.218.50]:35675 "EHLO mail-oi0-f50.google.com" rhost-flags-OK-OK-OK-OK) by vger.kernel.org with ESMTP id S1752325AbcH3PP7 (ORCPT ); Tue, 30 Aug 2016 11:15:59 -0400 Date: Tue, 30 Aug 2016 10:15:56 -0500 From: Christopher Arges To: Petr Mladek Cc: live-patching@vger.kernel.org, Josh Poimboeuf , Jessica Yu , Jiri Kosina , Miroslav Benes , linux-kernel@vger.kernel.org Subject: Re: [PATCH] livepatch: add load/unload hooks to objects Message-ID: <20160830151555.GA5196@gmail.com> References: <1472237448-22270-1-git-send-email-chris.j.arges@canonical.com> <1472237448-22270-2-git-send-email-chris.j.arges@canonical.com> <20160829152330.GN4866@pathway.suse.cz> <20160829161627.GA32390@gmail.com> <20160830144330.GB4554@pathway.suse.cz> MIME-Version: 1.0 Content-Type: text/plain; charset=us-ascii Content-Disposition: inline In-Reply-To: <20160830144330.GB4554@pathway.suse.cz> User-Agent: Mutt/1.6.0 (2016-04-01) Sender: linux-kernel-owner@vger.kernel.org List-ID: X-Mailing-List: linux-kernel@vger.kernel.org On Tue, Aug 30, 2016 at 04:43:30PM +0200, Petr Mladek wrote: > On Mon 2016-08-29 11:16:28, Christopher Arges wrote: > > On Mon, Aug 29, 2016 at 05:23:30PM +0200, Petr Mladek wrote: > > > On Fri 2016-08-26 13:50:27, Chris J Arges wrote: > > > > It can be useful to execute hook functions whenever a livepatch is applied > > > > or unapplied to a particular object. Currently this is possible by writing > > > > logic in the __init function of the livepatch kernel module. However to > > > > handle executing functions when a module loads requires an additional > > > > module notifier to be set up with the correct priority. > > > > > > > > By using load/unload hooks we can execute these functions using the > > > > existing livepatch notifier infrastructure and ensure consistent ordering > > > > of notifications. > > > > > > > > The load hook executes right before enabling functions, and the unload hook > > > > executes right after disabling functions. > > > > > > Could you please provide an example(s), what these hooks will be > > > useful for? > > > > > > The callbacks will still need to be implemented in the patch module. > > > If they are generally useful, it would make sense to implement them > > > in the livepatch code directly, so they get more review and are > > > shared. > > > > > > Best Regards, > > > Petr > > > > These hooks could be used as a yet another tool to implement a specific patch. > > And yes, the callbacks to these hooks will be part of the patch module. > > > > If there are 'hooks' that are applicable generically to livepatch they should > > absolutely go into the core code. > > > > As an example, CVE-2015-5307 requires that a bit be set in the exception bitmap > > in order to handle #AC exceptions. One could write code in the init function of > > the patch that checks if the module is loaded and then applies this fix. Or if > > hooks are available, write a load hook that sets this structure whenever the > > patch is loaded and the kvm module is loaded. In the future when patch > > unloading is possible, one could also write an unload hook to return the > > exception bitmap back to normal as the patched function(s) may not be available > > any longer. > > Also this change looks racy when done by the hooks. I did not study it > in detail. But I wonder if it is correct to set the bit in the mask > before update_exception_bitmap() and ac_interception() are avalable. > > My feeling is that you try to find a solution for something that > need to be supported by a more strict consistency model. You > try to change values of structures that might already be in use > and we need to be very careful here. > This is a good point. Perhaps the strict consistency will obviate the need for hooks of this sort. > Your hooks are called for both already loaded objects and for objects > that are being loaded. Something that is safe for a module in COMMING > state might be dangerous for an already loaded one. > > Best Regards, > Petr Yea maybe this should have been [DRAFT RFC], I think more thought will need to be done here about how to handle modifying existing data structures (and I see you already have a proposal for this during plumbers). In both cases; however I see the need for allowing patch authors to be able to write some custom logic to safely handle changing existing data structures. This could also be dependent on any user-space tooling requirements too. --chris