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

[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]

 



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.

[khlebnikov:
* 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
]

[mbenes:
* remove modinfo call. LIVEPATCH_ in Makefiled
]

[jmoreira:
* 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/module-common.lds -T
./arch/powerpc/kernel/module.lds --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 it's
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:
- https://github.com/SUSE/klp-convert/tree/fixppc64le

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.

Thanks,
João


-- Joe




[Index of Archives]     [Linux&nblp;USB Development]     [Linux Media]     [Video for Linux]     [Linux Audio Users]     [Yosemite Secrets]     [Linux Kernel]     [Linux SCSI]

  Powered by Linux