[lkml]   [2019]   [Apr]   [17]   [last100]   RSS Feed
Views: [wrap][no wrap]   [headers]  [forward] 
Messages in this thread
Patch in this message
SubjectRe: [PATCH v3 0/9] klp-convert livepatch build tooling
On Wed, Apr 10, 2019 at 11:50:49AM -0400, Joe Lawrence wrote:
> [ ... snip ... ]
> ----
> There are a few outstanding items that I came across while reviewing v2,
> but as changes started accumulating, it made sense to defer those to a
> later version. I'll reply to this thread individual topics to start
> discussion sub-threads for those.

Multiple object files

For v3, we tweaked the build scripts so that we could successfully build
a klp-converted livepatch module from multiple object files.

One interesting use-case is supporting two livepatch symbols of the same
name, but different object/position values.

An example target kernel module might be layed out like this:

test_klp_convert_mod.ko << target module is comprised of
two object files, each defining
test_klp_convert_mod_a.o their own local get_homonym_string()
get_homonym_string() function and homonym_string[]
homonym_string[] character arrays.


A look at interesting parts of the the module's symbol table:

% eu-readelf --symbols lib/livepatch/test_klp_convert_mod.ko | \
grep -e 'homonym' -e test_klp_convert_mod_

29: 0000000000000000 0 FILE LOCAL DEFAULT ABS test_klp_convert_mod_a.c
32: 0000000000000010 8 FUNC LOCAL DEFAULT 3 get_homonym_string
33: 0000000000000000 17 OBJECT LOCAL DEFAULT 5 homonym_string
37: 0000000000000000 0 FILE LOCAL DEFAULT ABS test_klp_convert_mod_b.c
38: 0000000000000020 8 FUNC LOCAL DEFAULT 3 get_homonym_string
39: 0000000000000000 17 OBJECT LOCAL DEFAULT 11 homonym_string

suggests that any livepatch module that wishes to resolve to
test_klp_convert_mod_a.c :: get_homonym_string() should include an
annotation like:


