| lists.openwall.net | lists / announce owl-users owl-dev john-users john-dev passwdqc-users yescrypt popa3d-users / oss-security kernel-hardening musl sabotage tlsify passwords / crypt-dev xvendor / Bugtraq Full-Disclosure linux-kernel linux-netdev linux-ext4 linux-hardening linux-cve-announce PHC | |
|
Open Source and information security mailing list archives
| ||
|
Date: Thu, 4 Apr 2019 12:59:30 +0200 (CEST)
From: Miroslav Benes <mbenes@...e.cz>
To: Joe Lawrence <joe.lawrence@...hat.com>
cc: Joao Moreira <jmoreira@...e.de>, live-patching@...r.kernel.org,
pmladek@...e.cz, jikos@...e.cz, nstange@...e.de,
jpoimboe@...hat.com, khlebnikov@...dex-team.ru, jeyu@...nel.org,
matz@...e.de, linux-kernel@...r.kernel.org,
yamada.masahiro@...ionext.com, linux-kbuild@...r.kernel.org,
michal.lkml@...kovi.net
Subject: Re: [PATCH v2 2/8] kbuild: Support for Symbols.list creation
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
Powered by blists - more mailing lists