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
