Re: [PATCH v2 2/6] livepatch: Add klp-convert tool

Joe Lawrence
2024-05-30T20:07:27Z

On Thu, May 16, 2024 at 03:30:05PM +0200, Lukas Hruska wrote:
> Livepatches need to access external symbols which can't be handled
> by the normal relocation mechanism. It is needed for two types
> of symbols:
>
> + Symbols which can be local for the original livepatched function.
> The alternative implementation in the livepatch sees them
> as external symbols.
>
> + Symbols in modules which are exported via EXPORT_SYMBOL*(). They
> must be handled special way otherwise the livepatch module would
> depend on the livepatched one. Loading such livepatch would cause
> loading the other module as well.
>
> The address of these symbols can be found via kallsyms. Or they can
> be relocated using livepatch specific relocation sections as 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 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, create scripts/livepatch to hold tools developed for
> livepatches and add source files for klp-convert there.
>
> Allow to annotate such external symbols in the livepatch by a macro
> KLP_RELOC_SYMBOL(). It will create symbol with all needed
> metadata. For example:
>
> extern char *saved_command_line \
> KLP_RELOC_SYMBOL(vmlinux, vmlinux, saved_command_line, 0);
>
> would create symbol
>
> $>readelf -r -W <compiled livepatch module>:
> Relocation section '.rela.text' at offset 0x32e60 contains 10 entries:
> Offset Info Type Symbol's Value Symbol's Name + Addend
> [...]
> 0000000000000068 0000003c00000002 R_X86_64_PC32 0000000000000000 .klp.sym.rela.vmlinux.vmlinux.saved_command_line,0 - 4
> [...]
>
> Also add scripts/livepatch/klp-convert. The tool transforms symbols
> created by KLP_RELOC_SYMBOL() to object specific rela sections
> and rela entries which would later be proceed when the livepatch
> or the livepatched object is loaded.
>
> For example, klp-convert would replace the above symbol with:
>
> $> readelf -r -W <livepatch_module_proceed_by_klp_convert>
> Relocation section '.klp.rela.vmlinux.text' at offset 0x5cb60 contains 1 entry:
> Offset Info Type Symbol's Value Symbol's Name + Addend
> 0000000000000068 0000003c00000002 R_X86_64_PC32 0000000000000000 .klp.sym.vmlinux.saved_command_line,0 - 4
>
> klp-convert relies on libelf and on a list implementation. Add files
> scripts/livepatch/elf.c and scripts/livepatch/elf.h, which are a libelf
> interfacing layer and scripts/livepatch/list.h, which is a list
> implementation.
>
> Update Makefiles to correctly support the compilation of the new tool,
> update MAINTAINERS file and add a .gitignore file.
>
> [jpoimboe@redhat.com: initial version]
> Signed-off-by: Josh Poimboeuf <jpoimboe@redhat.com>
> [joe.lawrence@redhat.com: clean-up and fixes]
> Signed-off-by: Joe Lawrence <joe.lawrence@redhat.com>
> [lhruska@suse.cz: klp-convert code, minimal approach]
> Signed-off-by: Lukas Hruska <lhruska@suse.cz>
> Reviewed-by: Marcos Paulo de Souza <mpdesouza@suse.com>
> ---
> MAINTAINERS | 1 +
> include/linux/livepatch.h | 19 +
> scripts/Makefile | 1 +
> scripts/livepatch/.gitignore | 1 +
> scripts/livepatch/Makefile | 5 +
> scripts/livepatch/elf.c | 817 ++++++++++++++++++++++++++++++++
> scripts/livepatch/elf.h | 73 +++
> scripts/livepatch/klp-convert.c | 284 +++++++++++
> scripts/livepatch/klp-convert.h | 23 +
> scripts/livepatch/list.h | 391 +++++++++++++++
> 10 files changed, 1615 insertions(+)
> 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
>
> diff --git a/MAINTAINERS b/MAINTAINERS
> index 130b8b0bd4f7..d2facc1f4e15 100644
> --- a/MAINTAINERS
> +++ b/MAINTAINERS
> @@ -12618,6 +12618,7 @@ F: include/uapi/linux/livepatch.h
> F: kernel/livepatch/
> F: kernel/module/livepatch.c
> F: samples/livepatch/
> +F: scripts/livepatch/
> F: tools/testing/selftests/livepatch/
>
> LLC (802.2)
> diff --git a/include/linux/livepatch.h b/include/linux/livepatch.h
> index 9b9b38e89563..83bbcd1c43fd 100644
> --- a/include/linux/livepatch.h
> +++ b/include/linux/livepatch.h
> @@ -235,6 +235,25 @@ int klp_apply_section_relocs(struct module *pmod, Elf_Shdr *sechdrs,
> unsigned int symindex, unsigned int secindex,
> const char *objname);
>
> +/**
> + * KLP_RELOC_SYMBOL_POS - define relocation for external symbols
> + *
> + * @LP_OBJ_NAME: name of the livepatched object where the symbol is needed
> + * @SYM_OBJ_NAME: name of the object where the symbol exists
> + * @SYM_NAME: symbol name
> + * @SYM_POS: position of the symbol in SYM_OBJ when there are more
> + * symbols of the same name.
> + *
> + * Use for annotating external symbols used in livepatches which are
> + * not exported in vmlinux or are in livepatched modules, see
> + * Documentation/livepatch/module-elf-format.rst
> + */
> +#define KLP_RELOC_SYMBOL_POS(LP_OBJ_NAME, SYM_OBJ_NAME, SYM_NAME, SYM_POS) \
> + asm("\".klp.sym.rela." #LP_OBJ_NAME "." #SYM_OBJ_NAME "." #SYM_NAME "," #SYM_POS "\"")
^^^
I think I found a potential bug, or at least compatiblity problem with
including a comma "," character in this symbol format and older versions
of the GNU assembler. The good news is that other delimiter characters
like "." or "#" seem to work out fine.

If you want to reproduce, you'll need a version of `as` like binutils
2.36.1 and try building the samples/livepatch/livepatch-extern-symbol.ko
and you should get an error like:

Assembler messages:
Warning: missing closing '"'
Warning: missing closing '"'
Error: too many memory references for `movq'

If you want to retrace my adventure, here are my steps:

1) Clone klp-convert-tree repo branch containing this patchset +
Petr's review comments + a few helpful things for klp-convert
development:

$ git clone \
--single-branch --branch=klp-convert-minimal-v1-review --depth=9 \
https://github.com/joe-lawrence/klp-convert-tree.git
[ ... snip ... ]
$ cd klp-convert-tree

2) Override .cross-dev defaults:

$ export BUILD_ARCHES=x86_64
$ export COMPILER=gcc-11.1.0
$ export URL=https://cdn.kernel.org/pub/tools/crosstool/files/bin/x86_64/
$ export OUTDIR_PREFIX=$(pwd)/build
$ export COMPILER_INSTALL_PATH=$(pwd)/cross-compiler

3) Setup x86_64 default .config (this will download and install the
gcc-11.1.0 compiler from cdn.kernel.org):

$ ./cross-dev make defconfig

x86_64 : make defconfig ...
Compiler will be installed in /root/klp-convert-tree/cross-compiler
[ ... snip ... ]

4) Add kernel livepatching configuration options:

$ ./cross-dev klp-config

Configuring x86_64 ...
[ ... snip ... ]

$ grep LIVEPATCH "$OUTDIR_PREFIX"-x86_64/.config
CONFIG_HAVE_LIVEPATCH=y
CONFIG_LIVEPATCH=y
CONFIG_SAMPLE_LIVEPATCH=m

5) Run the cross-compiler build until it hits a build error on
livepatch-extern-symbol.ko:

$ ./cross-dev make -j$(nproc)
[ ... snip ... ]
make: Target '__all' not remade because of errors.
[ x86_64 : make -j48 = FAIL ]

6) With pre-requisites already built, retry the external symbol sample
and add -save-temps to the KCFLAGS to keep the generated assembly file:

$ KCFLAGS="-save-temps=obj" ./cross-dev make samples/livepatch/livepatch-extern-symbol.ko
[ ... snip ... ]
samples/livepatch/livepatch-extern-symbol.s: Assembler messages:
samples/livepatch/livepatch-extern-symbol.s:103: Warning: missing closing '"'
samples/livepatch/livepatch-extern-symbol.s:103: Warning: missing closing '"'
samples/livepatch/livepatch-extern-symbol.s:103: Error: too many memory references for `movq'
[ ... snip ... ]

7) Which line is that?

$ awk 'NR==103' "$OUTDIR_PREFIX"-x86_64/samples/livepatch/livepatch-extern-symbol.s
movq ".klp.sym.rela.vmlinux.vmlinux.saved_command_line,0"(%rip), %rdx

You could alternatively poke at it through the compiler explorer service
and toggle the source and binutils versions:

(error) binutils 2.36.1 : https://godbolt.org/z/cGGs6rfWe
(success) binutils 2.38 : https://godbolt.org/z/ffzza3vYd

--
Joe