linux-kernel - Re: [PATCH 6/8] livepatch: Remove Nop structures when unused

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 linux-hardening linux-cve-announce 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, 10 Apr 2018 13:09:22 +0200
From:   Petr Mladek <pmladek@...e.com>
To:     Miroslav Benes <mbenes@...e.cz>
Cc:     Jiri Kosina <jikos@...nel.org>,
        Josh Poimboeuf <jpoimboe@...hat.com>,
        Jason Baron <jbaron@...mai.com>,
        Joe Lawrence <joe.lawrence@...hat.com>,
        Jessica Yu <jeyu@...nel.org>,
        Evgenii Shatokhin <eshatokhin@...tuozzo.com>,
        live-patching@...r.kernel.org, linux-kernel@...r.kernel.org
Subject: Re: [PATCH 6/8] livepatch: Remove Nop structures when unused

On Tue 2018-04-10 11:14:21, Miroslav Benes wrote:
> On Fri, 23 Mar 2018, Petr Mladek wrote:
> > This patch allows to unpatch and free the dynamic structures independently
> > when the transition finishes.
> > 
> > The free part is a bit tricky because kobject free callbacks are called
> > asynchronously. We could not wait for them easily. Fortunately, we do
> > not have to. Any further access can be avoided by removing them from
> > the dynamic lists.
> > 
> > Finally, the patch become the first on the stack when enabled. The replace
> > functionality will not longer be needed. Let's clear patch->replace to
> > avoid the special handling when it is eventually disabled/enabled again.
> > 
> > diff --git a/include/linux/livepatch.h b/include/linux/livepatch.h
> > index d6e6d8176995..1635b30bb1ec 100644
> > --- a/include/linux/livepatch.h
> > +++ b/include/linux/livepatch.h
> > @@ -172,6 +172,9 @@ struct klp_patch {
> >  #define klp_for_each_object_static(patch, obj) \
> >  	for (obj = patch->objs; obj->funcs || obj->name; obj++)
> >  
> > +#define klp_for_each_object_safe(patch, obj, tmp_obj)		\
> > +	list_for_each_entry_safe(obj, tmp_obj, &patch->obj_list, node)
> > +
> >  #define klp_for_each_object(patch, obj)	\
> >  	list_for_each_entry(obj, &patch->obj_list, node)
> >  
> > @@ -180,6 +183,9 @@ struct klp_patch {
> >  	     func->old_name || func->new_func || func->old_sympos; \
> >  	     func++)
> >  
> > +#define klp_for_each_func_safe(obj, func, tmp_func)			\
> > +	list_for_each_entry_safe(func, tmp_func, &obj->func_list, node)
> > +
> >  #define klp_for_each_func(obj, func)	\
> >  	list_for_each_entry(func, &obj->func_list, node)
> 
> Is there a benefit of the newly added iterators?

You are right that there is nothing special and it is used
on a single place only.

Well, it increases the chance that you will catch it when looking
for the iterators. Also it makes it easier to see the difference
against the non-safe iterators. I'll keep it if you are not
strongly against it.

Best Regards,
Petr