lists.openwall.net   lists  /  announce  owl-users  owl-dev  john-users  john-dev  passwdqc-users  yescrypt  popa3d-users  /  oss-security  kernel-hardening  musl  sabotage  tlsify  passwords  /  crypt-dev  xvendor  /  Bugtraq  Full-Disclosure  linux-kernel  linux-netdev  linux-ext4  PHC 
Open Source and information security mailing list archives
 
Hash Suite: Windows password security audit tool. GUI, reports in PDF.
[<prev] [next>] [<thread-prev] [thread-next>] [day] [month] [year] [list]
Date:   Tue, 30 Apr 2019 13:00:05 +0200 (CEST)
From:   Miroslav Benes <mbenes@...e.cz>
To:     "Tobin C. Harding" <tobin@...nel.org>
cc:     Josh Poimboeuf <jpoimboe@...hat.com>,
        Jiri Kosina <jikos@...nel.org>, Petr Mladek <pmladek@...e.com>,
        Greg Kroah-Hartman <gregkh@...uxfoundation.org>,
        Joe Lawrence <joe.lawrence@...hat.com>,
        live-patching@...r.kernel.org, linux-kernel@...r.kernel.org
Subject: Re: [PATCH 2/2] livepatch: Use correct kobject cleanup function

On Tue, 30 Apr 2019, Tobin C. Harding wrote:

> The correct cleanup function after a call to kobject_init_and_add() has
> succeeded is kobject_del() _not_ kobject_put().  kobject_del() calls
> kobject_put().
> 
> Use correct cleanup function when removing a kobject.
> 
> Signed-off-by: Tobin C. Harding <tobin@...nel.org>
> ---
>  kernel/livepatch/core.c | 8 +++-----
>  1 file changed, 3 insertions(+), 5 deletions(-)
> 
> diff --git a/kernel/livepatch/core.c b/kernel/livepatch/core.c
> index 98a7bec41faa..4cce6bb6e073 100644
> --- a/kernel/livepatch/core.c
> +++ b/kernel/livepatch/core.c
> @@ -589,9 +589,8 @@ static void __klp_free_funcs(struct klp_object *obj, bool nops_only)
>  
>  		list_del(&func->node);
>  
> -		/* Might be called from klp_init_patch() error path. */

Could you leave the comment as is? If I am not mistaken, it is still 
valid. func->kobj_added check is here exactly because the function may be 
called as mentioned.

One could argue that the comment is not so important, but the change does 
not belong to the patch anyway in my opinion.

>  		if (func->kobj_added) {
> -			kobject_put(&func->kobj);
> +			kobject_del(&func->kobj);
>  		} else if (func->nop) {
>  			klp_free_func_nop(func);
>  		}
> @@ -625,9 +624,8 @@ static void __klp_free_objects(struct klp_patch *patch, bool nops_only)
>  
>  		list_del(&obj->node);
>  
> -		/* Might be called from klp_init_patch() error path. */

Same here.

>  		if (obj->kobj_added) {
> -			kobject_put(&obj->kobj);
> +			kobject_del(&obj->kobj);
>  		} else if (obj->dynamic) {
>  			klp_free_object_dynamic(obj);
>  		}
> @@ -676,7 +674,7 @@ static void klp_free_patch_finish(struct klp_patch *patch)
>  	 * cannot get enabled again.
>  	 */
>  	if (patch->kobj_added) {
> -		kobject_put(&patch->kobj);
> +		kobject_del(&patch->kobj);
>  		wait_for_completion(&patch->finish);
>  	}

Miroslav

Powered by blists - more mailing lists