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

Lukas Hruska
2024-06-06T10:05:51Z

Hello Joe,
> > +#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.

Thank you for spotting this. I was using binutils 2.38, so I did not
even notice this problem. Unfortunately, I was not able to make it work
with "#" as a delimiter; only "." worked. Additionally, any type of
parenthesis apparently has some special purpose even in labels, so they
are also not an option.

> 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

Thank you for those detailed step-by-step instruction to reproduce it!
It helped me a lot to understand the problem.

Lukas