KLP_MODULE_RELOC(test_klp_convert_mod) relocs_a[] = {
KLP_SYMPOS(get_homonym_string, 1),

and to resolve test_klp_convert_mod_b.c :: get_homonym_string():


KLP_MODULE_RELOC(test_klp_convert_mod) relocs_b[] = {
KLP_SYMPOS(get_homonym_string, 2),

Unfortunately klp-convert v3 will fail to build such livepatch module,
regardless of sympos values:

klp-convert: Conflicting KLP_SYMPOS definition: vmlinux.state_show,2 vs. vmlinux.state_show,1.
klp-convert: Unable to load user-provided sympos
make[1]: *** [scripts/Makefile.modpost:148: lib/livepatch/test_klp_convert_multi_objs.ko] Error 255

This abort message can be traced to sympos_sanity_check() as it verifies
that there no duplicate symbol names found in its list of user specified
symbols (ie, those needing to be converted).

Common sympos

New test case and related fixup can be found here:

To better debug this issue, I added another selftest that demonstrates
this configuration in isolation. "show_state" is a popular kernel
function name (my VM reported 5 instances in kallsyms) so I chose that
symbol name.

Initially I specified the *same* symbol position (1) in both .c file
KLP_SYMPOS annotations. At the very least, we can trivially patch
klp-convert v3 to support annotations for the the same symbol <object,
name, position> value across object files.

Result: a small tweak to sympos_sanity_check() to relax its symbol
uniqueness verification: allow for duplicate <object, name, position>
instances. Now it will only complain when a supplied symbol references
the same <object, name> but a different <position>.

diff --git a/scripts/livepatch/klp-convert.c b/scripts/livepatch/klp-convert.c
index 82c27d219372..713835dfc9ec 100644
--- a/scripts/livepatch/klp-convert.c
+++ b/scripts/livepatch/klp-convert.c
@@ -194,7 +194,10 @@ static void clear_sympos_annontations(struct elf *klp_elf)

-/* Checks if two or more elements in usr_symbols have the same name */
+ * Checks if two or more elements in usr_symbols have the same
+ * object and name, but different symbol position
+ */
static bool sympos_sanity_check(void)
bool sane = true;
@@ -203,7 +206,9 @@ static bool sympos_sanity_check(void)
list_for_each_entry(sp, &usr_symbols, list) {
aux = list_next_entry(sp, list);
list_for_each_entry_from(aux, &usr_symbols, list) {
- if (strcmp(sp->symbol_name, aux->symbol_name) == 0) {
+ if (sp->pos != aux->pos &&
+ strcmp(sp->object_name, aux->object_name) == 0 &&
+ strcmp(sp->symbol_name, aux->symbol_name) == 0)
WARN("Conflicting KLP_SYMPOS definition: %s.%s,%d vs. %s.%s,%d.",
sp->object_name, sp->symbol_name, sp->pos,
aux->object_name, aux->symbol_name, aux->pos);

Unique sympos
But even with the above workaround, specifying unique symbol positions
will (and should) result in a klp-convert complaint.

When modifying the test module with unique symbol position annotation
values (1 and 2 respectively):


extern void *state_show;
KLP_MODULE_RELOC(vmlinux) vmlinux_relocs_b[] = {
KLP_SYMPOS(state_show, 1)

extern void *state_show;
KLP_MODULE_RELOC(vmlinux) vmlinux_relocs_b[] = {
KLP_SYMPOS(state_show, 2)

Each object file's symbol table contains a "state_show" instance:

% eu-readelf --symbols lib/livepatch/test_klp_convert_multi_objs_a.o | grep '\<state_show\>'
30: 0000000000000000 0 NOTYPE GLOBAL DEFAULT UNDEF state_show
% eu-readelf --symbols lib/livepatch/test_klp_convert_multi_objs_b.o | grep '\<state_show\>'
20: 0000000000000000 0 NOTYPE GLOBAL DEFAULT UNDEF state_show
and the intermediate test_klp_convert_multi_objs.klp.o file contains a
combined .klp.module_relocs.vmlinux relocation section with two
KLP_SYMPOS structures, each with a unique <sympos> value:

% objcopy -O binary --only-section=.klp.module_relocs.vmlinux \
lib/livepatch/test_klp_convert_multi_objs.klp.o >(hexdump)

0000000 0000 0000 0000 0000 0001 0000 0000 0000
0000010 0000 0000 0002 0000
but the symbol tables were merged, sorted and non-unique symbols
removed, leaving a *single* unresolved "state_show" instance:

% eu-readelf --symbols lib/livepatch/test_klp_convert_multi_objs.klp.o | grep '\<state_show\>'
53: 0000000000000000 0 NOTYPE GLOBAL DEFAULT UNDEF state_show
which means that even though we have separate relocations for each
"state_show" instance:

Relocation section [ 6] '.rela.text.unlikely' for section [ 5] '.text.unlikely' at offset 0x44510 contains 11 entries:
Offset Type Value Addend Name
0x0000000000000003 X86_64_32S 000000000000000000 +0 state_show
0x0000000000000034 X86_64_32S 000000000000000000 +0 state_show
they share the same rela->sym and there is no way to distinguish which
one correlates to which KLP_SYMPOS structure.

Future Work
I don't see an easy way to support multiple homonym <object, name>
symbols with unique <position> values in the current livepatch module
Elf format. The only solutions that come to mind right now include
renaming homonym symbols somehow to retain the relocation->symbol
relationship when separate object files are combined. Perhaps an
intermediate linker step could make annotated symbols unique in some way
to achieve this. /thinking out loud

In the meantime, the unique symbol <position> case is already detected
and with a little tweaking we could support multiple common symbol
<position> values.

-- Joe

 \ /
  Last update: 2019-04-17 22:14    [W:0.253 / U:29.892 seconds]
©2003-2020 Jasper Spaans|hosted at Digital Ocean and TransIP|Read the blog|Advertise on this site