On Wed, 3 Apr 2019, Miroslav Benes wrote: > > > > Hmm, maybe I spoke too soon. > > > > > > I am having issues if I have a two-object livepatch module in which each > > > object file needs to specify its own KLP_MODULE_RELOC for the same > > > symbol name. > > > > > > For example: I have test_klp_convert.ko which is comprised of > > > test_klp_convert_a.o. which needs to refer to state_show,1 and > > > test_klp_convert_b.o. which needs to refer to state_show,2. > > > > > > % make > > > ... > > > KLP lib/livepatch/test_klp_convert.ko > > > klp-convert: Conflicting KLP_SYMPOS definition: vmlinux.state_show,0 vs. vmlinux.state_show,1. > > > klp-convert: Unable to load user-provided sympos > > > make[2]: *** [scripts/Makefile.modpost:148: lib/livepatch/test_klp_convert.ko] Error 255 > > > make[1]: *** [/home/cloud-user/disk/linux/Makefile:1282: modules] Error 2 > > > make: *** [Makefile:170: sub-make] Error 2 > > > > > > I take a closer look next week, but in the meantime, see the source > > > files below. > > > > Okay, with a fresh set of eyes today, I think the issue can be > > summarized as: > > > > * "Special" livepatch symbols, as processed by klp-convert, require > > external linkage, otherwise a new local storage instance will be > > created and miss klp-convert altogether > > > > * When linking together two object files, their combined symbol table > > will include a de-duped list of uniquly named global symbols > > > > So if we are to run klp-convert on the combined module object file (as > > per v2 plus suggested changes in this thread), we are going to run into > > problems if ... > > > > > > Example > > ======= > > > > (quoted in previous reply), test_klp_convert_a and test_klp_convert_b > > have their own "state_show" variable in which each wishes to resolve to > > unique symbol positions (2 and 3 accordingly). > > > > > > test_klp_convert_a > > ------------------ > > > > We get one symbol table entry and one relocation as expected. > > > > % eu-readelf --symbols lib/livepatch/test_klp_convert_a.o > > > > Symbol table [27] '.symtab' contains 38 entries: > > 29 local symbols String table: [28] '.strtab' > > Num: Value Size Type Bind Vis Ndx Name > > ... > > 30: 0000000000000000 0 NOTYPE GLOBAL DEFAULT UNDEF state_show > > ... > > > > % eu-readelf --relocs lib/livepatch/test_klp_convert_a.o > > > > Relocation section [12] '.rela.klp.module_relocs.vmlinux' for section [11] '.klp.module_relocs.vmlinux' at offset 0x4b8 contains 1 entry: > > Offset Type Value Addend Name > > 000000000000000000 X86_64_64 000000000000000000 +0 state_show > > > > > > test_klp_convert_b > > ------------------ > > > > Just like the other object file, one symbol table entry and one > > relocation: > > > > % eu-readelf --symbols lib/livepatch/test_klp_convert_a.o > > Symbol table [24] '.symtab' contains 23 entries: > > 19 local symbols String table: [25] '.strtab' > > Num: Value Size Type Bind Vis Ndx Name > > ... > > 20: 0000000000000000 0 NOTYPE GLOBAL DEFAULT UNDEF state_show > > ... > > > > % eu-readelf --relocs lib/livepatch/test_klp_convert_b.o > > > > Relocation section [ 9] '.rela.klp.module_relocs.vmlinux' for section [ 8] '.klp.module_relocs.vmlinux' at offset 0x118 contains 1 entry: > > Offset Type Value Addend Name > > 000000000000000000 X86_64_64 000000000000000000 +0 state_show > > > > > > test_klp_convert > > ---------------- > > > > But the combined test_klp_convert.klp.o file has only a single > > unresolved "state_show" symbol in its symbol table: > > > > % eu-readelf --symbols lib/livepatch/test_klp_convert.klp.o > > > > Symbol table [35] '.symtab' contains 57 entries: > > 47 local symbols String table: [36] '.strtab' > > Num: Value Size Type Bind Vis Ndx Name > > ... > > 52: 0000000000000000 0 NOTYPE GLOBAL DEFAULT UNDEF state_show > > ... > > > > though the .rela.klp.module_relocs.vmlinux relocation section has two > > entries: > > > > % eu-readelf --relocs lib/livepatch/test_klp_convert.klp.o > > > > Relocation section [17] '.rela.klp.module_relocs.vmlinux' for section [16] '.klp.module_relocs.vmlinux' at offset 0x446c8 contains 2 entries: > > Offset Type Value Addend Name > > 000000000000000000 X86_64_64 000000000000000000 +0 state_show > > 0x0000000000000010 X86_64_64 000000000000000000 +0 state_show > > > > and it looks like the combined KLP_MODULE_RELOC still contains the two > > unique symbol position values (2 and 3): > > > > % objcopy -O binary --only-section=.klp.module_relocs.vmlinux lib/livepatch/test_klp_convert.klp.o >(hexdump -C) > > 00000000 00 00 00 00 00 00 00 00 02 00 00 00 00 00 00 00 |................| > > 00000010 00 00 00 00 00 00 00 00 03 00 00 00 |............| > > 0000001c > > Nice :/ > > > Maybe we can work around this by modifying the annotation macros and/or > > klp-convert, or live with this for now. > > The question is (and I'll check later. I cannot wrap my head around it > now) if it at least works if there are two references of the same symbol > in two different .o. It would be same state_show in this case and not two > different ones. If it works then I think we can live with it for a while, > because after all duplicate symbols are quite rare in the kernel. I think it should work. There is only one case which causes problems. There exists an ambiguous local symbol in one parent object and a livepatch needs to reference two (or more) different versions of the symbol. In that case there have to be more sympos records provided by a user to help klp-convert distinguish between the cases. klp-convert is of course confused because it currently cannot cope with that. All other cases should be fine (theoretically). So we need to come up with a solution which would help klp-convert recognize the cases (symbol usage sites). Some sort of user annotation, but I cannot think of anything right now. Miroslav
