Re: [PATCH 0/8] livepatch: klp-convert tool

[Josh Poimboeuf <jpoimboe@redhat.com>]

On Tue, Oct 10, 2017 at 04:17:10PM +0200, Miroslav Benes wrote:
> On Wed, 30 Aug 2017, Josh Poimboeuf wrote:
> 
> > On Tue, Aug 29, 2017 at 04:01:32PM -0300, Joao Moreira wrote:
> > > Livepatches may use symbols which are not contained in its own scope,
> > > and, because of that, may end up compiled with relocations that will
> > > only be resolved during module load. Yet, when the referenced symbols are
> > > not exported, solving this relocation requires information on the object
> > > that holds the symbol (either vmlinux or modules) and its position inside
> > > the object, as an object may contain multiple symbols with the same name.
> > > Providing such information must be done accordingly to what is specified
> > > in Documentation/livepatch/module-elf-format.txt.
> > > 
> > > Currently, there is no trivial way to embed the required information as
> > > requested in the final livepatch elf object. klp-convert solves this
> > > problem in two different forms: (i) by relying on a symbol map, which is
> > > built during kernel compilation, to automatically infers the relocation
> > > targeted symbol, and, when such inference is not possible (ii) by using
> > > annotations in the elf object to convert the relocation accordingly to
> > > the specification, enabling it to be handled by the livepatch loader.
> > > 
> > > Given the above, add support for symbol mapping in the form of
> > > Symbols.list file; add klp-convert tool; integrate klp-convert tool into
> > > kbuild; make livepatch modules discernible during kernel compilation
> > > pipeline; add data-structure and macros to enable users to annotate
> > > livepatch source code; make modpost stage compatible with livepatches;
> > > update livepatch-sample and update documentation.
> > > 
> > > The patch was tested under three use-cases:
> > > 
> > > use-case 1: There is a relocation in the lp that can be automatically
> > > resolved by klp-convert (tested by removing the annotations from
> > > samples/livepatch/livepatch-annotated-sample.c)
> > > 
> > > use-case 2: There is a relocation in the lp that cannot be automatically
> > > resolved, as the name of the respective symbol appears in multiple
> > > objects. The livepatch contains an annotation to enable a correct
> > > relocation - reproducible with this livepatch sample:
> > > www.livewire.com.br/suse/klp/livepatch-sample.1.c
> > > 
> > > use-case 3: There is a relocation in the lp that cannot be automatically
> > > resolved similarly as 2, but no annotation was provided in the livepatch,
> > > triggering an error during compilation - reproducible with this livepatch
> > > sample: www.livewire.com.br/suse/klp/livepatch-sample.2.c
> > > 
> > > Joao Moreira (2):
> > >   kbuild: Support for Symbols.list creation
> > >   documentation: Update on livepatch elf format
> > > 
> > > Josh Poimboeuf (5):
> > >   livepatch: Create and include UAPI headers
> > >   livepatch: Add klp-convert tool
> > >   livepatch: Add klp-convert annotation helpers
> > >   modpost: Integrate klp-convert
> > >   livepatch: Add sample livepatch module
> > > 
> > > Miroslav Benes (1):
> > >   modpost: Add modinfo flag to livepatch modules
> > 
> > Thanks a lot for picking these patches up and improving them.  I've only
> > glanced at the code, but so far it's looking good.  It may be a few
> > weeks before a I get a chance to do a proper review.
> > 
> > One quick question, possibly for Miroslav.  Do we have a plan yet for
> > dealing with GCC optimizations?
> > 
> >   https://lkml.kernel.org/r/20161110161053.heua3abuaekz4yy7@treble
> > 
> > I still like the '-fpreserve-function-abi' idea, but maybe it's not
> > realistic.
> 
> I'm sorry for the late response, I failed to reply immediately and then 
> completely forgot about it :(

No worries...

> I talked to Martin Jambor from gcc community month ago and he told me it 
> could be possible to do. But we need to come up with a good proposal. We 
> need a good description of what it should do and provide reasons why we 
> need it. I'll talk to him again tomorrow and I'll start to work on the 
> proposal.

Sounds good!  For klp-convert to be successful, we really need a
strategy for dealing with such optimizations.  I'm thinking that a
'-fpreserve-function-abi' flag would be the cleanest way to handle it.

If we don't have a strategy for dealing with optimizations, then we may
instead need to go with a binary diff-based tool like kpatch-build.

> Ideas are of course more than welcome.
> 
> However that might be the easier part. We need to find out what it would 
> mean for the whole kernel and its performance.

TBH, I'd be surprised if it affected kernel performance in any
measurable way.  The vast majority of non-inlined functions seem to
conform to function ABI.

-- 
Josh