Messages in this thread | | | Date | Tue, 9 Dec 2014 12:32:49 -0600 | From | Josh Poimboeuf <> | Subject | Re: [PATCH 1/6] livepatch v5: avoid race when checking for state of the patch |
| |
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.
> > Signed-off-by: Petr Mladek <pmladek@suse.cz> > --- > kernel/livepatch/core.c | 16 ++++++++++------ > 1 file changed, 10 insertions(+), 6 deletions(-) > > diff --git a/kernel/livepatch/core.c b/kernel/livepatch/core.c > index d6d0f50e81f8..b848069e44cc 100644 > --- a/kernel/livepatch/core.c > +++ b/kernel/livepatch/core.c > @@ -425,11 +425,13 @@ int klp_disable_patch(struct klp_patch *patch) > { > int ret; > > - if (!klp_is_enabled()) > - return -ENODEV; > - > mutex_lock(&klp_mutex); > > + if (!klp_is_enabled()) { > + ret = -ENODEV; > + goto err; > + } > + > if (!klp_patch_is_registered(patch)) { > ret = -EINVAL; > goto err; > @@ -489,11 +491,13 @@ int klp_enable_patch(struct klp_patch *patch) > { > int ret; > > - if (!klp_is_enabled()) > - return -ENODEV; > - > mutex_lock(&klp_mutex); > > + if (!klp_is_enabled()) { > + ret = -ENODEV; > + goto err; > + } > + > if (!klp_patch_is_registered(patch)) { > ret = -EINVAL; > goto err; > -- > 1.8.5.2 >
-- Josh
| |