linux-kernel - Re: [PATCH v14 03/11] livepatch: Consolidate klp

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]

Message-ID: <20181206081501.jkki7hg4u43fqwth@pathway.suse.cz>
Date:   Thu, 6 Dec 2018 09:15:01 +0100
From:   Petr Mladek <pmladek@...e.com>
To:     Joe Lawrence <joe.lawrence@...hat.com>
Cc:     Jiri Kosina <jikos@...nel.org>,
        Josh Poimboeuf <jpoimboe@...hat.com>,
        Miroslav Benes <mbenes@...e.cz>,
        Jason Baron <jbaron@...mai.com>,
        Evgenii Shatokhin <eshatokhin@...tuozzo.com>,
        live-patching@...r.kernel.org, linux-kernel@...r.kernel.org,
        Jessica Yu <jeyu@...nel.org>
Subject: Re: [PATCH v14 03/11] livepatch: Consolidate klp_free functions

On Wed 2018-12-05 14:02:20, Joe Lawrence wrote:
> On Thu, Nov 29, 2018 at 10:44:23AM +0100, Petr Mladek wrote:
> > The code for freeing livepatch structures is a bit scattered and tricky:
> > 
> > [ ... snip ... ]
> >
> > +static int klp_init_patch(struct klp_patch *patch)
> > +{
> > +	struct klp_object *obj;
> > +	int ret;
> > +
> > +	mutex_lock(&klp_mutex);
> > +
> > +	ret = klp_init_patch_before_free(patch);
> >  	if (ret) {
> >  		mutex_unlock(&klp_mutex);
> >  		return ret;
> >  	}
> > 
> 
> I believe klp_init_patch_before_free() accumulates more responsibilities
> later in the patchset, but I'll ask here: does it really need the
> klp_mutex since it looks to be operating only on the klp_patch, its
> objects and functions? 

I do not have a strong opinion about it.

On one hand, we are manipulating all the structures and should prevent
any parallel use. On the other hand, the rest of the code will not
touch the patch until it is added into klp_patches list or until
the sysfs interface is created.

If you think that it might cause false expectations and confusions
then I could move it out of the lock.

Well, in the final version we need to call klp_check_patch_conflict()
under the mutex.

Best Regards,
Petr