Re: [PATCH v2 5/8] modpost: Integrate klp-convert

From: Joe Lawrence
Date: Fri Mar 22 2019 - 14:29:36 EST

On 3/22/19 12:37 PM, Joao Moreira wrote:
On 2019-03-22 11:54, Joe Lawrence wrote:
On Fri, Mar 01, 2019 at 11:13:10AM -0300, Joao Moreira wrote:
From: Josh Poimboeuf <jpoimboe@xxxxxxxxxx>

Create cmd_klp_convert and hook it into scripts/Makefile.modpost.
cmd_klp_convert invokes klp-convert with the right arguments for the
conversion of unresolved symbols inside a livepatch.

* save cmd_ld_ko_o into .module.cmd, if_changed_rule doesn't do that
* fix bashisms for debian where /bin/sh is a symlink to /bin/dash
* rename rule_link_module to rule_ld_ko_o, otherwise arg-check inside
if_changed_rule compares cmd_link_module and cmd_ld_ko_o
* check modinfo -F livepatch only if CONFIG_LIVEPATCH is true

* remove modinfo call. LIVEPATCH_ in Makefiled

* split up: move the .livepatch file-based scheme for identifying
livepatches to a previous patch, as it was required for correctly
building Symbols.list there.

Signed-off-by: Josh Poimboeuf <jpoimboe@xxxxxxxxxx>
Signed-off-by: Konstantin Khlebnikov <khlebnikov@xxxxxxxxxxxxxx>
Signed-off-by: Miroslav Benes <mbenes@xxxxxxx>
Signed-off-by: Joao Moreira <jmoreira@xxxxxxx>
scripts/Kbuild.include | 4 +++-
scripts/Makefile.modpost | 16 +++++++++++++++-
scripts/mod/modpost.c | 6 +++++-
scripts/mod/modpost.h | 1 +
4 files changed, 24 insertions(+), 3 deletions(-)

[ ... snip ... ]

diff --git a/scripts/Makefile.modpost b/scripts/Makefile.modpost
index 7d4af0d0accb..da779a185218 100644
--- a/scripts/Makefile.modpost
+++ b/scripts/Makefile.modpost
@@ -125,8 +125,22 @@ quiet_cmd_ld_ko_o = LD [M] $@
-o $@ $(filter-out FORCE,$^) ;
$(if $(ARCH_POSTLINK), $(MAKE) -f $(ARCH_POSTLINK) $@, true)

+SLIST = $(objtree)/Symbols.list
+KLP_CONVERT = scripts/livepatch/klp-convert
+quiet_cmd_klp_convert = KLP $@
+ cmd_klp_convert = mv $@ $(@:.ko=.klp.o); \
+ $(KLP_CONVERT) $(SLIST) $(@:.ko=.klp.o) $@
+define rule_ld_ko_o
+ $(call cmd,ld_ko_o) $(cmd_ld_ko_o) ; \
Should there be a ';' semicolon here (and maybe a line-break) between
$(call cmd,ld_ko_o) and $(cmd_ld_ko_o)?

I didn't see this in my x86_64 VM, but on a ppc64le box, I kept getting
really strange build errors that I traced to this line. Without a
semicolon, the build was trying to run a make command with a linker
command smashed onto the end of it:

make -f ./arch/powerpc/Makefile.postlink crypto/xts.ko ld -r -EL -m
elf64lppc -T ./scripts/ -T
./arch/powerpc/kernel/ --save-restore-funcs --build-id -o
crypto/xts.ko crypto/xts.o crypto/xts.mod.o

Now, klp-convert looks like will need some ppc64le work as well, as
confused about .TOC. symbols:

CC samples/livepatch/livepatch-annotated-sample.mod.o
CC samples/livepatch/livepatch-callbacks-busymod.mod.o
CC samples/livepatch/livepatch-callbacks-demo.mod.o
CC samples/livepatch/livepatch-callbacks-mod.mod.o
CC samples/livepatch/livepatch-sample.mod.o
CC samples/livepatch/livepatch-shadow-fix1.mod.o
CC samples/livepatch/livepatch-shadow-fix2.mod.o
CC samples/livepatch/livepatch-shadow-mod.mod.o
LD [M] samples/livepatch/livepatch-annotated-sample.ko
LD [M] samples/livepatch/livepatch-callbacks-demo.ko
KLP samples/livepatch/livepatch-annotated-sample.ko
LD [M] samples/livepatch/livepatch-callbacks-mod.ko
KLP samples/livepatch/livepatch-callbacks-demo.ko
LD [M] samples/livepatch/livepatch-sample.ko
LD [M] samples/livepatch/livepatch-shadow-fix1.ko
klp-convert: Define KLP_SYMPOS for the symbol: .TOC.
Valid KLP_SYMPOS for symbol .TOC.:

[ ... snip listing of all .TOC's across the kernel ... ]

but we can save that for another day.

Hi Joe, first of all, thank you for you in-depth review. I did not have
the time to go through everything yet, and I'll reply properly to the
other comments soon. Yet, I would like to add a quick note here, since
you are testing ppc64le. ppc64le is exactly one of the things I have
been dealing with in the last two days, specifically the .TOC. symbols
(there is also another bug related to converting relocations of symbols
with index 0, but this one is architecture agnostic).

If you would like to take a look, the fix is in the latest commit here:

I would be very glad if you have any comment or input on that, and I
guess that if the fix is that simple, I can manage to squash it into v3.

Hi JoÃo,

I added on top of my branch with positive results:

(No build klp-convert build complaints)

% uname -r
Linux 5.1.0-rc1+ #6 SMP Thu Mar 21 11:40:24 EDT 2019 ppc64le ppc64le ppc64le GNU/Linux

% insmod samples/livepatch/livepatch-annotated-sample.ko
% grep -o 'livepatch.*' /proc/cmdline

% ./tools/testing/selftests/livepatch/
TEST: basic function patching ... ok
TEST: multiple livepatches ... ok
TEST: atomic replace livepatch ... ok
TEST: klp-convert symbols ... ok

There may be more ppc64le specifics lurking, but if we could get samples and self-tests working, I think we'd be in good shape for v3.

-- Joe