Re: [PATCH v7 00/10] livepatch: klp-convert tool

Re: [PATCH v7 00/10] livepatch: klp-convert tool

[Alexey Dobriyan]

Joe Lawrence wrote:
> +static int update_strtab(struct elf *elf)
> +{
>
> +	buf = malloc(new_size);
> +	if (!buf) {
> +		WARN("malloc failed");
> +		return -1;
> +	}
> +	memcpy(buf, (void *)strtab->data, orig_size);

This code is called realloc(). :-)

> +static int write_file(struct elf *elf, const char *file)
> +{
>
> +	fd = creat(file, 0664);
> +	e = elf_begin(fd, ELF_C_WRITE, NULL);

elf_end() doesn't close descriptor, so there is potentially corrupted
data. There is no unlink() call if writes fail as well.

> +void elf_close(struct elf *elf)
> +{
> +
> +	if (elf->fd > 0)
> +		close(elf->fd);

Techically, it is "fd >= 0".

> +filechk_klp_map = \
> +	echo "klp-convert-symbol-data.0.1";		\
> +	echo "*vmlinux";				\
> +	$(NM) -f posix vmlinux | cut -d\  -f1;		\
> +	sort $(MODORDER) $(MODULES_LIVEPATCH) |		\

This probably should be "LC_ALL=C sort" for speed and reproducibility (?).

Re: [PATCH v7 00/10] livepatch: klp-convert tool

[Alexey Dobriyan]

This patchset somehow breaks the build of the simplest livepatch module:

	make -f linux/linux-1/scripts/Makefile.modfinal
	make[1]: *** No rule to make target 'linux/module-klp/main.tmp.ko', needed by 'linux/module-klp/main.ko'.  Stop.

$ cat Kbuild
obj-m := main.o

$ cat main.c
#include <linux/module.h>
#include <linux/kernel.h>
#include <linux/livepatch.h>
#include <linux/seq_file.h>

static int livepatch_cmdline_proc_show(struct seq_file *m, void *data)
{
	seq_puts(m, "REDACTED 001\n");
	return 0;
}

static struct klp_func funcs[] = {
	{
		.old_name = "cmdline_proc_show",
		.new_func = livepatch_cmdline_proc_show,
	},
	{}
};

static struct klp_object objs[] = {
	{
		.funcs = funcs,
	},
	{}
};

static struct klp_patch g_patch = {
	.mod = THIS_MODULE,
	.objs = objs,
};

static int livepatch_init(void)
{
	return klp_enable_patch(&g_patch);
}

static void livepatch_exit(void)
{
}
module_init(livepatch_init);
module_exit(livepatch_exit);
MODULE_LICENSE("GPL");
MODULE_INFO(livepatch, "Y");

Re: [PATCH v7 00/10] livepatch: klp-convert tool

[Joe Lawrence]

On 3/30/23 08:10, Alexey Dobriyan wrote:
> Joe Lawrence wrote:
>> +static int update_strtab(struct elf *elf)
>> +{
>>
>> +	buf = malloc(new_size);
>> +	if (!buf) {
>> +		WARN("malloc failed");
>> +		return -1;
>> +	}
>> +	memcpy(buf, (void *)strtab->data, orig_size);
> 
> This code is called realloc(). :-)
> 
>> +static int write_file(struct elf *elf, const char *file)
>> +{
>>
>> +	fd = creat(file, 0664);
>> +	e = elf_begin(fd, ELF_C_WRITE, NULL);
> 
> elf_end() doesn't close descriptor, so there is potentially corrupted
> data. There is no unlink() call if writes fail as well.
> 
>> +void elf_close(struct elf *elf)
>> +{
>> +
>> +	if (elf->fd > 0)
>> +		close(elf->fd);
> 
> Techically, it is "fd >= 0".
> 
>> +filechk_klp_map = \
>> +	echo "klp-convert-symbol-data.0.1";		\
>> +	echo "*vmlinux";				\
>> +	$(NM) -f posix vmlinux | cut -d\  -f1;		\
>> +	sort $(MODORDER) $(MODULES_LIVEPATCH) |		\
> 
> This probably should be "LC_ALL=C sort" for speed and reproducibility (?).
> 

Thanks, will incorporate these into the next version.

-- 
Joe

Re: [PATCH v7 00/10] livepatch: klp-convert tool

[Joe Lawrence]

On 3/30/23 13:04, Alexey Dobriyan wrote:
> This patchset somehow breaks the build of the simplest livepatch module:
> 
> 	make -f linux/linux-1/scripts/Makefile.modfinal
> 	make[1]: *** No rule to make target 'linux/module-klp/main.tmp.ko', needed by 'linux/module-klp/main.ko'.  Stop.
> 

Thanks for testing.

Presumably this is an out-of-tree livepatch module?  If so, that is
still on the TODO list.  If not, that is weird as the patchset itself
includes updates to samples/ and lib/ livepatches that build and load fine.

-- 
Joe

> $ cat Kbuild
> obj-m := main.o
> 
> $ cat main.c
> #include <linux/module.h>
> #include <linux/kernel.h>
> #include <linux/livepatch.h>
> #include <linux/seq_file.h>
> 
> static int livepatch_cmdline_proc_show(struct seq_file *m, void *data)
> {
> 	seq_puts(m, "REDACTED 001\n");
> 	return 0;
> }
> 
> static struct klp_func funcs[] = {
> 	{
> 		.old_name = "cmdline_proc_show",
> 		.new_func = livepatch_cmdline_proc_show,
> 	},
> 	{}
> };
> 
> static struct klp_object objs[] = {
> 	{
> 		.funcs = funcs,
> 	},
> 	{}
> };
> 
> static struct klp_patch g_patch = {
> 	.mod = THIS_MODULE,
> 	.objs = objs,
> };
> 
> static int livepatch_init(void)
> {
> 	return klp_enable_patch(&g_patch);
> }
> 
> static void livepatch_exit(void)
> {
> }
> module_init(livepatch_init);
> module_exit(livepatch_exit);
> MODULE_LICENSE("GPL");
> MODULE_INFO(livepatch, "Y");
> 

Re: [PATCH v7 00/10] livepatch: klp-convert tool

[Alexey Dobriyan]

On Fri, Mar 31, 2023 at 12:03:52PM -0400, Joe Lawrence wrote:
> On 3/30/23 13:04, Alexey Dobriyan wrote:
> > This patchset somehow breaks the build of the simplest livepatch module:
> > 
> > 	make -f linux/linux-1/scripts/Makefile.modfinal
> > 	make[1]: *** No rule to make target 'linux/module-klp/main.tmp.ko', needed by 'linux/module-klp/main.ko'.  Stop.
> > 
> 
> Thanks for testing.
> 
> Presumably this is an out-of-tree livepatch module?  If so, that is
> still on the TODO list.  If not, that is weird as the patchset itself
> includes updates to samples/ and lib/ livepatches that build and load fine.

Yes, this is external module.

make doesn't see this implicit rule despite it is hiding in the open:

	+%.tmp.ko: %.o %.mod.o symbols.klp FORCE
	+	+$(call if_changed,ld_ko_o)

Re: [PATCH v7 00/10] livepatch: klp-convert tool

[Marcos Paulo de Souza]

On Wed, May 03, 2023 at 03:54:47PM -0400, Joe Lawrence wrote:
> On 4/11/23 06:06, Nicolai Stange wrote:
> > Josh Poimboeuf <jpoimboe@kernel.org> writes:
> > 
> >> On Fri, Mar 17, 2023 at 04:29:48PM -0400, Joe Lawrence wrote:
> >>> Have you tried retrofitting klp-convert into any real-world livepatch?
> >>> I'm curious as to your observations on the overall experience, or
> >>> thoughts on the sympos annotation style noted above.
> >>
> >> On a related note, the patch creation process (of which klp-convert
> >> would be part of) needs to be documented.
> >>
> >> If I remember correctly, the proper safe usage of klp-convert requires a
> >> kernel built with -flive-patching, plus some scripting and/or manual
> >> processes.
> > 
> > Not always, I think: -flive-patching or IPA optimizations in general
> > aren't a concern in the context of data symbols. From a quick glance, it
> > seems like the selftests introduced as part of this patchset are
> > all restricted to this usecase.
> > 
> 
> IIRC there is nothing currently stopping klp-convert from converting
> function symbol relocations.  That may be dangerous when taking
> optimizations like sibling functions (and their sharing of stack) into
> consideration.  This is about the point I stopped to turn and see what
> the real use cases may be.
> 
> >> If nobody knows how to safely use it then there wouldn't be much value
> >> in merging it.
> > 
> > I tend to agree, but would put it a bit differently: the current
> > implementation of klp-convert features quite some convenience logic,
> > which, until the question of a documented livepatch preparation process
> > has been settled, is not known yet to ever be of any use.
> > 
> 
> Good observation and perhaps something that Marcos could elaborate on
> (pros and cons of klp-convert in his experiments).
> 

In my tests, I took the exact code generated by klp-ccp, and adapted it to not
rely on kallsyms anymore, removing the symbol lookups. For data symbols, I
changed it to be a extern variable instead of a pointer to it. For the function
symbols, also removed the pointer lookup, and left only the function prototype,
and it worked as expected.

I was quite surprised that it worked quite well. But I agree with
Nicolai that the tool itself could be shrunk into a smaller version. In our
usage, klp-ccp knows all unexported functions and to which modules they belong,
as it's currently used for the symbol lookup. I believe that kpatch-build also
has similar information so all the symbols.klp and other Kbuild machinery could be
avoided, making the tool responsible to only generate the klp relocations based
on the undefined symbols, just as proposed by Nicolai.

Re: [PATCH v7 00/10] livepatch: klp-convert tool

[Joe Lawrence]

On 4/11/23 06:06, Nicolai Stange wrote:
> Josh Poimboeuf <jpoimboe@kernel.org> writes:
> 
>> On Fri, Mar 17, 2023 at 04:29:48PM -0400, Joe Lawrence wrote:
>>> Have you tried retrofitting klp-convert into any real-world livepatch?
>>> I'm curious as to your observations on the overall experience, or
>>> thoughts on the sympos annotation style noted above.
>>
>> On a related note, the patch creation process (of which klp-convert
>> would be part of) needs to be documented.
>>
>> If I remember correctly, the proper safe usage of klp-convert requires a
>> kernel built with -flive-patching, plus some scripting and/or manual
>> processes.
> 
> Not always, I think: -flive-patching or IPA optimizations in general
> aren't a concern in the context of data symbols. From a quick glance, it
> seems like the selftests introduced as part of this patchset are
> all restricted to this usecase.
> 

IIRC there is nothing currently stopping klp-convert from converting
function symbol relocations.  That may be dangerous when taking
optimizations like sibling functions (and their sharing of stack) into
consideration.  This is about the point I stopped to turn and see what
the real use cases may be.

>> If nobody knows how to safely use it then there wouldn't be much value
>> in merging it.
> 
> I tend to agree, but would put it a bit differently: the current
> implementation of klp-convert features quite some convenience logic,
> which, until the question of a documented livepatch preparation process
> has been settled, is not known yet to ever be of any use.
> 

Good observation and perhaps something that Marcos could elaborate on
(pros and cons of klp-convert in his experiments).

> For example, from [3/10]:
> 
>   "For automatic resolution of livepatch relocations, a file called
>    symbols.klp is used. This file maps symbols within every compiled kernel
>    object allowing the identification of symbols whose name is unique, thus
>    relocation can be automatically inferred, or providing information that
>    helps developers when code annotation is required for solving the
>    matter."
> 
> For the source based approach to livepatch preparation we're using
> internally, this is not really needed: the entity generating the source
> -- be it klp-ccp or the author doing it manually -- needs to examine the
> target objects long before link resp. klp-convert time for which symbols
> can be referenced from the livepatch and how (i.e. determine a potential
> sympos). I would expect it works similar for kpatch-build conceptually,
> albeit kpatch-build probably doesn't rely on any external utility like
> klp-convert for the .klp.* relas generation at all.
> 

Yes the conversion for kpatch-build the conversion is internal, though
split in parts:

The first step is during the binary comparison program
(create-diff-object) to place all the potential klp-relocations into a
temporary .kpatch_relocations section with info like dest, type,
objname, etc.

A second program, either create-klp-module [1] or the mostly deprecated
create-kpatch-module, iterates through the intermediate
.kpatch_relocation sections and converts to klp-relocation in the final
module output.

[1]
https://github.com/dynup/kpatch/blob/master/kpatch-build/create-klp-module.c#L158

> So with that, I agree that merging the klp-convert patchset in its
> current form with those potentially unused convenience features,
> presumably born out of certain assumptions about a manual livepatch
> preparation process, indeed can be argued about, probably.
> 
> 
> However, OTOH, there's currently no means whatsoever to create those
> .klp.* relas (*) (**) and I would like to propose resorting to a more
> minimal utility doing only that single thing: to stubbornly create
> .klp.* relas out of certain "regular" ones using a very simple
> transformation rule and nothing else beyond that. The "stripped"
> klp-convert would have no knowledge of the symbols available in the
> livepatched target objects at all, i.e. there would be no symbols.klp
> file or alike anymore. Instead, it would simply walk through all of a
> livepatch object's SHN_UNDEF symbols of format
> ".klp.sym.<loading-obj-name>.<foo-providing-mod>.some_foo,0" somewhen at
> modpost time and
> - rename the symbol to ".klp.sym.<foo-providing-mod>.some_foo,0" --
>   shortening the name should always be feasible as far as strtab is
>   concerned.
> - turn the symbol's SHN_UNDEF into SHN_LIVEPATCH
> - move any relocation (initially created by the compiler with source
>   based lp preparation approaches) against this symbol into a separate,
>   newly created rela section with flag SHF_RELA_LIVEPATCH set and whose
>   name is of format
>   .klp.rela.<loading-obj-name>.<livepatch-obj-dst-section-name>.
>   Furthermore, the new .klp.rela section's ->sh_info needs to be made to
>   refer to the destination section.
> 

If I understand the idea, it would be similar to the second kpatch-build
step I mentioned above -- nothing fancy, just blindly convert symbols
marked SHN_UNDEF and a given naming convention.

FWIW, it may even be possible to retrofit kpatch-build to the format for
testing -- that would offer the ability to test many real-world CVE fixups.

> So, the only thing which would depend on the yet unspecified details of
> the livepatch preparation process would be the creation of those
> intermediate
> ".klp.sym.<loading-obj-name>.<foo-providing-mod>.some_foo,0" SHN_UNDEF
> symbols to be processed by klp-convert. For source based livepatch
> preparation approaches, counting in the selftests, this can be easily
> controlled by means of asm("...") alias specifications at the respective
> declarations like in e.g.  extern int foo
> asm("\".klp.sym.<loading-obj-name>.<foo-providing-mod>.some_foo,0\"");
> 
> 
> I imagine the first ones to benefit from having such a "stripped"
> klp-convert available in the kernel tree would be new upstream selftests
> for .klp.* rela coverage (like introduced with this here patchset
> already) and for those some means of creating .klp.* relas would be
> needed anyway. We (SUSE), and perhaps others as well, could integrate
> this "stripped" klp-convert into our source based, production livepatch
> preparation workflows right away, of course, and so we're obviously keen
> on having it. Such a tool providing only the bare minimum would be
> pretty much self-contained -- it would only need to hook into the
> modpost Kbuild stage one way or the other -- and we could certainly
> maintain it downstream out-of-tree, but that would potentially only
> contribute to the current fragmentation around the livepatch creation
> processes even more and there still wouldn't have a solution for the
> upstream selftests.
> 
I think you summed it up that having an in-tree user, a test suite at
that, is a good motivation to maintain the conversion tool alongside.
Logistically it may easier to prototype and spin bugfixes faster locally
(esp. if it's supporting a quick CVE mitigation service), but hopefully
the rapid-dev/fix phase is only temporary and the normal upstream-first
development flow prevails.

> What do you think, does it make sense to eventually have such a bare
> minimum klp-convert merged in-tree, independently of the ongoing
> discussion around the livepatch preparation processes, respectively (the
> lack of) documentation around it? If yes, Lukas, now on CC, is
> interested in this topic and would be willing to help out in any form
> desired: either by contributing to Joe's work here or, if deemed more
> feasible, to start out completely new from scratch -- dependent on your
> opinion on the proposed, more minimal approach as well as on Joe's plans
> around klp-convert.
> 

I like the incremental approach of minimizing the complexity of the
conversion tool.  In fact, the hardest parts of this patchset (for me)
revolved around the kbuild integration to generate that symbols file :)
The next hardest where hypothetical problems with resolving to weird
read-only sections and then approaching compiler optimizations.

-- 
Joe

Re: [PATCH v7 00/10] livepatch: klp-convert tool

[Marcos Paulo de Souza]

On Tue, Apr 11, 2023 at 12:06:43PM +0200, Nicolai Stange wrote:
> Josh Poimboeuf <jpoimboe@kernel.org> writes:
> 
> > On Fri, Mar 17, 2023 at 04:29:48PM -0400, Joe Lawrence wrote:
> >> Have you tried retrofitting klp-convert into any real-world livepatch?
> >> I'm curious as to your observations on the overall experience, or
> >> thoughts on the sympos annotation style noted above.
> >
> > On a related note, the patch creation process (of which klp-convert
> > would be part of) needs to be documented.
> >
> > If I remember correctly, the proper safe usage of klp-convert requires a
> > kernel built with -flive-patching, plus some scripting and/or manual
> > processes.
> 
> Not always, I think: -flive-patching or IPA optimizations in general
> aren't a concern in the context of data symbols. From a quick glance, it
> seems like the selftests introduced as part of this patchset are
> all restricted to this usecase.

<snip>

> What do you think, does it make sense to eventually have such a bare
> minimum klp-convert merged in-tree, independently of the ongoing
> discussion around the livepatch preparation processes, respectively (the
> lack of) documentation around it? If yes, Lukas, now on CC, is
> interested in this topic and would be willing to help out in any form
> desired: either by contributing to Joe's work here or, if deemed more
> feasible, to start out completely new from scratch -- dependent on your
> opinion on the proposed, more minimal approach as well as on Joe's plans
> around klp-convert.
> 
> Looking forward to hearing your feedback!

So guys, any feedback? Ping :)

> 
> Thanks,
> 
> Nicolai
> 
> (*) We've been experimenting with building the relocation records
>     manually by various means, e.g. with GNU as' .reloc directive as an
>     example, but this all turned out impractical for various
>     reasons. Most noteworthy, because the records' offsets wouldn't get
>     adjusted properly when linking AFAIR.
> 
> (**) by some other means than directly with kpatch-build
> 
> -- 
> SUSE Software Solutions Germany GmbH, Frankenstraße 146, 90461 Nürnberg, Germany
> GF: Ivo Totev, Andrew Myers, Andrew McDonald, Boudien Moerman
> (HRB 36809, AG Nürnberg)

Re: [PATCH v7 00/10] livepatch: klp-convert tool

[Marcos Paulo de Souza]

> Testing
> -------
> 
> The patchset selftests build and execute on x86_64, s390x, and ppc64le
> for both default config (with added livepatch dependencies) and a larger
> RHEL-9-ish config.
> 
> Using the Intel's Linux Kernel Performance tests's make.cross,
> klp-convert builds and processes livepatch .ko's for x86_64 ppc64le
> ppc32 s390 arm64 arches.
> 

So I tested using real x86 livepatches, and it worked as expected. The test was
done by taking the livepatches, removing all externalized variables (those that
we need to call kallsyms_lookup), and compiling as usual and using
klp-convert to create the necessary relocation entries.

I'll keep testing with more livepatches for now on, and let you know if I find
any issue.

Re: [PATCH v7 00/10] livepatch: klp-convert tool

[Nicolai Stange]

Josh Poimboeuf <jpoimboe@kernel.org> writes:

> On Fri, Mar 17, 2023 at 04:29:48PM -0400, Joe Lawrence wrote:
>> Have you tried retrofitting klp-convert into any real-world livepatch?
>> I'm curious as to your observations on the overall experience, or
>> thoughts on the sympos annotation style noted above.
>
> On a related note, the patch creation process (of which klp-convert
> would be part of) needs to be documented.
>
> If I remember correctly, the proper safe usage of klp-convert requires a
> kernel built with -flive-patching, plus some scripting and/or manual
> processes.

Not always, I think: -flive-patching or IPA optimizations in general
aren't a concern in the context of data symbols. From a quick glance, it
seems like the selftests introduced as part of this patchset are
all restricted to this usecase.


> If nobody knows how to safely use it then there wouldn't be much value
> in merging it.

I tend to agree, but would put it a bit differently: the current
implementation of klp-convert features quite some convenience logic,
which, until the question of a documented livepatch preparation process
has been settled, is not known yet to ever be of any use.

For example, from [3/10]:

  "For automatic resolution of livepatch relocations, a file called
   symbols.klp is used. This file maps symbols within every compiled kernel
   object allowing the identification of symbols whose name is unique, thus
   relocation can be automatically inferred, or providing information that
   helps developers when code annotation is required for solving the
   matter."

For the source based approach to livepatch preparation we're using
internally, this is not really needed: the entity generating the source
-- be it klp-ccp or the author doing it manually -- needs to examine the
target objects long before link resp. klp-convert time for which symbols
can be referenced from the livepatch and how (i.e. determine a potential
sympos). I would expect it works similar for kpatch-build conceptually,
albeit kpatch-build probably doesn't rely on any external utility like
klp-convert for the .klp.* relas generation at all.

So with that, I agree that merging the klp-convert patchset in its
current form with those potentially unused convenience features,
presumably born out of certain assumptions about a manual livepatch
preparation process, indeed can be argued about, probably.


However, OTOH, there's currently no means whatsoever to create those
.klp.* relas (*) (**) and I would like to propose resorting to a more
minimal utility doing only that single thing: to stubbornly create
.klp.* relas out of certain "regular" ones using a very simple
transformation rule and nothing else beyond that. The "stripped"
klp-convert would have no knowledge of the symbols available in the
livepatched target objects at all, i.e. there would be no symbols.klp
file or alike anymore. Instead, it would simply walk through all of a
livepatch object's SHN_UNDEF symbols of format
".klp.sym.<loading-obj-name>.<foo-providing-mod>.some_foo,0" somewhen at
modpost time and
- rename the symbol to ".klp.sym.<foo-providing-mod>.some_foo,0" --
  shortening the name should always be feasible as far as strtab is
  concerned.
- turn the symbol's SHN_UNDEF into SHN_LIVEPATCH
- move any relocation (initially created by the compiler with source
  based lp preparation approaches) against this symbol into a separate,
  newly created rela section with flag SHF_RELA_LIVEPATCH set and whose
  name is of format
  .klp.rela.<loading-obj-name>.<livepatch-obj-dst-section-name>.
  Furthermore, the new .klp.rela section's ->sh_info needs to be made to
  refer to the destination section.

So, the only thing which would depend on the yet unspecified details of
the livepatch preparation process would be the creation of those
intermediate
".klp.sym.<loading-obj-name>.<foo-providing-mod>.some_foo,0" SHN_UNDEF
symbols to be processed by klp-convert. For source based livepatch
preparation approaches, counting in the selftests, this can be easily
controlled by means of asm("...") alias specifications at the respective
declarations like in e.g.  extern int foo
asm("\".klp.sym.<loading-obj-name>.<foo-providing-mod>.some_foo,0\"");


I imagine the first ones to benefit from having such a "stripped"
klp-convert available in the kernel tree would be new upstream selftests
for .klp.* rela coverage (like introduced with this here patchset
already) and for those some means of creating .klp.* relas would be
needed anyway. We (SUSE), and perhaps others as well, could integrate
this "stripped" klp-convert into our source based, production livepatch
preparation workflows right away, of course, and so we're obviously keen
on having it. Such a tool providing only the bare minimum would be
pretty much self-contained -- it would only need to hook into the
modpost Kbuild stage one way or the other -- and we could certainly
maintain it downstream out-of-tree, but that would potentially only
contribute to the current fragmentation around the livepatch creation
processes even more and there still wouldn't have a solution for the
upstream selftests.

What do you think, does it make sense to eventually have such a bare
minimum klp-convert merged in-tree, independently of the ongoing
discussion around the livepatch preparation processes, respectively (the
lack of) documentation around it? If yes, Lukas, now on CC, is
interested in this topic and would be willing to help out in any form
desired: either by contributing to Joe's work here or, if deemed more
feasible, to start out completely new from scratch -- dependent on your
opinion on the proposed, more minimal approach as well as on Joe's plans
around klp-convert.

Looking forward to hearing your feedback!

Thanks,

Nicolai

(*) We've been experimenting with building the relocation records
    manually by various means, e.g. with GNU as' .reloc directive as an
    example, but this all turned out impractical for various
    reasons. Most noteworthy, because the records' offsets wouldn't get
    adjusted properly when linking AFAIR.

(**) by some other means than directly with kpatch-build

-- 
SUSE Software Solutions Germany GmbH, Frankenstraße 146, 90461 Nürnberg, Germany
GF: Ivo Totev, Andrew Myers, Andrew McDonald, Boudien Moerman
(HRB 36809, AG Nürnberg)

Re: [PATCH v7 00/10] livepatch: klp-convert tool

[Marcos Paulo de Souza]

On Fri, Mar 17, 2023 at 04:29:48PM -0400, Joe Lawrence wrote:
> On Tue, Mar 14, 2023 at 05:23:56PM -0300, Marcos Paulo de Souza wrote:
> > On Mon, Mar 06, 2023 at 09:08:14AM -0500, Joe Lawrence wrote:
> > > Summary
> > > -------
> > > 
> > > 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 infer 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 a
> > > symbols.klp 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.  For example. see the saved_command_line
> > > variable in lib/livepatch/test_klp_convert2.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.  See the KLP_MODULE_RELOC / KLP_SYMPOS annotation sections
> > > in lib/livepatch/test_klp_convert{1,2}.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 by
> > > removing the KLP_MODULE_RELOC / KLP_SYMPOS annotation sections in
> > > lib/livepatch/test_klp_convert{1,2}.c.
> > > 
> > > Selftests have been added to exercise these klp-convert use-cases
> > > through several tests.
> > > 
> > > 
> > > Testing
> > > -------
> > > 
> > > The patchset selftests build and execute on x86_64, s390x, and ppc64le
> > > for both default config (with added livepatch dependencies) and a larger
> > > RHEL-9-ish config.
> > > 
> > > Using the Intel's Linux Kernel Performance tests's make.cross,
> > > klp-convert builds and processes livepatch .ko's for x86_64 ppc64le
> > > ppc32 s390 arm64 arches.
> > > 
> > > 
> > > Summary of changes in v7
> > > ------------------------
> > > 
> > > - rebase for v6.2
> > > - combine ("livepatch: Add klp-convert tool") with ("livepatch: Add
> > >   klp-convert annotation helpers")
> > > - combine ("kbuild: Support for symbols.klp creation") with ("modpost:
> > >   Integrate klp-convert") to simplify Kbuild magic [Petr, Nicolas]
> > > - klp-convert: add safe_snprintf() (-Wsign-compare)
> > > - klp-convert: fix -Wsign-compare warnings
> > > - klp-convert: use calloc() where appropriate
> > > - klp-convert: copy ELF e_flags
> > > - selftests: fix various build warnings
> > > - klp-convert: WARN msg simplification, failed sanity checks, and sympos
> > >   comment [Marcos]
> > > - klp-convert: fix elf_write_file() error paths [Petr]
> > 
> > Thanks for the new version Joe. I've run the ksefltests on my x86 laptop, and it
> > succeed as expected, so
> > 
> > Tested-by: Marcos Paulo de Souza <mpdesouza@suse.com>
> > 
> 
> Thanks for the testing and reviews, Marcos.
> 
> The selftests are the first level of testing... we should probably
> tackle a real or simulated CVE fix to see how well the tooling fits
> larger livepatches.

Our plan is to start testing new livepatches with klp-convert to ensure that it
works as expected, removing the need of kallsyms_lookup. Let's see how it goes
in the next few weeks.

> 
> One complication that I can envision is symbol positioning.  Currently,
> the klp-convert annotations are a direct mirror of the kernel's
> <obj,symbol,pos> tuple.  It should be possible to make this a bit more
> user friendly for the livepatch developer if the annotations were
> <obj,file,symbol>, as derived from the vmlinux / module.tmp.ko symbol
> tables.
> 
> For example, the following code:
> 
>   KLP_MODULE_RELOC(test_klp_convert_mod, test_klp_convert_mod_b.c) test_klp_convert_mod_relocs_b[] = {
>         KLP_SYMPOS(homonym_string),
>         KLP_SYMPOS(get_homonym_string),
>   };
> 
> could generate the following relocations:
> 
>   Relocation section '.rela.klp.module_relocs.test_klp_convert_mod.test_klp_convert_mod_b.c' at offset 0x1dc0 contains 2 entries:
>       Offset             Info             Type               Symbol's Value  Symbol's Name + Addend
>   0000000000000000  0000003f00000001 R_X86_64_64            0000000000000000 homonym_string + 0
>   0000000000000008  0000004900000001 R_X86_64_64            0000000000000000 get_homonym_string + 0
> 
> for which klp-convert looks up in symbols.klp:
> 
>   klp-convert-symbol-data.0.2
>   *vmlinux
>   ...
>   *test_klp_convert_mod
>   -test_klp_convert_mod_a.c             << added filenames to the format
>   test_klp_get_driver_name
>   driver_name
>   get_homonym_string                    << sympos = 1
>   homonym_string                        << sympos = 1
>   ...
>   -test_klp_convert_mod_b.c
>   get_homonym_string                    << sympos = 2
>   homonym_string                        << sympos = 2
>   ...
> 
> and then generates the usual klp-relocations as currently defined.

This can get verbose very quickly, but I liked the idea. The sympos is much more
cryptic than specifying the file where the symbol lives on.

> 
> (Unfortunately full pathnames are not saved in the STT_FILE symbol table
> entries, so there will be a few non-unique <obj,file,symbol> entries.  I
> believe the last time this was discussed, we found that there were a
> relatively small number of such symbols.)
> 
> Have you tried retrofitting klp-convert into any real-world livepatch?
> I'm curious as to your observations on the overall experience, or
> thoughts on the sympos annotation style noted above.

As I mentioned I'll start using klp-convert with new livepaches for testing
purposes in the next coming weeks. I'll reply in a few weeks about the
experience so far.

> 
> Regards,
> 
> -- Joe
> 

Re: [PATCH v7 00/10] livepatch: klp-convert tool

[Joe Lawrence]

On 3/17/23 19:20, Josh Poimboeuf wrote:
> On Fri, Mar 17, 2023 at 04:29:48PM -0400, Joe Lawrence wrote:
>> Have you tried retrofitting klp-convert into any real-world livepatch?
>> I'm curious as to your observations on the overall experience, or
>> thoughts on the sympos annotation style noted above.
> 
> On a related note, the patch creation process (of which klp-convert
> would be part of) needs to be documented.
> 
> If I remember correctly, the proper safe usage of klp-convert requires a
> kernel built with -flive-patching, plus some scripting and/or manual
> processes.
> 
> If nobody knows how to safely use it then there wouldn't be much value
> in merging it.
> 

I took a trip though my inbox and dug up a 2020 discussion on
documenting livepatch and compiler considerations [1].  This led to the
suggestion of a greater, end-to-end livepatch author guide, for which
everyone agreed, but has since remained unwritten :/

If we have a miniconf @ LPC this year, maybe we can volunteer to write
sections, decide on an outline, or even better, have a rough draft to
review and discuss?


Aside: technically klp-convert doesn't require -flive-patching, but it's
probably strongly recommended in order to use it safely.  And fwiw,
kpatch-build could leverage the tool should it desire one day.  In the
meantime, if kpatch-build doesn't need/want to use it, perhaps
klp-convert should have its own CONFIG option?  (Or something in modinfo
to key on.)


[1]
https://lore.kernel.org/live-patching/20200721161407.26806-1-joe.lawrence@redhat.com/

-- 
Joe

Re: [PATCH v7 00/10] livepatch: klp-convert tool

[Josh Poimboeuf]

On Fri, Mar 17, 2023 at 04:29:48PM -0400, Joe Lawrence wrote:
> Have you tried retrofitting klp-convert into any real-world livepatch?
> I'm curious as to your observations on the overall experience, or
> thoughts on the sympos annotation style noted above.

On a related note, the patch creation process (of which klp-convert
would be part of) needs to be documented.

If I remember correctly, the proper safe usage of klp-convert requires a
kernel built with -flive-patching, plus some scripting and/or manual
processes.

If nobody knows how to safely use it then there wouldn't be much value
in merging it.

-- 
Josh

Re: [PATCH v7 00/10] livepatch: klp-convert tool

[Joe Lawrence]

On Tue, Mar 14, 2023 at 05:23:56PM -0300, Marcos Paulo de Souza wrote:
> On Mon, Mar 06, 2023 at 09:08:14AM -0500, Joe Lawrence wrote:
> > Summary
> > -------
> > 
> > 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 infer 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 a
> > symbols.klp 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.  For example. see the saved_command_line
> > variable in lib/livepatch/test_klp_convert2.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.  See the KLP_MODULE_RELOC / KLP_SYMPOS annotation sections
> > in lib/livepatch/test_klp_convert{1,2}.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 by
> > removing the KLP_MODULE_RELOC / KLP_SYMPOS annotation sections in
> > lib/livepatch/test_klp_convert{1,2}.c.
> > 
> > Selftests have been added to exercise these klp-convert use-cases
> > through several tests.
> > 
> > 
> > Testing
> > -------
> > 
> > The patchset selftests build and execute on x86_64, s390x, and ppc64le
> > for both default config (with added livepatch dependencies) and a larger
> > RHEL-9-ish config.
> > 
> > Using the Intel's Linux Kernel Performance tests's make.cross,
> > klp-convert builds and processes livepatch .ko's for x86_64 ppc64le
> > ppc32 s390 arm64 arches.
> > 
> > 
> > Summary of changes in v7
> > ------------------------
> > 
> > - rebase for v6.2
> > - combine ("livepatch: Add klp-convert tool") with ("livepatch: Add
> >   klp-convert annotation helpers")
> > - combine ("kbuild: Support for symbols.klp creation") with ("modpost:
> >   Integrate klp-convert") to simplify Kbuild magic [Petr, Nicolas]
> > - klp-convert: add safe_snprintf() (-Wsign-compare)
> > - klp-convert: fix -Wsign-compare warnings
> > - klp-convert: use calloc() where appropriate
> > - klp-convert: copy ELF e_flags
> > - selftests: fix various build warnings
> > - klp-convert: WARN msg simplification, failed sanity checks, and sympos
> >   comment [Marcos]
> > - klp-convert: fix elf_write_file() error paths [Petr]
> 
> Thanks for the new version Joe. I've run the ksefltests on my x86 laptop, and it
> succeed as expected, so
> 
> Tested-by: Marcos Paulo de Souza <mpdesouza@suse.com>
> 

Thanks for the testing and reviews, Marcos.

The selftests are the first level of testing... we should probably
tackle a real or simulated CVE fix to see how well the tooling fits
larger livepatches.

One complication that I can envision is symbol positioning.  Currently,
the klp-convert annotations are a direct mirror of the kernel's
<obj,symbol,pos> tuple.  It should be possible to make this a bit more
user friendly for the livepatch developer if the annotations were
<obj,file,symbol>, as derived from the vmlinux / module.tmp.ko symbol
tables.

For example, the following code:

  KLP_MODULE_RELOC(test_klp_convert_mod, test_klp_convert_mod_b.c) test_klp_convert_mod_relocs_b[] = {
        KLP_SYMPOS(homonym_string),
        KLP_SYMPOS(get_homonym_string),
  };

could generate the following relocations:

  Relocation section '.rela.klp.module_relocs.test_klp_convert_mod.test_klp_convert_mod_b.c' at offset 0x1dc0 contains 2 entries:
      Offset             Info             Type               Symbol's Value  Symbol's Name + Addend
  0000000000000000  0000003f00000001 R_X86_64_64            0000000000000000 homonym_string + 0
  0000000000000008  0000004900000001 R_X86_64_64            0000000000000000 get_homonym_string + 0

for which klp-convert looks up in symbols.klp:

  klp-convert-symbol-data.0.2
  *vmlinux
  ...
  *test_klp_convert_mod
  -test_klp_convert_mod_a.c             << added filenames to the format
  test_klp_get_driver_name
  driver_name
  get_homonym_string                    << sympos = 1
  homonym_string                        << sympos = 1
  ...
  -test_klp_convert_mod_b.c
  get_homonym_string                    << sympos = 2
  homonym_string                        << sympos = 2
  ...

and then generates the usual klp-relocations as currently defined.

(Unfortunately full pathnames are not saved in the STT_FILE symbol table
entries, so there will be a few non-unique <obj,file,symbol> entries.  I
believe the last time this was discussed, we found that there were a
relatively small number of such symbols.)

Have you tried retrofitting klp-convert into any real-world livepatch?
I'm curious as to your observations on the overall experience, or
thoughts on the sympos annotation style noted above.

Regards,

-- Joe

Re: [PATCH v7 00/10] livepatch: klp-convert tool

[Marcos Paulo de Souza]

On Mon, Mar 06, 2023 at 09:08:14AM -0500, Joe Lawrence wrote:
> Summary
> -------
> 
> 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 infer 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 a
> symbols.klp 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.  For example. see the saved_command_line
> variable in lib/livepatch/test_klp_convert2.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.  See the KLP_MODULE_RELOC / KLP_SYMPOS annotation sections
> in lib/livepatch/test_klp_convert{1,2}.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 by
> removing the KLP_MODULE_RELOC / KLP_SYMPOS annotation sections in
> lib/livepatch/test_klp_convert{1,2}.c.
> 
> Selftests have been added to exercise these klp-convert use-cases
> through several tests.
> 
> 
> Testing
> -------
> 
> The patchset selftests build and execute on x86_64, s390x, and ppc64le
> for both default config (with added livepatch dependencies) and a larger
> RHEL-9-ish config.
> 
> Using the Intel's Linux Kernel Performance tests's make.cross,
> klp-convert builds and processes livepatch .ko's for x86_64 ppc64le
> ppc32 s390 arm64 arches.
> 
> 
> Summary of changes in v7
> ------------------------
> 
> - rebase for v6.2
> - combine ("livepatch: Add klp-convert tool") with ("livepatch: Add
>   klp-convert annotation helpers")
> - combine ("kbuild: Support for symbols.klp creation") with ("modpost:
>   Integrate klp-convert") to simplify Kbuild magic [Petr, Nicolas]
> - klp-convert: add safe_snprintf() (-Wsign-compare)
> - klp-convert: fix -Wsign-compare warnings
> - klp-convert: use calloc() where appropriate
> - klp-convert: copy ELF e_flags
> - selftests: fix various build warnings
> - klp-convert: WARN msg simplification, failed sanity checks, and sympos
>   comment [Marcos]
> - klp-convert: fix elf_write_file() error paths [Petr]

Thanks for the new version Joe. I've run the ksefltests on my x86 laptop, and it
succeed as expected, so

Tested-by: Marcos Paulo de Souza <mpdesouza@suse.com>

> 
> 
> Previous versions
> -----------------
> 
> RFC:
>   https://lore.kernel.org/lkml/cover.1477578530.git.jpoimboe@redhat.com/
> v2:
>   https://lore.kernel.org/lkml/f52d29f7-7d1b-ad3d-050b-a9fa8878faf2@redhat.com/
> v3:
>   https://lore.kernel.org/lkml/20190410155058.9437-1-joe.lawrence@redhat.com/
> v4:
>   https://lore.kernel.org/lkml/20190509143859.9050-1-joe.lawrence@redhat.com/
> v5:
>   (not posted)
>   https://github.com/joe-lawrence/klp-convert-tree/tree/klp-convert-v5-devel
> v6:
>   https://lore.kernel.org/live-patching/20220216163940.228309-1-joe.lawrence@redhat.com/
> 
> 
> Joe Lawrence (10):
>   livepatch: Create and include UAPI headers
>   livepatch: Add klp-convert tool
>   kbuild/modpost: create symbols.klp and integrate klp-convert
>   livepatch: Add sample livepatch module
>   documentation: Update on livepatch elf format
>   livepatch/selftests: add klp-convert
>   livepatch/selftests: test multiple sections
>   livepatch/selftests: add __asm__ symbol renaming examples
>   livepatch/selftests: add data relocations test
>   livepatch/selftests: add static keys test
> 
>  .gitignore                                    |   2 +
>  Documentation/dontdiff                        |   1 +
>  Documentation/livepatch/livepatch.rst         |   3 +
>  Documentation/livepatch/module-elf-format.rst |  42 +-
>  MAINTAINERS                                   |   2 +
>  Makefile                                      |  16 +-
>  include/linux/livepatch.h                     |  13 +
>  include/uapi/linux/livepatch.h                |  25 +
>  kernel/livepatch/core.c                       |   4 +-
>  lib/livepatch/Makefile                        |  12 +
>  lib/livepatch/test_klp_convert.h              |  45 +
>  lib/livepatch/test_klp_convert1.c             | 121 +++
>  lib/livepatch/test_klp_convert2.c             | 110 +++
>  lib/livepatch/test_klp_convert_data.c         | 190 ++++
>  lib/livepatch/test_klp_convert_keys.c         |  91 ++
>  lib/livepatch/test_klp_convert_keys_mod.c     |  52 +
>  lib/livepatch/test_klp_convert_mod_a.c        |  31 +
>  lib/livepatch/test_klp_convert_mod_b.c        |  19 +
>  lib/livepatch/test_klp_convert_mod_c.c        |  36 +
>  lib/livepatch/test_klp_convert_sections.c     | 120 +++
>  samples/livepatch/Makefile                    |   1 +
>  .../livepatch/livepatch-annotated-sample.c    |  93 ++
>  scripts/Makefile                              |   1 +
>  scripts/Makefile.modfinal                     |  33 +
>  scripts/Makefile.modpost                      |   5 +
>  scripts/livepatch/.gitignore                  |   1 +
>  scripts/livepatch/Makefile                    |   5 +
>  scripts/livepatch/elf.c                       | 817 ++++++++++++++++
>  scripts/livepatch/elf.h                       |  74 ++
>  scripts/livepatch/klp-convert.c               | 893 ++++++++++++++++++
>  scripts/livepatch/klp-convert.h               |  47 +
>  scripts/livepatch/list.h                      | 391 ++++++++
>  scripts/mod/modpost.c                         |  28 +-
>  scripts/mod/modpost.h                         |   1 +
>  .../selftests/livepatch/test-livepatch.sh     | 403 ++++++++
>  35 files changed, 3716 insertions(+), 12 deletions(-)
>  create mode 100644 include/uapi/linux/livepatch.h
>  create mode 100644 lib/livepatch/test_klp_convert.h
>  create mode 100644 lib/livepatch/test_klp_convert1.c
>  create mode 100644 lib/livepatch/test_klp_convert2.c
>  create mode 100644 lib/livepatch/test_klp_convert_data.c
>  create mode 100644 lib/livepatch/test_klp_convert_keys.c
>  create mode 100644 lib/livepatch/test_klp_convert_keys_mod.c
>  create mode 100644 lib/livepatch/test_klp_convert_mod_a.c
>  create mode 100644 lib/livepatch/test_klp_convert_mod_b.c
>  create mode 100644 lib/livepatch/test_klp_convert_mod_c.c
>  create mode 100644 lib/livepatch/test_klp_convert_sections.c
>  create mode 100644 samples/livepatch/livepatch-annotated-sample.c
>  create mode 100644 scripts/livepatch/.gitignore
>  create mode 100644 scripts/livepatch/Makefile
>  create mode 100644 scripts/livepatch/elf.c
>  create mode 100644 scripts/livepatch/elf.h
>  create mode 100644 scripts/livepatch/klp-convert.c
>  create mode 100644 scripts/livepatch/klp-convert.h
>  create mode 100644 scripts/livepatch/list.h
> 
> -- 
> 2.39.2
> 

[PATCH v7 00/10] livepatch: klp-convert tool

[Joe Lawrence]

Summary
-------

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 infer 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 a
symbols.klp 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.  For example. see the saved_command_line
variable in lib/livepatch/test_klp_convert2.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.  See the KLP_MODULE_RELOC / KLP_SYMPOS annotation sections
in lib/livepatch/test_klp_convert{1,2}.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 by
removing the KLP_MODULE_RELOC / KLP_SYMPOS annotation sections in
lib/livepatch/test_klp_convert{1,2}.c.

Selftests have been added to exercise these klp-convert use-cases
through several tests.


Testing
-------

The patchset selftests build and execute on x86_64, s390x, and ppc64le
for both default config (with added livepatch dependencies) and a larger
RHEL-9-ish config.

Using the Intel's Linux Kernel Performance tests's make.cross,
klp-convert builds and processes livepatch .ko's for x86_64 ppc64le
ppc32 s390 arm64 arches.


Summary of changes in v7
------------------------

- rebase for v6.2
- combine ("livepatch: Add klp-convert tool") with ("livepatch: Add
  klp-convert annotation helpers")
- combine ("kbuild: Support for symbols.klp creation") with ("modpost:
  Integrate klp-convert") to simplify Kbuild magic [Petr, Nicolas]
- klp-convert: add safe_snprintf() (-Wsign-compare)
- klp-convert: fix -Wsign-compare warnings
- klp-convert: use calloc() where appropriate
- klp-convert: copy ELF e_flags
- selftests: fix various build warnings
- klp-convert: WARN msg simplification, failed sanity checks, and sympos
  comment [Marcos]
- klp-convert: fix elf_write_file() error paths [Petr]


Previous versions
-----------------

RFC:
  https://lore.kernel.org/lkml/cover.1477578530.git.jpoimboe@redhat.com/
v2:
  https://lore.kernel.org/lkml/f52d29f7-7d1b-ad3d-050b-a9fa8878faf2@redhat.com/
v3:
  https://lore.kernel.org/lkml/20190410155058.9437-1-joe.lawrence@redhat.com/
v4:
  https://lore.kernel.org/lkml/20190509143859.9050-1-joe.lawrence@redhat.com/
v5:
  (not posted)
  https://github.com/joe-lawrence/klp-convert-tree/tree/klp-convert-v5-devel
v6:
  https://lore.kernel.org/live-patching/20220216163940.228309-1-joe.lawrence@redhat.com/


Joe Lawrence (10):
  livepatch: Create and include UAPI headers
  livepatch: Add klp-convert tool
  kbuild/modpost: create symbols.klp and integrate klp-convert
  livepatch: Add sample livepatch module
  documentation: Update on livepatch elf format
  livepatch/selftests: add klp-convert
  livepatch/selftests: test multiple sections
  livepatch/selftests: add __asm__ symbol renaming examples
  livepatch/selftests: add data relocations test
  livepatch/selftests: add static keys test

 .gitignore                                    |   2 +
 Documentation/dontdiff                        |   1 +
 Documentation/livepatch/livepatch.rst         |   3 +
 Documentation/livepatch/module-elf-format.rst |  42 +-
 MAINTAINERS                                   |   2 +
 Makefile                                      |  16 +-
 include/linux/livepatch.h                     |  13 +
 include/uapi/linux/livepatch.h                |  25 +
 kernel/livepatch/core.c                       |   4 +-
 lib/livepatch/Makefile                        |  12 +
 lib/livepatch/test_klp_convert.h              |  45 +
 lib/livepatch/test_klp_convert1.c             | 121 +++
 lib/livepatch/test_klp_convert2.c             | 110 +++
 lib/livepatch/test_klp_convert_data.c         | 190 ++++
 lib/livepatch/test_klp_convert_keys.c         |  91 ++
 lib/livepatch/test_klp_convert_keys_mod.c     |  52 +
 lib/livepatch/test_klp_convert_mod_a.c        |  31 +
 lib/livepatch/test_klp_convert_mod_b.c        |  19 +
 lib/livepatch/test_klp_convert_mod_c.c        |  36 +
 lib/livepatch/test_klp_convert_sections.c     | 120 +++
 samples/livepatch/Makefile                    |   1 +
 .../livepatch/livepatch-annotated-sample.c    |  93 ++
 scripts/Makefile                              |   1 +
 scripts/Makefile.modfinal                     |  33 +
 scripts/Makefile.modpost                      |   5 +
 scripts/livepatch/.gitignore                  |   1 +
 scripts/livepatch/Makefile                    |   5 +
 scripts/livepatch/elf.c                       | 817 ++++++++++++++++
 scripts/livepatch/elf.h                       |  74 ++
 scripts/livepatch/klp-convert.c               | 893 ++++++++++++++++++
 scripts/livepatch/klp-convert.h               |  47 +
 scripts/livepatch/list.h                      | 391 ++++++++
 scripts/mod/modpost.c                         |  28 +-
 scripts/mod/modpost.h                         |   1 +
 .../selftests/livepatch/test-livepatch.sh     | 403 ++++++++
 35 files changed, 3716 insertions(+), 12 deletions(-)
 create mode 100644 include/uapi/linux/livepatch.h
 create mode 100644 lib/livepatch/test_klp_convert.h
 create mode 100644 lib/livepatch/test_klp_convert1.c
 create mode 100644 lib/livepatch/test_klp_convert2.c
 create mode 100644 lib/livepatch/test_klp_convert_data.c
 create mode 100644 lib/livepatch/test_klp_convert_keys.c
 create mode 100644 lib/livepatch/test_klp_convert_keys_mod.c
 create mode 100644 lib/livepatch/test_klp_convert_mod_a.c
 create mode 100644 lib/livepatch/test_klp_convert_mod_b.c
 create mode 100644 lib/livepatch/test_klp_convert_mod_c.c
 create mode 100644 lib/livepatch/test_klp_convert_sections.c
 create mode 100644 samples/livepatch/livepatch-annotated-sample.c
 create mode 100644 scripts/livepatch/.gitignore
 create mode 100644 scripts/livepatch/Makefile
 create mode 100644 scripts/livepatch/elf.c
 create mode 100644 scripts/livepatch/elf.h
 create mode 100644 scripts/livepatch/klp-convert.c
 create mode 100644 scripts/livepatch/klp-convert.h
 create mode 100644 scripts/livepatch/list.h

-- 
2.39.2