linux-kernel - Re: [PATCH v4] livepatch: introduce shadow variable API

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:   Fri, 18 Aug 2017 11:42:50 +0200
From:   Petr Mladek <pmladek@...e.com>
To:     Joe Lawrence <joe.lawrence@...hat.com>
Cc:     live-patching@...r.kernel.org, linux-kernel@...r.kernel.org,
        Josh Poimboeuf <jpoimboe@...hat.com>,
        Jessica Yu <jeyu@...hat.com>, Jiri Kosina <jikos@...nel.org>,
        Miroslav Benes <mbenes@...e.cz>
Subject: Re: [PATCH v4] livepatch: introduce shadow variable API

On Thu 2017-08-17 12:01:33, Joe Lawrence wrote:
> On 08/17/2017 10:05 AM, Petr Mladek wrote:
> >> diff --git a/kernel/livepatch/shadow.c b/kernel/livepatch/shadow.c
> >> new file mode 100644
> >> index 000000000000..0ebd4b635e4f
> >> --- /dev/null
> >> +++ b/kernel/livepatch/shadow.c
> >> +/**
> >> + * klp_shadow_match() - verify a shadow variable matches given <obj, id>
> >> + * @shadow:	shadow variable to match
> >> + * @obj:	pointer to parent object
> >> + * @id:		data identifier
> >> + *
> >> + * Return: true if the shadow variable matches.
> >> + *
> >> + * Callers should hold the klp_shadow_lock.
> >> + */
> >> +static inline bool klp_shadow_match(struct klp_shadow *shadow, void *obj,
> >> +				unsigned long id)
> >> +{
> >> +	return shadow->obj == obj && shadow->id == id;
> >> +}
> > 
> > Do we really need this function? It is called only in situations
> > where shadow->obj == obj is always true. Especially the use in
> > klp_shadow_detach_all() is funny because we pass shadow->obj as
> > the shadow parameter.
> 
> Personal preference.  Abstracting out all of the routines that operated
> on the shadow variables (setting up, comparison) did save some code
> lines and centralized these common bits.

I take this back. We actually need to check obj because different
objects might have the same hash.

I think that I did the same mistake also the last time. I hope that
I will be able to fix this in my mind faster than "never" vs. "newer"
typo that I do for years.

Also I forgot to say that you did great work. Each version of the
patch is much better than the previous one.

Best Regards,
Petr