On Wed 2014-12-10 09:25:06, Josh Poimboeuf wrote: > On Wed, Dec 10, 2014 at 11:11:47AM +0100, Petr Mladek wrote: > > On Tue 2014-12-09 12:32:49, Josh Poimboeuf wrote: > > > On Tue, Dec 09, 2014 at 07:05:02PM +0100, Petr Mladek wrote: > > > > klp_patch_enable() and klp_patch_disable() should check the current state > > > > of the patch under the klp_lock. Otherwise, it might detect that the operation > > > > is valid but the situation might change before it takes the lock. > > > > > > Hi Petr, > > > > > > Thanks for the patches. > > > > > > I don't think this patch is necessary. klp_is_enabled() doesn't check > > > the state of the patch. It checks the initialization state of the core > > > module (klp_root_kobj), which can only be set in klp_init(). It's not > > > protected by the lock, so I don't see the point of this patch. > > > > Ah, I have misread the name and expected that it checked whether > > the patch was enabled or disabled. The original code is OK then. > > > > Well, Jiri Kosina pointed out that the check did not make much sense. > > klp_is_enabled() could not be called if the livepatch module is not > > loaded. And the later check for klp_patch_is_registered() is enough > > to check whether the klp_enable_patch()/klp_disable_patch() calls > > are allowed or not. > > But livepatch isn't a module, it's part of the kernel. Ah, I remembered that module_init(klp_init) and created a wrong mental link ;-) > Even if the init > function returns an error, that doesn't prevent any of the other > exported functions from getting called. Well, it still will be covered by that later klp_patch_is_registered() check. But I am find with leaving it as is. Best Regards, Petr -- To unsubscribe from this list: send the line "unsubscribe live-patching" in the body of a message to majordomo@xxxxxxxxxxxxxxx More majordomo info at http://vger.kernel.org/majordomo-info.html
