linux-kernel - Re: [PATCH 2/2] kernel: add support for live patching

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: <20141107182103.GE1136@dhcp128.suse.cz>
Date:	Fri, 7 Nov 2014 19:21:03 +0100
From:	Petr Mladek <pmladek@...e.cz>
To:	Seth Jennings <sjenning@...hat.com>
Cc:	Jiri Slaby <jslaby@...e.cz>, Josh Poimboeuf <jpoimboe@...hat.com>,
	Jiri Kosina <jkosina@...e.cz>,
	Vojtech Pavlik <vojtech@...e.cz>,
	Steven Rostedt <rostedt@...dmis.org>,
	live-patching@...r.kernel.org, kpatch@...hat.com,
	linux-kernel@...r.kernel.org
Subject: Re: [PATCH 2/2] kernel: add support for live patching

On Thu 2014-11-06 10:57:48, Seth Jennings wrote:
> On Thu, Nov 06, 2014 at 04:51:02PM +0100, Jiri Slaby wrote:
> > On 11/06/2014, 03:39 PM, Seth Jennings wrote:
> > > +/*************************************
> > > + * Core structures
> > > + ************************************/
> > > +
> > > +/*
> > > + * lp_ structs vs lpc_ structs
> > > + *
> > > + * For each element (patch, object, func) in the live-patching code,
> > > + * there are two types with two different prefixes: lp_ and lpc_.
> > > + *
> > > + * Structures used by the live-patch modules to register with this core module
> > > + * are prefixed with lp_ (live patching).  These structures are part of the
> > > + * registration API and are defined in livepatch.h.  The structures used
> > > + * internally by this core module are prefixed with lpc_ (live patching core).
> > > + */
> > 
> > I am not sure if the separation and the allocations/kobj handling are
> > worth it. It makes the code really less understandable. Can we have just
> > struct lip_function (don't unnecessarily abbreviate), lip_objectfile
> > (object is too generic, like Java object) and lip_patch containing all
> > the needed information? It would clean up the code a lot. (Yes, we would
> > have profited from c++ here.)
> 
> I looked at doing this and this is actually what we did in kpatch.  We
> made one structure that had "private" members that the user wasn't
> suppose to access that were only used in the core.  This was messy
> though.  Every time you wanted to add a "private" field to the struct so
> the core could do something new, you were changing the API to the patch
> modules as well.  While copying the data into an internal structure does
> add code and opportunity for errors, that functionality is localized
> into functions that are specifically tasked with taking care of that.
> So the risk is minimized and we gain flexibility within the core and
> more self-documenting API structures.

I am not sure if the modified API is really such a big limit. The
modules initialize the needed members using ".member = value".
Also we do not need to take care of API/ABI backward compatibility because
there is very strict dependency between patches and the patched
kernel.

Well, I have to think more about it.

Best Regards,
Petr
--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majordomo@...r.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/