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@xxxxxxx>
> ---
> 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
--
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
