From: Joe Lawrence <joe.lawrence@redhat.com>
To: linux-kernel@vger.kernel.org, live-patching@vger.kernel.org,
linux-kbuild@vger.kernel.org
Cc: Jessica Yu <jeyu@kernel.org>, Jiri Kosina <jikos@kernel.org>,
Joao Moreira <jmoreira@suse.de>,
Josh Poimboeuf <jpoimboe@redhat.com>,
Konstantin Khlebnikov <khlebnikov@yandex-team.ru>,
Masahiro Yamada <yamada.masahiro@socionext.com>,
Michael Matz <matz@suse.de>, Miroslav Benes <mbenes@suse.cz>,
Nicolai Stange <nstange@suse.de>, Petr Mladek <pmladek@suse.com>,
Jason Baron <jbaron@akamai.com>,
Evgenii Shatokhin <eshatokhin@virtuozzo.com>
Subject: Re: [PATCH v3 0/9] klp-convert livepatch build tooling
Date: Fri, 12 Apr 2019 17:26:54 -0400 [thread overview]
Message-ID: <20190412212654.GA21627@redhat.com> (raw)
In-Reply-To: <20190410155058.9437-1-joe.lawrence@redhat.com>
On Wed, Apr 10, 2019 at 11:50:49AM -0400, Joe Lawrence wrote:
> Hi folks,
>
> This is the third installment of the klp-convert tool for generating and
> processing livepatch symbols for livepatch module builds. For those
> following along at home, archive links to previous versions:
>
> RFC:
> https://lore.kernel.org/lkml/cover.1477578530.git.jpoimboe@redhat.com/
> v2:
> https://lore.kernel.org/lkml/f52d29f7-7d1b-ad3d-050b-a9fa8878faf2@redhat.com/
>
> (Note that I don't see v2 archived at lore, but that is a link to the
> most recent subthread that lore did catch.)
>
>
> Livepatches may use symbols which are not contained in its own scope,
> and, because of that, may end up compiled with relocations that will
> only be resolved during module load. Yet, when the referenced symbols are
> not exported, solving this relocation requires information on the object
> that holds the symbol (either vmlinux or modules) and its position inside
> the object, as an object may contain multiple symbols with the same name.
> Providing such information must be done accordingly to what is specified
> in Documentation/livepatch/module-elf-format.txt.
>
> Currently, there is no trivial way to embed the required information as
> requested in the final livepatch elf object. klp-convert solves this
> problem in two different forms: (i) by relying on a symbol map, which is
> built during kernel compilation, to automatically infer the relocation
> targeted symbol, and, when such inference is not possible (ii) by using
> annotations in the elf object to convert the relocation accordingly to
> the specification, enabling it to be handled by the livepatch loader.
>
> Given the above, add support for symbol mapping in the form of
> Symbols.list file; add klp-convert tool; integrate klp-convert tool into
> kbuild; make livepatch modules discernible during kernel compilation
> pipeline; add data-structure and macros to enable users to annotate
> livepatch source code; make modpost stage compatible with livepatches;
> update livepatch-sample and update documentation.
>
> The patch was tested under three use-cases:
>
> use-case 1: There is a relocation in the lp that can be automatically
> resolved by klp-convert. For example. see the saved_command_line
> variable in lib/livepatch/test_klp_convert2.c.
>
> use-case 2: There is a relocation in the lp that cannot be automatically
> resolved, as the name of the respective symbol appears in multiple
> objects. The livepatch contains an annotation to enable a correct
> relocation. See the KLP_MODULE_RELOC / KLP_SYMPOS annotation sections
> in lib/livepatch/test_klp_convert{1,2}.c.
>
> use-case 3: There is a relocation in the lp that cannot be automatically
> resolved similarly as 2, but no annotation was provided in the
> livepatch, triggering an error during compilation. Reproducible by
> removing the KLP_MODULE_RELOC / KLP_SYMPOS annotation sections in
> lib/livepatch/test_klp_convert{1,2}.c.
>
>
> Branches
> --------
>
> Since I spent some time tinkering with v2 and accumulated a bunch of
> fixes, I collected them up and am posting this new version. For review
> purposes, I posted two branches up to github:
>
> 1 - an expanded branch that with changes separate from the original
> https://github.com/joe-lawrence/linux/commits/klp-convert-v3-expanded
>
> 2 - a squashed branch of (1) that comprises v3:
> https://github.com/joe-lawrence/linux/commits/klp-convert-v3
>
> Non-trivial commits in the expanded branch have some extra commentary
> and details for debugging in the commit message that were dropped when
> squashing into their respective parent commits.
>
>
> TODO
> ----
>
> 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.
>
>
> Changelogs
> ----------
>
> The commit changelogs were getting long, so I've moved them here for
> posterity and review purposes:
>
> livepatch: Create and include UAPI headers
> v2:
> - jmoreira: split up and changelog
> v3:
> - joe: convert to SPDX license tags
>
> kbuild: Support for Symbols.list creation
> v3:
> - jmoreira: adjust for multiobject livepatch
> - joe: add klpclean to PHONY
> - joe: align KLP prefix
>
> livepatch: Add klp-convert tool
> v2:
> - khlebnikov: use HOSTLOADLIBES_ instead of HOSTLDFLAGS: -lelf must be
> at the end
> - jmoreira: add support to automatic relocation conversion in
> klp-convert.c, changelog
> v3:
> - joe: convert to SPDX license tags
> - jmoreira: add rela symbol name to WARNs
> - jmoreira: ignore relocations to .TOC and symbols with index 0
> - joe: separate and fix valid_sympos() sympos=0 and sympos=1..n checks
> - joe: fix symbol use-after-frees
> - joe: fix remaining valgrind leak complaints (non-error paths only)
> - joe: checkpatch nits
>
> livepatch: Add klp-convert annotation helpers
> v2:
> - jmoreira: split up: move KLP_MODULE_RELOC from previous patch to
> here, add KLP_SYMPOS, move macros from include/uapi/livepatch.h to
> include/linux/livepatch.h
> v3:
> - joe: suggested by jpoimboe, KLP_MODULE_RELOC macro should 4-byte
> align klp_module_reloc structures
>
> modpost: Integrate klp-convert
> v2:
> - khlebnikov: save cmd_ld_ko_o into .module.cmd, if_changed_rule
> doesn't do that.f
> - khlebnikov: fix bashisms for debian where /bin/sh is a symlink to
> /bin/dash
> - khlebnikov: 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.
> - khlebnikov: check modinfo -F livepatch only if CONFIG_LIVEPATCH is
> true.
> - mbenes: remove modinfo call. LIVEPATCH_ in Makefile
> - 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.
> v3:
> - joe: adjust rule_ld_ko_o to call echo-cmd
> - joe: rebase for v5.1
> - joe: align KLP prefix
>
> modpost: Add modinfo flag to livepatch modules
> v2:
> - jmoreira: fix modpost.c (add_livepatch_flag) to update module
> structure with livepatch flag and prevent modpost from breaking due to
> unresolved symbols
> v3:
> - joe: adjust modpost.c::get_modinfo() call for v5.0 version
>
> livepatch: Add sample livepatch module
> v3:
> - joe: update all in-tree livepatches with LIVEPATCH_* modinfo flag
>
> documentation: Update on livepatch elf format
> v2:
> - jmoreira: update module to use KLP_SYMPOS
> - jmoreira: Comments on symbol resolution scheme
> - jmoreira: Update Makefile
> - jmoreira: Remove MODULE_INFO statement
> - jmoreira: Changelog
> v3:
> - joe: rebase for v5.1
> - joe: code shuffle to better match original source file
>
> livepatch/selftests: add klp-convert
> v3:
> - joe: clarify sympos=0 and sympos=1..n
>
>
> And now the usual git cover-letter boilerplate...
>
> Joao Moreira (2):
> kbuild: Support for Symbols.list creation
> documentation: Update on livepatch elf format
>
> Joe Lawrence (1):
> livepatch/selftests: add klp-convert
>
> Josh Poimboeuf (5):
> livepatch: Create and include UAPI headers
> livepatch: Add klp-convert tool
> livepatch: Add klp-convert annotation helpers
> modpost: Integrate klp-convert
> livepatch: Add sample livepatch module
>
> Miroslav Benes (1):
> modpost: Add modinfo flag to livepatch modules
>
> .gitignore | 1 +
> Documentation/livepatch/livepatch.txt | 3 +
> Documentation/livepatch/module-elf-format.txt | 50 +-
> MAINTAINERS | 2 +
> Makefile | 30 +-
> include/linux/livepatch.h | 13 +
> include/uapi/linux/livepatch.h | 22 +
> kernel/livepatch/core.c | 4 +-
> lib/livepatch/Makefile | 15 +
> lib/livepatch/test_klp_atomic_replace.c | 1 -
> lib/livepatch/test_klp_callbacks_demo.c | 1 -
> lib/livepatch/test_klp_callbacks_demo2.c | 1 -
> lib/livepatch/test_klp_convert1.c | 106 +++
> lib/livepatch/test_klp_convert2.c | 103 +++
> lib/livepatch/test_klp_convert_mod_a.c | 25 +
> lib/livepatch/test_klp_convert_mod_b.c | 13 +
> lib/livepatch/test_klp_livepatch.c | 1 -
> samples/livepatch/Makefile | 7 +
> .../livepatch/livepatch-annotated-sample.c | 102 +++
> samples/livepatch/livepatch-sample.c | 1 -
> scripts/Kbuild.include | 4 +-
> scripts/Makefile | 1 +
> scripts/Makefile.build | 7 +
> scripts/Makefile.modpost | 24 +-
> scripts/livepatch/.gitignore | 1 +
> scripts/livepatch/Makefile | 7 +
> scripts/livepatch/elf.c | 753 ++++++++++++++++++
> scripts/livepatch/elf.h | 72 ++
> scripts/livepatch/klp-convert.c | 692 ++++++++++++++++
> scripts/livepatch/klp-convert.h | 39 +
> scripts/livepatch/list.h | 391 +++++++++
> scripts/mod/modpost.c | 82 +-
> scripts/mod/modpost.h | 1 +
> .../selftests/livepatch/test-livepatch.sh | 64 ++
> 34 files changed, 2616 insertions(+), 23 deletions(-)
> create mode 100644 include/uapi/linux/livepatch.h
> create mode 100644 lib/livepatch/test_klp_convert1.c
> create mode 100644 lib/livepatch/test_klp_convert2.c
> create mode 100644 lib/livepatch/test_klp_convert_mod_a.c
> create mode 100644 lib/livepatch/test_klp_convert_mod_b.c
> create mode 100644 samples/livepatch/livepatch-annotated-sample.c
> create mode 100644 scripts/livepatch/.gitignore
> create mode 100644 scripts/livepatch/Makefile
> create mode 100644 scripts/livepatch/elf.c
> create mode 100644 scripts/livepatch/elf.h
> create mode 100644 scripts/livepatch/klp-convert.c
> create mode 100644 scripts/livepatch/klp-convert.h
> create mode 100644 scripts/livepatch/list.h
>
> --
> 2.20.1
[ cc += Jason and Evgenii ]
Apologies for the long brain dump, but I promised to reply to this
thread with some of the larger TODO issues I came across with this
patchset. Static key support is probably the most complicated item, so
there is a lot of debugging output I can provide.
See the tl;dr and Future Work sections for the executive summary.
Static key support
==================
tl;dr
-----
Livepatch symbols are created when building livepatch modules that
reference the (non-exported) static keys of a target object.
When loading a livepatch that contains klp-converted static key symbols,
the module loader crashes.
Testing setup
-------------
The easiest way to see the source and repro is to grab this branch:
https://github.com/joe-lawrence/linux/commits/klp-convert-v4-static-keys
and note these last two commits:
- livepatch/klp-convert: abort on static key conversion: this will
prevent klp-convert from converting any relocations to the
__jump_table. Revert this commit to see the crash. If we don't
have a fix before merging, I would suggest a temporary abort
like this to avoid silently creating dangerous livepatches.
- livepatch/selftests: add livepatch static keys: this adds the
self-test which will repro the crash.
I added a new livepatch selftest to verify klp-convert and static key
behavior: it consists of two kernel modules: test_klp_static_keys_mod.ko
and livepatch module that patches it, test_klp_static_keys.ko.
The base module contains a few different types of static keys: default
true / false, exported / not-exported, and one created by the trace
event macro infrastructure.
The livepatch module references each of these, relying upon klp-convert.
This module also provides its own static key for reference.
test_klp_static_keys_mod.ko
---------------------------
module_key (false) - exported
module_key2 (true) <----
TRACE_EVENT(test_klp_static_keys) <-- |
| |
| | klp-convert resolves
test_klp_static_keys.ko - livepatch | | these references
----------------------------------- | |
klp_key (true) | |
test_klp_static_keys ----------------- |
module_key2 ----------------------------
module_key
Currently .klp symbols are created as well as a relocation section for those
symbols in the their corresponding .text locations.
test_klp_static_keys_mod.ko
---------------------------
% eu-readelf --symbols lib/livepatch/test_klp_static_keys_mod.ko | \
grep -e '__tracepoint_test_klp_static_keys' -e '\<module_key[2]*\>'
49: 0000000000000010 16 OBJECT LOCAL DEFAULT 44 module_key
65: 0000000000000010 16 OBJECT LOCAL DEFAULT 32 module_key2
96: 0000000000000000 48 OBJECT GLOBAL DEFAULT 40 __tracepoint_test_klp_static_keys
test_klp_static_keys.klp.o
--------------------------
% eu-readelf --symbols lib/livepatch/test_klp_static_keys.klp.o | \
grep -e '__tracepoint_test_klp_static_keys' -e '\<module_key[2]*\>'
71: 0000000000000000 0 NOTYPE GLOBAL DEFAULT UNDEF __tracepoint_test_klp_static_keys
78: 0000000000000000 0 NOTYPE GLOBAL DEFAULT UNDEF module_key2
84: 0000000000000000 0 NOTYPE GLOBAL DEFAULT UNDEF module_key
Lots of __tracepoint_test_klp_static_keys relocations since each
module function registers an event when it is called:
% eu-readelf --reloc lib/livepatch/test_klp_static_keys.klp.o
Relocation section [ 4] '.rela.text' for section [ 3] '.text' at offset 0x2e7d8 contains 88 entries:
Offset Type Value Addend Name
...
0x000000000000002c X86_64_32S 000000000000000000 +0 module_key2
...
0x000000000000007c X86_64_PC32 000000000000000000 +36 __tracepoint_test_klp_static_keys
...
0x00000000000000d9 X86_64_PC32 000000000000000000 +36 __tracepoint_test_klp_static_keys
...
0x0000000000000169 X86_64_PC32 000000000000000000 +36 __tracepoint_test_klp_static_keys
...
0x00000000000001f9 X86_64_PC32 000000000000000000 +36 __tracepoint_test_klp_static_keys
...
0x000000000000028c X86_64_32S 000000000000000000 +0 module_key
...
0x00000000000002dc X86_64_PC32 000000000000000000 +36 __tracepoint_test_klp_static_keys
...
0x000000000000038c X86_64_PC32 000000000000000000 +36 __tracepoint_test_klp_static_keys
...
0x00000000000003eb X86_64_PC32 000000000000000000 +36 __tracepoint_test_klp_static_keys
Relocations generated for the __jump_table itself, note that I grouped
each jump entry <code, target, key> relocation set:
Relocation section [19] '.rela__jump_table' for section [18] '__jump_table' at offset 0x2fb28 contains 30 entries:
Offset Type Value Addend Name
000000000000000000 X86_64_PC32 000000000000000000 +12 .text
0x0000000000000004 X86_64_PC32 000000000000000000 +102 .text
0x0000000000000008 X86_64_PC64 000000000000000000 +8 __tracepoint_test_klp_static_keys
0x0000000000000010 X86_64_PC32 000000000000000000 +188 .text
0x0000000000000014 X86_64_PC32 000000000000000000 +195 .text
0x0000000000000018 X86_64_PC64 000000000000000000 +8 __tracepoint_test_klp_static_keys
0x0000000000000020 X86_64_PC32 000000000000000000 +304 .text
0x0000000000000024 X86_64_PC32 000000000000000000 +0 .text.unlikely
0x0000000000000028 X86_64_PC64 000000000000000000 +16 .bss
0x0000000000000030 X86_64_PC32 000000000000000000 +332 .text
0x0000000000000034 X86_64_PC32 000000000000000000 +339 .text
0x0000000000000038 X86_64_PC64 000000000000000000 +8 __tracepoint_test_klp_static_keys
0x0000000000000040 X86_64_PC32 000000000000000000 +448 .text
0x0000000000000044 X86_64_PC32 000000000000000000 +52 .text.unlikely
0x0000000000000048 X86_64_PC64 000000000000000000 +0 module_key
0x0000000000000050 X86_64_PC32 000000000000000000 +476 .text
0x0000000000000054 X86_64_PC32 000000000000000000 +483 .text
0x0000000000000058 X86_64_PC64 000000000000000000 +8 __tracepoint_test_klp_static_keys
0x0000000000000060 X86_64_PC32 000000000000000000 +592 .text
0x0000000000000064 X86_64_PC32 000000000000000000 +104 .text.unlikely
0x0000000000000068 X86_64_PC64 000000000000000000 +1 module_key2
0x0000000000000070 X86_64_PC32 000000000000000000 +620 .text
0x0000000000000074 X86_64_PC32 000000000000000000 +710 .text
0x0000000000000078 X86_64_PC64 000000000000000000 +8 __tracepoint_test_klp_static_keys
0x0000000000000080 X86_64_PC32 000000000000000000 +796 .text
0x0000000000000084 X86_64_PC32 000000000000000000 +886 .text
0x0000000000000088 X86_64_PC64 000000000000000000 +8 __tracepoint_test_klp_static_keys
0x0000000000000090 X86_64_PC32 000000000000000000 +962 .text
0x0000000000000094 X86_64_PC32 000000000000000000 +981 .text
0x0000000000000098 X86_64_PC64 000000000000000000 +8 __tracepoint_test_klp_static_keys
test_klp_static_keys.ko
-----------------------
Finally klp-convert has transformed a bunch of symbols for us:
% eu-readelf --symbols lib/livepatch/test_klp_static_keys.ko | \
grep -e '__tracepoint_test_klp_static_keys' -e '\<module_key[2]*\>'
71: 0000000000000000 0 NOTYPE GLOBAL DEFAULT LOOS+0 .klp.sym.test_klp_static_keys_mod.__tracepoint_test_klp_static_keys,0
78: 0000000000000000 0 NOTYPE GLOBAL DEFAULT LOOS+0 .klp.sym.test_klp_static_keys_mod.module_key2,0
84: 0000000000000000 0 NOTYPE GLOBAL DEFAULT UNDEF module_key
% eu-readelf --reloc lib/livepatch/test_klp_static_keys.ko
Relocation section [19] '.rela__jump_table' for section [18] '__jump_table' at offset 0x2480 contains 30 entries:
Offset Type Value Addend Name
000000000000000000 X86_64_PC32 000000000000000000 +12 .text
0x0000000000000004 X86_64_PC32 000000000000000000 +102 .text
0x0000000000000008 X86_64_PC64 000000000000000000 +8 .klp.sym.test_klp_static_keys_mod.__tracepoint_test_klp_static_keys,0
0x0000000000000010 X86_64_PC32 000000000000000000 +188 .text
0x0000000000000014 X86_64_PC32 000000000000000000 +195 .text
0x0000000000000018 X86_64_PC64 000000000000000000 +8 .klp.sym.test_klp_static_keys_mod.__tracepoint_test_klp_static_keys,0
0x0000000000000020 X86_64_PC32 000000000000000000 +304 .text
0x0000000000000024 X86_64_PC32 000000000000000000 +0 .text.unlikely
0x0000000000000028 X86_64_PC64 000000000000000000 +16 .bss
0x0000000000000030 X86_64_PC32 000000000000000000 +332 .text
0x0000000000000034 X86_64_PC32 000000000000000000 +339 .text
0x0000000000000038 X86_64_PC64 000000000000000000 +8 .klp.sym.test_klp_static_keys_mod.__tracepoint_test_klp_static_keys,0
0x0000000000000040 X86_64_PC32 000000000000000000 +448 .text
0x0000000000000044 X86_64_PC32 000000000000000000 +52 .text.unlikely
0x0000000000000048 X86_64_PC64 000000000000000000 +0 module_key
0x0000000000000050 X86_64_PC32 000000000000000000 +476 .text
0x0000000000000054 X86_64_PC32 000000000000000000 +483 .text
0x0000000000000058 X86_64_PC64 000000000000000000 +8 .klp.sym.test_klp_static_keys_mod.__tracepoint_test_klp_static_keys,0
0x0000000000000060 X86_64_PC32 000000000000000000 +592 .text
0x0000000000000064 X86_64_PC32 000000000000000000 +104 .text.unlikely
0x0000000000000068 X86_64_PC64 000000000000000000 +1 .klp.sym.test_klp_static_keys_mod.module_key2,0
0x0000000000000070 X86_64_PC32 000000000000000000 +620 .text
0x0000000000000074 X86_64_PC32 000000000000000000 +710 .text
0x0000000000000078 X86_64_PC64 000000000000000000 +8 .klp.sym.test_klp_static_keys_mod.__tracepoint_test_klp_static_keys,0
0x0000000000000080 X86_64_PC32 000000000000000000 +796 .text
0x0000000000000084 X86_64_PC32 000000000000000000 +886 .text
0x0000000000000088 X86_64_PC64 000000000000000000 +8 .klp.sym.test_klp_static_keys_mod.__tracepoint_test_klp_static_keys,0
0x0000000000000090 X86_64_PC32 000000000000000000 +962 .text
0x0000000000000094 X86_64_PC32 000000000000000000 +981 .text
0x0000000000000098 X86_64_PC64 000000000000000000 +8 .klp.sym.test_klp_static_keys_mod.__tracepoint_test_klp_static_keys,0
And here's our final set of klp-converted relocations which include the
unexported trace point symbol and module_key2 symbols:
Relocation section [44] '.klp.rela.test_klp_static_keys_mod..text' for section [ 3] '.text' at offset 0x55c18 contains 12 entries:
Offset Type Value Addend Name
...
0x00000000000003eb X86_64_PC32 000000000000000000 +36 .klp.sym.test_klp_static_keys_mod.__tracepoint_test_klp_static_keys,0
0x000000000000038c X86_64_PC32 000000000000000000 +36 .klp.sym.test_klp_static_keys_mod.__tracepoint_test_klp_static_keys,0
0x00000000000002dc X86_64_PC32 000000000000000000 +36 .klp.sym.test_klp_static_keys_mod.__tracepoint_test_klp_static_keys,0
0x00000000000001f9 X86_64_PC32 000000000000000000 +36 .klp.sym.test_klp_static_keys_mod.__tracepoint_test_klp_static_keys,0
0x0000000000000169 X86_64_PC32 000000000000000000 +36 .klp.sym.test_klp_static_keys_mod.__tracepoint_test_klp_static_keys,0
0x00000000000000d9 X86_64_PC32 000000000000000000 +36 .klp.sym.test_klp_static_keys_mod.__tracepoint_test_klp_static_keys,0
0x000000000000007c X86_64_PC32 000000000000000000 +36 .klp.sym.test_klp_static_keys_mod.__tracepoint_test_klp_static_keys,0
0x000000000000002c X86_64_32S 000000000000000000 +0 .klp.sym.test_klp_static_keys_mod.module_key2,0
...
Current behavior
----------------
Not good. The livepatch successfully builds but crashes on load:
% insmod lib/livepatch/test_klp_static_keys_mod.ko
% insmod lib/livepatch/test_klp_static_keys.ko
BUG: unable to handle kernel NULL pointer dereference at 0000000000000010
#PF error: [normal kernel read fault]
PGD 0 P4D 0
Oops: 0000 [#1] SMP PTI
CPU: 3 PID: 9367 Comm: insmod Tainted: G E K 5.1.0-rc4+ #4
Hardware name: QEMU Standard PC (i440FX + PIIX, 1996), BIOS ?-20180724_192412-buildhw-07.phx2.fedoraproject.org-1.fc29 04/01/2014
RIP: 0010:jump_label_apply_nops+0x3b/0x60
Code: 02 00 00 48 c1 e5 04 48 01 dd 48 39 eb 74 3a 72 0b eb 36 48 83 c3 10 48 39 dd 76 2d 48 8b 43 08 48 89 c2 83 e0 01 48 83 e2 fc <48> 8b 54 13 10 83 e2 01 38 c2 75 dd 48 89 df 31 f6 48 83 c3 10 e8
RSP: 0018:ffffa8874068fcf8 EFLAGS: 00010206
RAX: 0000000000000000 RBX: ffffffffc07fd000 RCX: 000000000000000d
RDX: 000000003f803000 RSI: ffffffffa5077be0 RDI: ffffffffc07fe540
RBP: ffffffffc07fd0a0 R08: ffffa88740f43878 R09: ffffa88740eed000
R10: 0000000000055a4b R11: ffffa88740f43878 R12: ffffa88740f430b8
R13: 0000000000000000 R14: ffffa88740f42df8 R15: 0000000000042b01
FS: 00007f4f1dafb740(0000) GS:ffff9a81fbb80000(0000) knlGS:0000000000000000
CS: 0010 DS: 0000 ES: 0000 CR0: 0000000080050033
CR2: 0000000000000010 CR3: 00000000b5d8a000 CR4: 00000000000006e0
Call Trace:
module_finalize+0x184/0x1c0
load_module+0x1400/0x1910
? kernel_read_file+0x18d/0x1c0
? __do_sys_finit_module+0xa8/0x110
__do_sys_finit_module+0xa8/0x110
do_syscall_64+0x55/0x1a0
entry_SYSCALL_64_after_hwframe+0x44/0xa9
RIP: 0033:0x7f4f1cae82bd
Future work
-----------
At the very least, I think this call-chain ordering is wrong for
livepatch static key symbols:
load_module
apply_relocations
post_relocation
module_finalize
jump_label_apply_nops <<
...
prepare_coming_module
blocking_notifier_call_chain(&module_notify_list, MODULE_STATE_COMING, mod);
jump_label_module_notify
case MODULE_STATE_COMING
jump_label_add_module <<
do_init_module
do_one_initcall(mod->init)
__init patch_init [kpatch-patch]
klp_register_patch
klp_init_patch
klp_for_each_object(patch, obj)
klp_init_object
klp_init_object_loaded
klp_write_object_relocations <<
blocking_notifier_call_chain(&module_notify_list, MODULE_STATE_LIVE, mod);
jump_label_module_notify
case MODULE_STATE_LIVE
jump_label_invalidate_module_init
where klp_write_object_relocations() is called way *after*
jump_label_apply_nops() and jump_label_add_module().
Detection
---------
I have been tinkering with some prototype code to defer
jump_label_apply_nops() and jump_label_add_module(), but it has been
slow going. I think the jist of it is that we're going to need to call
these dynamically when individual klp_objects are patched, not when the
livepatch module itself loads. If anyone with static key expertise
wants to jump in here, let me know.
In the meantime, I cooked up a potential followup commit to detect
conversion of static key symbols and klp-convert failure. It basically
runs through the output .ko's ELF symbols and verifies that none of the
converted ones can be found as a .rela__jump_table relocated symbol. It
accurately catches the problematic references in test_klp_static_keys.ko
thus far.
This was based on a similar issue reported as a bug against
kpatch-build, in which Josh wrote code to detect this scenario:
https://github.com/dynup/kpatch/issues/946
https://github.com/jpoimboe/kpatch/commit/2cd2d27607566aee9590c367e615207ce1ce24c6
I can post ("livepatch/klp-convert: abort on static key conversion")
here as a follow commit if it looks reasonable and folks wish to review
it... or we can try and tackle static keys before merging klp-convert.
-- Joe
next prev parent reply other threads:[~2019-04-12 21:27 UTC|newest]
Thread overview: 42+ messages / expand[flat|nested] mbox.gz Atom feed top
2019-04-10 15:50 [PATCH v3 0/9] klp-convert livepatch build tooling Joe Lawrence
2019-04-10 15:50 ` [PATCH v3 1/9] livepatch: Create and include UAPI headers Joe Lawrence
2019-04-11 0:32 ` Masahiro Yamada
2019-04-11 14:30 ` Joe Lawrence
2019-04-11 15:48 ` Josh Poimboeuf
2019-04-11 18:41 ` Miroslav Benes
2019-04-10 15:50 ` [PATCH v3 2/9] kbuild: Support for Symbols.list creation Joe Lawrence
2019-04-11 9:18 ` Artem Savkov
2019-04-11 15:18 ` Joe Lawrence
2019-04-11 19:04 ` Miroslav Benes
2019-04-16 14:13 ` Joe Lawrence
2019-04-16 19:02 ` Miroslav Benes
2019-04-10 15:50 ` [PATCH v3 3/9] livepatch: Add klp-convert tool Joe Lawrence
2019-04-10 18:22 ` Joe Lawrence
2019-04-12 9:02 ` Miroslav Benes
2019-04-23 20:35 ` Joe Lawrence
2019-04-24 17:47 ` Miroslav Benes
2019-04-24 21:00 ` Joe Lawrence
2019-04-10 15:50 ` [PATCH v3 4/9] livepatch: Add klp-convert annotation helpers Joe Lawrence
2019-04-10 18:18 ` Joe Lawrence
2019-04-12 9:14 ` Miroslav Benes
2019-04-10 15:50 ` [PATCH v3 5/9] modpost: Integrate klp-convert Joe Lawrence
2019-04-11 15:54 ` Joe Lawrence
2019-04-10 15:50 ` [PATCH v3 6/9] modpost: Add modinfo flag to livepatch modules Joe Lawrence
2019-04-12 10:43 ` Miroslav Benes
2019-04-10 15:50 ` [PATCH v3 7/9] livepatch: Add sample livepatch module Joe Lawrence
2019-04-10 15:50 ` [PATCH v3 8/9] documentation: Update on livepatch elf format Joe Lawrence
2019-04-10 15:50 ` [PATCH v3 9/9] livepatch/selftests: add klp-convert Joe Lawrence
2019-04-12 21:26 ` Joe Lawrence [this message]
2019-04-16 11:37 ` [PATCH v3 0/9] klp-convert livepatch build tooling Miroslav Benes
2019-05-03 14:29 ` Joe Lawrence
2019-05-06 14:39 ` Miroslav Benes
2019-04-16 5:24 ` Balbir Singh
2019-04-16 8:29 ` Miroslav Benes
2019-04-16 13:37 ` Joe Lawrence
2019-04-16 13:55 ` Joe Lawrence
2019-04-16 19:09 ` Miroslav Benes
2019-04-17 20:13 ` Joe Lawrence
2019-04-24 18:19 ` Miroslav Benes
2019-04-24 19:13 ` Joao Moreira
2019-04-24 19:23 ` Josh Poimboeuf
2019-04-24 19:31 ` Joe Lawrence
Reply instructions:
You may reply publicly to this message via plain-text email
using any one of the following methods:
* Save the following mbox file, import it into your mail client,
and reply-to-all from there: mbox
Avoid top-posting and favor interleaved quoting:
https://en.wikipedia.org/wiki/Posting_style#Interleaved_style
* Reply using the --to, --cc, and --in-reply-to
switches of git-send-email(1):
git send-email \
--in-reply-to=20190412212654.GA21627@redhat.com \
--to=joe.lawrence@redhat.com \
--cc=eshatokhin@virtuozzo.com \
--cc=jbaron@akamai.com \
--cc=jeyu@kernel.org \
--cc=jikos@kernel.org \
--cc=jmoreira@suse.de \
--cc=jpoimboe@redhat.com \
--cc=khlebnikov@yandex-team.ru \
--cc=linux-kbuild@vger.kernel.org \
--cc=linux-kernel@vger.kernel.org \
--cc=live-patching@vger.kernel.org \
--cc=matz@suse.de \
--cc=mbenes@suse.cz \
--cc=nstange@suse.de \
--cc=pmladek@suse.com \
--cc=yamada.masahiro@socionext.com \
/path/to/YOUR_REPLY
https://kernel.org/pub/software/scm/git/docs/git-send-email.html
* If your mail client supports setting the In-Reply-To header
via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line
before the message body.
This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.