Commit Graph

369 Commits

Author SHA1 Message Date
Josh Poimboeuf
687e2caabc create-diff-object: Don't strip callback section symbols
Internal CI is reporting a SIGSEGV in create-diff-object when it
processes macro-callbacks.patch, starting with 19baa5b7c7
("create-diff-object: process debug sections last").

The problem is that, after changing the order between callback and debug
section inclusion, kpatch_include_debug_sections() now tries to include
the callback section symbols.  But kpatch_include_callback_elements()
inadvertently un-includes the callback section symbols (e.g.,
".kpatch.callbacks.pre_patch") when it un-includes the callback struct
symbols (e.g., "kpatch_pre_patch_data").

So after kpatch_elf_teardown(kelf_patched), the callback section symbols
get freed even though there are DWARF .debug_info relocations which
reference them.  Then kpatch_check_relocations() goes off into the weeds
when it accesses one of the freed symbols.

Fix it by refining the callback un-include logic so that it *only*
strips the struct object symbols.

Fixes: 19baa5b7c7 ("create-diff-object: process debug sections last")
Signed-off-by: Josh Poimboeuf <jpoimboe@redhat.com>
2020-02-18 10:16:39 -06:00
Josh Poimboeuf
648be4c380
Merge pull request #1053 from julien-thierry/stricter-static-twin
kpatch-build: Make local static twining stricter
2020-02-17 15:15:52 -06:00
Josh Poimboeuf
1b7871507a
Merge pull request #1055 from jpoimboe/process-debug-sections-last
create-diff-object: process debug sections last
2020-02-17 14:50:39 -06:00
Kamalesh Babulal
5f68db22e5 create-diff-object: Fix -Wconversion warnings
Fix warnings reported by -Wconversion,-Wno-sign-conversion flags.

Signed-off-by: Kamalesh Babulal <kamalesh@linux.vnet.ibm.com>
2020-02-10 09:07:21 +05:30
Evgenii Shatokhin
f5f5479614 create-diff-object: fix relocations used for ZERO_PAGE(0)
On x86_64, GCC generates the following instruction to compute
'empty_zero_page - __START_KERNEL_map' (__phys_addr_nodebug(), used in
the implementation of ZERO_PAGE()):

    48 ba 00 00 00 00 00 00 00 00   movabs $0x0,%rdx
          R_X86_64_64  empty_zero_page+0x80000000

__START_KERNEL_map is 0xffffffff80000000.

However, the relocation addend becomes wrong in the patch module:

    48 ba 00 00 00 00 00 00 00 00   movabs $0x0,%rdx
          R_X86_64_64  empty_zero_page-0x80000000

Note the sign of the addend.

As a result, ZERO_PAGE(0) returns a wrong value in any function touched
by the patch, which may lead to memory corruption and difficult-to-debug
kernel crashes.

The cause is that 'struct rela' uses 'int' for the addend, which is not
enough to store such values. r_addend from Elf64_Rela is int64_t
(Elf64_Sxword) for that.

Let us use 'long' instead of 'int' for the addend in 'struct rela'.

v2:
* Moved 'addend' field after 'offset' in struct rela to facilitate
  structure packing (suggested by Kamalesh Babulal).

Fixes https://github.com/dynup/kpatch/issues/1064.

Signed-off-by: Evgenii Shatokhin <eshatokhin@virtuozzo.com>
2020-01-20 11:41:01 +03:00
Julien Thierry
68530e4c2b kpatch-build: Make local static twining stricter
The current code to find the twin of a local static variable allows two
variables of the same name to be wrongly matched with the other's twin.

While there isn't a magic formula to avoid this, make stricter
requirements for twining static local from the original object with
a symbol from the patched object. This reduces the risk of erroneous
matches.

Signed-off-by: Julien Thierry <jthierry@redhat.com>
2019-11-22 09:53:16 +00:00
Josh Poimboeuf
19baa5b7c7 create-diff-object: process debug sections last
Process the debug sections only after all the other inclusion logic has
finished, since it makes decisions based on what else has already been
included.

Signed-off-by: Josh Poimboeuf <jpoimboe@redhat.com>
2019-11-01 15:26:07 -05:00
Julien Thierry
852bad5e8d create-diff-object: Use new helpers for static local correlation
Simplify static local variable correlation and renaming code by using
the newly introduced helpers for section and symbol correlation.

Signed-off-by: Julien Thierry <jthierry@redhat.com>
2019-10-29 15:29:51 +00:00
Julien Thierry
e49e3a59c2 create-diff-object: Rename elements getting correlated
Change 935f199875 ('create-diff-object: simplify mangled function
correlation') simplified the way symbols are correlated and got rid of
symbol section renaming.

As a result a symbol/section can now have a CHANGED status, being
correlated to an element that doesn't have the exact same name. This
will cause lookups to the original object fail when creating the new
patch object.

So lets bring back the symbol/section renaming, but only once they
have actually been correlated.

Fixes: 935f199875 ('create-diff-object: simplify mangled function
correlation')
Signed-off-by: Julien Thierry <jthierry@redhat.com>
2019-10-29 15:27:51 +00:00
Julien Thierry
49a9adaf74 create-diff-object: Correlate objects related to a section at once
Elements from the original object and the patched object can be
correlated using their mangled names. In case an elements (section or
symbol) could be matched with more than one object through mangling,
make sure all elements related to a section are correlated with the
corresponding elements of the twin section.

Signed-off-by: Julien Thierry <jthierry@redhat.com>
2019-10-29 15:27:51 +00:00
Josh Poimboeuf
935f199875 create-diff-object: simplify mangled function correlation
The RHEL powerpc kernel is compiled with -O3, which triggers some
"interesting" new optimizations.  One of them, which seems to be
relatively common, is the replacing of a function with two separate
"constprop" functions.

Previously we only ever saw a single constprop clone, so we just renamed
the patched version of the function to match the original version.  Now
that we can have two such clones, that no longer makes sense.

Instead of renaming functions, just improve the correlation logic such
that they can be correlated despite having slightly different symbol
names.  The first clone in the original object is correlated with the
first clone in the patched object; the second clone is correlated with
the second clone; and so on.

This assumes that the order of the symbols and sections doesn't change,
which seems to be a reasonable assumption based on past experience with
the compiler.  Otherwise it will just unnecessarily mark the cloned
constprop functions as changed, which is annoying but harmless, and
noticeable by a human anyway.

Fixes #935.

Signed-off-by: Josh Poimboeuf <jpoimboe@redhat.com>
2019-10-03 09:38:29 -05:00
Josh Poimboeuf
c175d8697c
Merge pull request #1039 from julien-thierry/misc-cleanups
Misc cleanups
2019-09-27 14:29:53 -05:00
Julien Thierry
878bac379f create-diff-object: Simplify relocation processing
If the symbol associated with a relocation does not have a section set,
nothing is done for that relocation.

Skip iterating through all the symbols of the ELF file in such a case.

Signed-off-by: Julien Thierry <jthierry@redhat.com>
2019-09-17 15:42:01 +01:00
Julien Thierry
bd4fae25f9 create-diff-object: Add const to unmodified rela_insn arguments
rela_insn() only retrieves information about an instruction and does not
modify sections or relocations.

Add const to make this explicit.

Signed-off-by: Julien Thierry <jthierry@redhat.com>
2019-09-17 15:42:01 +01:00
Julien Thierry
50476c94cc create-diff-object: Check ELF headers earlier
There is no point inspecting through the symbols of the ELF files
(original and patched) when the ELF headers do not meet requirements.

Check ELF headers as soon as the files are mapped.

Signed-off-by: Julien Thierry <jthierry@redhat.com>
2019-09-17 15:42:01 +01:00
Josh Poimboeuf
2499eb2bdc Revert "create-diff-object: Check for *_fixup sections changes"
We are seeing the following error on a real world patch:

  unsupported reference to special section __barrier_nospec_fixup

The kpatch commit bb444c2168 ("create-diff-object: Check for *_fixup
sections changes") created this error because we were trying to be
future proof.  However, that may have been overly paranoid, as it
doesn't seem likely that those fixup sections will need relocations
anytime soon, because the replacement instructions are manually
generated in code.  And anyway that "future proof" commit breaks the
present.

Also we decided at LPC that we are going to remove .klp.arch sections
anyway, so once that happens we will be fully future-proof anyway.

This reverts commit bb444c2168.

Fixes #974.

Signed-off-by: Josh Poimboeuf <jpoimboe@redhat.com>
2019-09-16 09:22:54 -05:00
Joe Lawrence
a23c82d9e6
Merge pull request #1026 from jpoimboe/ppc-replace-sections-syms-callbacks-fix
create-diff-object: Don't strip callback symbols
2019-08-19 10:50:36 -04:00
Josh Poimboeuf
2975775768 create-diff-object: allow dynamic debug static keys
While static keys (jump labels) are currently broken in livepatch, a
broken dynamic debug static key is harmless since it just disables
dynamically enabled debug printks in the patched code.

Fixes: #1021

Signed-off-by: Josh Poimboeuf <jpoimboe@redhat.com>
2019-08-15 16:01:55 -05:00
Josh Poimboeuf
b13aca7cde create-diff-object: Don't strip callback symbols
We saw the following panic on ppc64le when loading the macro-callbacks
integration test:

  livepatch: enabling patch 'kpatch_macro_callbacks'
  Oops: Exception in kernel mode, sig: 4 [#1]
  LE SMP NR_CPUS=2048 NUMA pSeries
  Modules linked in: kpatch_macro_callbacks(OEK+) rpcsec_gss_krb5 auth_rpcgss nfsv4 dns_resolver nfs lockd grace fscache sunrpc sg pseries_rng xts vmx_crypto xfs libcrc32c sd_mod ibmvscsi scsi_transport_srp ibmveth dm_mirror dm_region_hash dm_log dm_mod [last unloaded: kpatch_gcc_static_local_var_6]
  CPU: 2 PID: 17445 Comm: insmod Kdump: loaded Tainted: G           OE K  --------- -  - 4.18.0-128.el8.ppc64le #1
  NIP:  d00000000bb708e0 LR: c0000000001fd610 CTR: d00000000bb708e0
  REGS: c00000040e98f640 TRAP: 0700   Tainted: G           OE K  --------- -  -  (4.18.0-128.el8.ppc64le)
  MSR:  800000000288b033 <SF,VEC,VSX,EE,FP,ME,IR,DR,RI,LE>  CR: 28008228  XER: 20040003
  CFAR: c0000000001fd60c IRQMASK: 0
  GPR00: c0000000001fd5c0 c00000040e98f8c0 c000000001662a00 c000000733525400
  GPR04: 0000000000000800 0000000000000800 c0000000015e2c00 c0000007335254a8
  GPR08: 0000000000000001 d00000000bb708e0 c0000007eeb68400 0000000000000000
  GPR12: d00000000bb708e0 c000000007fad600 0000000000000001 aaaaaaaaaaaaaaab
  GPR16: 000000000000ff20 000000000000fff1 000000000000fff2 d00000000bb90000
  GPR20: 00000000000000a9 c00000040e98fc00 c000000000d8a728 c00000040e98fc00
  GPR24: d00000000bb73f88 00000000006080c0 d00000000bb73a38 c000000733525400
  GPR28: 0000000000000001 c000000733525400 ffffffffffffffed c0000007eeb60900
  NIP [d00000000bb708e0] callback_info.isra.0+0x7c/0x66c [kpatch_macro_callbacks]
  LR [c0000000001fd610] __klp_enable_patch+0x130/0x230
  Call Trace:
  [c00000040e98f8c0] [c0000000001fd5c0] __klp_enable_patch+0xe0/0x230 (unreliable)
  [c00000040e98f940] [c0000000001fd7d8] klp_enable_patch+0xc8/0x100
  [c00000040e98f980] [d00000000bb7079c] patch_init+0x460/0x4cc [kpatch_macro_callbacks]
  [c00000040e98fa20] [c000000000010108] do_one_initcall+0x58/0x248
  [c00000040e98fae0] [c00000000023b860] do_init_module+0x80/0x330
  [c00000040e98fb70] [c0000000002416a4] load_module+0x3994/0x3d00
  [c00000040e98fd30] [c000000000241cf4] sys_finit_module+0xc4/0x130
  [c00000040e98fe30] [c00000000000b388] system_call+0x5c/0x70
  Instruction dump:
  7cea482a 48000235 e8410018 48000014 3c620000 e8638160 48000221 e8410018
  38210060 e8010010 7c0803a6 4e800020 <0000ae18> 00000000 3c4c0001 3842ae18

The problem was introduced by a recent fix:

  e8f7f2dfe8 ("create-diff-object/ppc64le: Fix replace_sections_syms() for bundled symbols")

We didn't notice the fact that there's a hack in
kpatch_include_callback_elements() which reverts the work of
kpatch_replace_sections_syms() for callback function symbols.

The problem is that that revert is only partial, causing the callback
pointers to point to the .TOC data which is located 8 bytes before the
start of the function code.  This happens because
kpatch_include_callback_elements() makes the same assumption that
kpatch_replace_sections_syms() had previously made: that bundled symbols
are always located at the start of their corresponding sections.

kpatch_include_callback_elements() mysteriously strips references to the
callback function symbols, replacing them with section symbols.  In this
case it replaced a 'pre_patch_callback' function reference with a
'.text.unlikely.pre_patch_callback' section reference.  But it didn't
adjust the rela->addend accordingly.

Joe discovered the reasoning for why kpatch_include_callback_elements()
removes function symbol references in the commit log for 7dfad2fb76
("fix dynrela corruption in load/unload hooks"):

  In the case of the hook functions, we strip the FUNC symbol to prevent
  it from being added to the kpatch.funcs section as a patched function.

But that justification doesn't really make sense, at least not with the
current code.  Callbacks aren't added to .kpatch.funcs anyway.  They're
classifed as NEW.  Only CHANGED functions are added to .kpatch.funcs.

So remove that hack, fixing this bug in the process.

This does have a side effect of showing the callback functions as new
functions, because their symbols are now included.

Before:

  aio.o: found callback: post_unpatch_callback
  aio.o: found callback: pre_patch_callback
  aio.o: found callback: pre_unpatch_callback
  aio.o: new function: callback_info.isra.0

After:

  aio.o: found callback: post_unpatch_callback
  aio.o: found callback: pre_patch_callback
  aio.o: found callback: pre_unpatch_callback
  aio.o: new function: callback_info.isra.0
  aio.o: new function: pre_patch_callback
  aio.o: new function: post_patch_callback
  aio.o: new function: pre_unpatch_callback
  aio.o: new function: post_unpatch_callback

But anyway they _are_ new functions, so the new output seems more
correct to me.

Fixes: e8f7f2dfe8 ("create-diff-object/ppc64le: Fix replace_sections_syms() for bundled symbols")
Signed-off-by: Josh Poimboeuf <jpoimboe@redhat.com>
2019-08-15 15:08:31 -05:00
Josh Poimboeuf
14cc8a013d create-diff-object: add is_callback_section() helper
This simplifies the code a bit.  Also this helper will be needed for
subsequent patches.

Signed-off-by: Josh Poimboeuf <jpoimboe@redhat.com>
2019-08-15 14:08:29 -05:00
Josh Poimboeuf
5665d06853 create-diff-object: fix kpatch_replace_sections_syms() comment
The existing comment is wrong.  It confusingly conflates the function's
offset, which is 8 bytes from the beginning of the section, with the
function's localentry offset which is 8 bytes from the beginning of the
function.

Signed-off-by: Josh Poimboeuf <jpoimboe@redhat.com>
2019-08-15 14:02:09 -05:00
Artem Savkov
f1263a4292 create-diff-object: section size check in rela_insn()
Without this check we are risking returning uninitialized insn var.

Signed-off-by: Artem Savkov <asavkov@redhat.com>
2019-08-05 12:11:43 +02:00
Artem Savkov
7129bb48c9 create-diff-object: free hint variable on failure
Make sure we are freeing previously allocated "hint" variable on error
before exiting.

Signed-off-by: Artem Savkov <asavkov@redhat.com>
2019-08-05 12:11:43 +02:00
Artem Savkov
054915e957 create-diff-object: remove unneeded var initialization in kpatch_regenerate_special_section()
group_size variable is assigned right after we enter for loop without
ever being read so there is no need to initialize it to 0 beforehand.

Signed-off-by: Artem Savkov <asavkov@redhat.com>
2019-08-05 12:11:43 +02:00
Artem Savkov
1722f14221 create-diff-object: make sure sym->sec is not null in kpatch_replace_sections_syms()
Check that sym->sec is not null before dereferencing it.

Signed-off-by: Artem Savkov <asavkov@redhat.com>
2019-08-05 12:11:29 +02:00
Artem Savkov
9049abd0f5 create-diff-object: remove unused variables
"funcs" in kpatch_create_patches_sections() and "entries" in
kpatch_create_kpatch_arch_section() were only used by sizeof, replaced
those with corresponding types.

Signed-off-by: Artem Savkov <asavkov@redhat.com>
2019-08-05 12:10:47 +02:00
Artem Savkov
38efb307c2 create-diff-object: check symtab in main()
Make sure symtab section was found before dereferencing it.

Found by covscan, see issue #984 for full log.

Signed-off-by: Artem Savkov <asavkov@redhat.com>
2019-08-05 12:10:47 +02:00
Artem Savkov
dba95bae62 create-diff-object: check mallor retval in kpatch_create_mcount_sections()
newdata variable is allocated through malloc call and requires a NULL
check.

Found by covscan, see issue #984 for full log.

Signed-off-by: Artem Savkov <asavkov@redhat.com>
2019-08-05 12:10:47 +02:00
Artem Savkov
deb7719280 create-diff-object: check ORC_STRUCT_SIZE correctness in kpatch_regenerate_orc_sections()
Since ORC_STRUCT_SIZE is used for division in
kpatch_regenerate_orc_sections() we need to make sure that it is
properly set.

Found by covscan, see issue #984 for full log.

Signed-off-by: Artem Savkov <asavkov@redhat.com>
2019-08-05 12:10:47 +02:00
Artem Savkov
f0d071def7 create-diff-object: check fixupsec in fixup_group_size()
Make sure fixup section was found before dereferencing it.

Found by covscan, see issue #984 for full log.

Signed-off-by: Artem Savkov <asavkov@redhat.com>
2019-08-05 12:10:47 +02:00
Artem Savkov
f367438b19 create-diff-object: null checks in rela_equal()
Make sure rela_toc(1|2) are not null before dereferencing them in
rela_equal().

Found by covscan, see issue #984 for full log.

Signed-off-by: Artem Savkov <asavkov@redhat.com>
2019-08-05 12:10:47 +02:00
Josh Poimboeuf
e8f7f2dfe8 create-diff-object/ppc64le: Fix replace_sections_syms() for bundled symbols
With the following patch:

diff --git a/net/sunrpc/xprtsock.c b/net/sunrpc/xprtsock.c
index b60c9c7498dd..39a39ca89230 100644
--- a/net/sunrpc/xprtsock.c
+++ b/net/sunrpc/xprtsock.c
@@ -1594,6 +1594,8 @@ static void xs_tcp_state_change(struct sock *sk)
 	struct rpc_xprt *xprt;
 	struct sock_xprt *transport;

+	asm("nop");
+
 	read_lock_bh(&sk->sk_callback_lock);
 	if (!(xprt = xprt_from_sock(sk)))
 		goto out;

I saw the following panic on a RHEL8 kernel:

  Unable to handle kernel paging request for data at address 0xcc0080040
  Faulting instruction address: 0xc000000000b1515c
  Oops: Kernel access of bad area, sig: 7 [#1]
  LE SMP NR_CPUS=2048 NUMA PowerNV
  Modules linked in: rpcsec_gss_krb5 nfsv4 dns_resolver nfs fscache nfsd auth_rpcgss nfs_acl lockd grace kpatch_4_18_0_118_1_1(OEK) i2c_dev sunrpc ofpart powernv_flash at24 sg xts ipmi_powernv ipmi_devintf ipmi_msghandler uio_pdrv_genirq uio mtd vmx_crypto ibmpowernv opal_prd xfs libcrc32c sd_mod ast i2c_algo_bit drm_kms_helper syscopyarea sysfillrect sysimgblt fb_sys_fops ttm drm ahci libahci libata tg3 drm_panel_orientation_quirks dm_mirror dm_region_hash dm_log dm_mod
  CPU: 15 PID: 7814 Comm: kworker/u260:0 Kdump: loaded Tainted: G           OE K  --------- -  - 4.18.0-118.el8.ppc64le #1
  Workqueue: xprtiod xs_tcp_setup_socket [sunrpc]
  NIP:  c000000000b1515c LR: c000000000ad9968 CTR: c000000000b15140
  REGS: c000001fab6ff6b0 TRAP: 0300   Tainted: G           OE K  --------- -  -  (4.18.0-118.el8.ppc64le)
  MSR:  9000000000009033 <SF,HV,EE,ME,IR,DR,RI,LE>  CR: 44002222  XER: 20040000
  CFAR: c000000000078c7c DAR: 0000000cc0080040 DSISR: 00080000 IRQMASK: 0
  GPR00: c000000000ad9968 c000001fab6ff930 c000000001662800 0000000cc0080000
  GPR04: c00800000f5cfaa4 c000001f998fd0a8 c000001ff67e8080 c0000000016f46f0
  GPR08: c000001fb4918f80 0000000000000000 0000000cc0080040 c0000000011b8980
  GPR12: 0000000000002000 c000001ffffee200 c00000000017c458 c000001fe8a23a40
  GPR16: c00000000150e480 c000001fd6e90090 0000000000000000 0000000000000000
  GPR20: c00000000150e498 fffffffffffffef7 0000000000000402 0000000000000000
  GPR24: c000001fd6e90380 0000000000000000 c00800000f5cfaa4 0000000000000000
  GPR28: 00000000000004c4 c000001f998fd0a8 c00800000f5cfaa4 0000000cc0080000
  NIP [c000000000b1515c] dst_release+0x2c/0x110
  LR [c000000000ad9968] skb_release_head_state+0x178/0x190
  Call Trace:
  [c000001fab6ff930] [c000000000b15140] dst_release+0x10/0x110 (unreliable)
  [c000001fab6ff9a0] [c000000000ad9968] skb_release_head_state+0x178/0x190
  [c000001fab6ff9d0] [c000000000adb058] __kfree_skb+0x28/0x120
  [c000001fab6ffa00] [c000000000be8d64] tcp_rcv_state_process+0xc24/0x1180
  [c000001fab6ffa90] [c000000000cd5478] tcp_v6_do_rcv+0x1a8/0x5e0
  [c000001fab6ffae0] [c000000000ad1724] __release_sock+0xc4/0x1a0
  [c000001fab6ffb40] [c000000000ad1850] release_sock+0x50/0xe0
  [c000001fab6ffb70] [c000000000c20018] inet_stream_connect+0x68/0x90
  [c000001fab6ffbb0] [c000000000ac0f50] kernel_connect+0x30/0x50
  [c000001fab6ffbd0] [c00800000f55dc34] xs_tcp_setup_socket+0xbc/0x650 [sunrpc]
  [c000001fab6ffc70] [c000000000172014] process_one_work+0x2f4/0x5c0
  [c000001fab6ffd10] [c000000000172adc] worker_thread+0xcc/0x760
  [c000001fab6ffdc0] [c00000000017c5fc] kthread+0x1ac/0x1c0
  [c000001fab6ffe30] [c00000000000b75c] ret_from_kernel_thread+0x5c/0x80
  Instruction dump:
  60000000 3c4c00b5 3842d6d0 7c0802a6 4b563b61 fbe1fff8 f821ff91 7c7f1b79
  4182003c fbc10060 7c0004ac 395f0040 <7d205028> 3129ffff 7d20512d 40c2fff4

The problem is that the function has a GCC switch jump table, and the
.toc had the wrong offset for the jump table.

This is the switch jump table code from xs_tcp_state_changed():

  70:   12 00 3d 89     lbz     r9,18(r29)
  74:   0b 00 89 2b     cmplwi  cr7,r9,11
  78:   f8 02 9d 41     bgt     cr7,370 <xs_tcp_state_change+0x368>
  7c:   00 00 42 3d     addis   r10,r2,0
                        7c: R_PPC64_TOC16_HA    .toc+0x188
  80:   00 00 4a e9     ld      r10,0(r10)
                        80: R_PPC64_TOC16_LO_DS .toc+0x188
  84:   64 17 29 79     rldicr  r9,r9,2,61
  88:   aa 4a 2a 7d     lwax    r9,r10,r9
  8c:   14 52 29 7d     add     r9,r9,r10
  90:   a6 03 29 7d     mtctr   r9
  94:   20 04 80 4e     bctr
  98:   d8 02 00 00     .long 0x2d8
  9c:   38 00 00 00     .long 0x38
  a0:   d8 02 00 00     .long 0x2d8
  a4:   d8 02 00 00     .long 0x2d8
  a8:   68 02 00 00     .long 0x268
  ac:   d8 02 00 00     .long 0x2d8
  b0:   d8 02 00 00     .long 0x2d8
  b4:   c8 01 00 00     .long 0x1c8
  b8:   38 01 00 00     .long 0x138
  bc:   88 01 00 00     .long 0x188
  c0:   d8 02 00 00     .long 0x2d8
  c4:   68 01 00 00     .long 0x168

The switch jump table address is at offset 0x98.  The code reads this
offset from .toc+0x188:

Relocation section '.rela.toc' at offset 0x75320 contains 134 entries:
    Offset             Info             Type               Symbol's Value  Symbol's Name + Addend
0000000000000188  0000003f00000026 R_PPC64_ADDR64         0000000000000000 .text.xs_tcp_state_change + 98

After create-diff-object runs, the .toc entry now looks like this:

0000000000000188  0000000200000026 R_PPC64_ADDR64         0000000000000008 xs_tcp_state_change + 98

Notice the offset is the same, but it's now referring to the function
symbol instead of the text symbol.  That's done by
kpatch_replace_sections_syms().

On x86, that's not a problem, because the function symbol is at offset 0
in the .text.function section.  So the section symbol and the function
symbol are at the same location.

But on ppc64le, with -ffunction-sections, GCC 6+ somehow thinks it's a
good idea to associate the function symbol with the localentry point,
which is at an 8-byte offset from its corresponding section:

   Num:    Value          Size Type    Bind   Vis      Ndx Name
     2: 0000000000000008  1228 FUNC    LOCAL  DEFAULT    3 xs_tcp_state_change  [<localentry>: 8]

Notice the "Value" is 8 instead of 0.

That causes the .toc entry's jump table address to be wrongly offset by
8 bytes.

The fix is to adjust the rela addend accordingly in
kpatch_replace_sections_syms().

Signed-off-by: Josh Poimboeuf <jpoimboe@redhat.com>
2019-07-23 18:11:25 -05:00
Josh Poimboeuf
8b952bd771 create-diff-object/ppc64le: Don't allow sibling calls
With the following patch:

diff --git a/net/sunrpc/xprtsock.c b/net/sunrpc/xprtsock.c
index e008aefc3a9d..7c70e369390d 100644
--- a/net/sunrpc/xprtsock.c
+++ b/net/sunrpc/xprtsock.c
@@ -2228,6 +2228,8 @@ static void xs_tcp_shutdown(struct rpc_xprt *xprt)
 	struct socket *sock = transport->sock;
 	int skst = transport->inet ? transport->inet->sk_state : TCP_CLOSE;

+	asm("nop");
+
 	if (sock == NULL)
 		return;
 	switch (skst) {

We saw the following panic on a RHEL7.6 kernel:

  Unable to handle kernel paging request for data at address 0xd00000000577f390
  Faulting instruction address: 0xd000000002e918f4
  Oops: Kernel access of bad area, sig: 11 [#1]
  SMP NR_CPUS=2048 NUMA pSeries
  Modules linked in: kpatch_3_10_0_957_1_3_1_1(OEK) nfsd nfs_acl rpcsec_gss_krb5 auth_rpcgss nfsv4 dns_resolver nfs lockd grace fscache sunrpc virtio_balloon ip_tables xfs libcrc32c virtio_net virtio_console virtio_blk virtio_pci virtio_ring virtio dm_mirror dm_region_hash dm_log dm_mod
  CPU: 9 PID: 5961 Comm: kworker/9:1H Kdump: loaded Tainted: G           OE K------------   3.10.0-957.1.3.el7.ppc64le #1
  Workqueue: xprtiod xprt_autoclose [sunrpc]
  task: c00000000300c3c0 ti: c0000003f1814000 task.ti: c0000003f1814000
  NIP: d000000002e918f4 LR: d000000002e57394 CTR: c00000000089d100
  REGS: c0000003f1817980 TRAP: 0300   Tainted: G           OE K------------    (3.10.0-957.1.3.el7.ppc64le)
  MSR: 8000000100009033 <SF,EE,ME,IR,DR,RI,LE>  CR: 240f2084  XER: 20000000
  CFAR: 000000010bb5270c DAR: d00000000577f390 DSISR: 40000000 SOFTE: 1
  GPR00: c00000000000b054 c0000003f1817c00 d00000000579add8 c000000214f0f4d0
  GPR04: c0000003fd618200 c0000003fd618200 0000000000000001 0000000000000dc2
  GPR08: 0000000000000dc3 0000000000000000 0000000000000000 d00000000577f370
  GPR12: c0000003f1814000 c000000007b85100 c00000000012fd88 c0000003f711bb40
  GPR16: 0000000000000000 0000000000000000 0000000000000000 0000000000000000
  GPR20: 0000000000000001 c0000000013510b0 0000000000000001 fffffffffffffef7
  GPR24: 0000000000000000 0000000000000000 0000000000000000 c000000001b60600
  GPR28: c000000214f0f000 c000000214f0f4d0 c000000214f0f408 c000000214f0f448
  NIP [d000000002e918f4] __rpc_create_common.part.6+0x640/0x533c [sunrpc]
  LR [d000000002e57394] xprt_autoclose+0x74/0xe0 [sunrpc]
  Call Trace:
  [c0000003f1817c00] [c00000000000b054] livepatch_handler+0x30/0x80 (unreliable)
  [c0000003f1817c40] [c00000000012333c] process_one_work+0x1dc/0x680
  [c0000003f1817ce0] [c000000000123980] worker_thread+0x1a0/0x520
  [c0000003f1817d80] [c00000000012fe74] kthread+0xf4/0x100
  [c0000003f1817e30] [c00000000000a628] ret_from_kernel_thread+0x5c/0xb4
  Instruction dump:
  396b4570 f8410018 e98b0020 7d8903a6 4e800420 00000000 73747562 000f49c0
  c0000000 3d62fffe 396b4598 f8410018 <e98b0020> 7d8903a6 4e800420 00000000
  ---[ end trace 98e026b8fa880db7 ]---

The original version of xs_tcp_shutdown() has the following sequence:

  0xd000000003cfda44 <xs_tcp_shutdown+148>:       addi    r1,r1,64
  0xd000000003cfda48 <xs_tcp_shutdown+152>:       ld      r0,16(r1)
  0xd000000003cfda4c <xs_tcp_shutdown+156>:       ld      r29,-24(r1)
  0xd000000003cfda50 <xs_tcp_shutdown+160>:       ld      r30,-16(r1)
  0xd000000003cfda54 <xs_tcp_shutdown+164>:       ld      r31,-8(r1)
  0xd000000003cfda58 <xs_tcp_shutdown+168>:       mtlr    r0
  0xd000000003cfda5c <xs_tcp_shutdown+172>:       b       0xd000000003cfd768

That is, it restores the stack to the caller's stack frame and then does
a sibling call to the localentry point of xs_reset_transport()).  So
when xs_reset_transport() returns, it will return straight to
xs_tcp_shutdown()'s caller (xprt_autoclose).

The patched version of the function has this instead (dumped from a
vmcore):

  0xd000000003df0834 <xs_tcp_shutdown+148>:       addi    r1,r1,64
  0xd000000003df0838 <xs_tcp_shutdown+152>:       ld      r0,16(r1)
  0xd000000003df083c <xs_tcp_shutdown+156>:       ld      r29,-24(r1)
  0xd000000003df0840 <xs_tcp_shutdown+160>:       ld      r30,-16(r1)
  0xd000000003df0844 <xs_tcp_shutdown+164>:       ld      r31,-8(r1)
  0xd000000003df0848 <xs_tcp_shutdown+168>:       mtlr    r0
  0xd000000003df084c <xs_tcp_shutdown+172>:       b       0xd000000003df0ad0

After restoring the stack, instead of branching directly to
xs_reset_transport(), it (rightfully) branches to a toc stub.  A stub is
needed because the function it's branching to is in another module
(branching from the patch module to the sunrpc module).

The stub is:

  0xd000000003df0ad0 <xs_tcp_shutdown+816>:       addis   r11,r2,-1
  0xd000000003df0ad4 <xs_tcp_shutdown+820>:       addi    r11,r11,26328
  0xd000000003df0ad8 <xs_tcp_shutdown+824>:       std     r2,24(r1)
  0xd000000003df0adc <xs_tcp_shutdown+828>:       ld      r12,32(r11)
  0xd000000003df0ae0 <xs_tcp_shutdown+832>:       mtctr   r12
  0xd000000003df0ae4 <xs_tcp_shutdown+836>:       bctr

And the "std r2,24(r1)" corrupts the caller's stack.

This stub makes sense for a normal call, because the stack would be
owned by the caller of the stub, so it's ok to write r2 to it.  But
because this is a sibling call, the stack has been restored and r2 gets
incorrectly saved to the original caller's stack (i.e., xprt_autoclose's
stack).

So xprt_autoclose() -- which is in the sunrpc module -- gets the
livepatch module's toc pointer written to its stack.  It panics on when
it tries to use that vlue on its very next call.

Fix it by disallowing sibling calls from patched functions on ppc64le.

In theory we could instead a) generate a custom stub, or b) modify the
kernel livepatch_handler code to save/restore the stack r2 value, but
this is easier for now.

Signed-off-by: Josh Poimboeuf <jpoimboe@redhat.com>
2019-07-23 11:11:23 -05:00
Josh Poimboeuf
4f4870dd05 create-diff-object: Don't allow jump labels
Create an error if a patched function uses a jump label.  We need this
until upstream livepatch supports them with a .klp.arch section.

Fixes #946.

Signed-off-by: Josh Poimboeuf <jpoimboe@redhat.com>
2019-07-17 09:03:17 -05:00
Josh Poimboeuf
7b367a4ef5 create-diff-object: Support dynamic debug on newer kernels
After Linux commit 47cdd64be483 ("dynamic_debug: refactor
dynamic_pr_debug and friends"), the name of the static local variable
used for dynamic printks is no longer "descriptor".

Make the is_special_static() check broader such that it doesn't care
about the variable name, and just whitelists any variable in the
__verbose section.

Fixes #990.

Signed-off-by: Josh Poimboeuf <jpoimboe@redhat.com>
2019-07-15 18:20:21 -05:00
Josh Poimboeuf
b5745d7ea6 Add support for R_X86_64_PLT32
Starting with binutils 2.31, the Linux kernel may have R_X86_64_PLT32
relocations. Make sure we support them. This should be as simple as
treating R_X86_64_PLT32 exactly like R_X86_64_PC32 everywhere. For more
details see upstream commit torvalds/linux@b21ebf2.

This also fixes the following issue seen on Fedora 29:

```
$ kpatch-build/kpatch-build -t vmlinux ./test/integration/fedora-27/convert-global-local.patch
Using cache at /home/jpoimboe/.kpatch/src
Testing patch file(s)
Reading special section data
Building original source
Building patched source
Extracting new and modified ELF sections
ERROR: slub.o: 1 function(s) can not be patched
slub.o: function __kmalloc has no fentry/mcount call, unable to patch
/home/jpoimboe/git/kpatch/kpatch-build/create-diff-object: unreconcilable difference
ERROR: 1 error(s) encountered. Check /home/jpoimboe/.kpatch/build.log for more details.
```

Fixes #975.

Signed-off-by: Josh Poimboeuf <jpoimboe@redhat.com>
2019-07-05 11:45:50 -05:00
Kamalesh Babulal
bb444c2168 create-diff-object: Check for *_fixup sections changes
Currently we do not support changes to functions referring to any of the
*_fixup sections on ppc64le. This patch introduces check for such
changes during the patchability check, where we abort building the
patch module.

This patch implements the phase 1 fix of 3 phases discussed at
https://github.com/dynup/kpatch/issues/974:
"
Phase 1 fix:

For kernel versions which don't have livepatch-specific powerpc code
(currently all kernel versions), kpatch-build needs to assert an error
if it detects that one of the following sections refers to a patched
function: __ftr_fixup, __mmu_ftr_fixup, __fw_ftr_fixup.
"

Suggested-by: Josh Poimboeuf <jpoimboe@redhat.com>
Signed-off-by: Kamalesh Babulal <kamalesh@linux.vnet.ibm.com>
2019-06-25 14:46:55 +05:30
Kamalesh Babulal
09a5686277 create-diff-object: Add support for __spec_barrier_fixup
Add support for __spec_barrier_fixup (barrier nospec fixup) special
section on ppc64le.

Suggested-by: Josh Poimboeuf <jpoimboe@redhat.com>
Signed-off-by: Kamalesh Babulal <kamalesh@linux.vnet.ibm.com>
2019-06-19 13:24:25 +05:30
Kamalesh Babulal
a2fb8719b7 create-diff-object: Fix lwsync group size
Fix the size of special group __lwsync_fixup on ppc64le.

Suggested-by: Josh Poimboeuf <jpoimboe@redhat.com>
Signed-off-by: Kamalesh Babulal <kamalesh@linux.vnet.ibm.com>
2019-06-19 13:24:22 +05:30
Joe Lawrence
13b0014671 kpatch-build: ensure that .altinstr_replacement section is included
.altinstr_replacement section may have relocation symbols which need to
be included, therefore we should call kpatch_include_symbol() to ensure
that its section is included as well.

The special section processing should also occur before
kpatch_print_changes() to provide accurate logging info.

Signed-off-by: Joe Lawrence <joe.lawrence@redhat.com>
2019-05-17 10:31:04 -04:00
chenzefeng
b6e19c7795 create-diff-object: fix the condition for the sections changed
The create-diff-object.c create intermediate ".kpatch.relocations"
sections instead of ".kpatch.dynrelas" sections, and add a new
section ".rela.kpatch.symbols", so we should update the conditions
in function kpatch_create_intermediate_sections for these changed.

Fixes: 87643703a7 ("create-diff-object: create .kpatch.relocations and .kpatch.symbols sections")

Signed-off-by: chenzefeng <chenzefeng2@huawei.com>
2019-05-10 08:58:36 +08:00
chenzefeng
8e3ffbc8f6 create-diff-objject: fix memleak of the struct lookup_table
reason: Firstly, in the function lookup_open use the malloc to
	allocate some memory, but call the function lookup_close
	to free the memory.
	Secondly, table->obj_sym->name, table->exp_sym->name and
	table->exp_sym->objname used the strdup, so them should
	free also.
	Thirdly, adjust the order of make_nodname, if not, it
	will cause an exception when free(exp_sym->objname) in
	lookup_close.

Signed-off-by: chenzefeng <chenzefeng2@huawei.com>
2019-04-18 10:15:29 +08:00
Josh Poimboeuf
09ee03f3df Revert "create-diff-object: add jump label support"
This reverts commit 87c64519fc.

The jump label support doesn't work with upstream livepatch.  Joe
Lawrence found the following ordering issue:

load_module

  apply_relocations

    /* Livepatch relocation sections are applied by livepatch */
    if (info->sechdrs[i].sh_flags & SHF_RELA_LIVEPATCH)
            continue;

  post_relocation
    module_finalize
      jump_label_apply_nops        << crash

  ...

  do_init_module
    do_one_initcall(mod->init)
      __init patch_init [kpatch-patch]
        klp_register_patch
          klp_init_patch
            klp_init_object
              klp_init_object_loaded
                klp_write_object_relocations

So jump_label_apply_nops() is called *before*
klp_write_object_relocations() has had a chance to write the klp
relocations (.klp.rela.kvm_intel.__jump_table, for example).

We need to resolve this upstream first.

Signed-off-by: Josh Poimboeuf <jpoimboe@redhat.com>
2019-02-19 16:10:54 -06:00
Joe Lawrence
b4e6085b6d
Merge pull request #928 from haoren3696/master
kpatch-build: include secsym in kpatch_mark_ignored_sections
2019-02-19 13:24:42 -05:00
Josh Poimboeuf
87c64519fc create-diff-object: add jump label support
Add support for jump labels, also known as static jumps, static keys,
static branches, and jump tables.  Luckily,
kpatch_process_special_sections() is already generic enough to make this
an easy fix.

Fixes: #931

Signed-off-by: Josh Poimboeuf <jpoimboe@redhat.com>
2019-02-08 13:45:38 -06:00
Josh Poimboeuf
d8a44076f8 create-diff-object: cleanup special section array
Clean up the special section array a bit, to make it a little more
readable.

Signed-off-by: Josh Poimboeuf <jpoimboe@redhat.com>
2019-01-29 13:04:09 -06:00
Balbir singh
3998784d71 Fix NULL pointer deref in main due to base_locals
For fun I tried to create a livepatch of upstream patch
ad211f3e94b314a910d4af03178a0b52a7d1ee0a for my kernel. This
caused kpatch-build to fail with a NULL pointer derefence because
base_locals was NULL (returned via kpatch_elf_locals(), which
can return a NULL pointer). This patch fixes the SIGSEGV
via a NULL check. The end result is a live patch is created
and loaded.

Signed-off-by: Balbir singh <bsingharora@gmail.com>
2019-01-17 19:53:46 +11:00
Zhipeng Xie
517e26a6cb kpatch-build: include secsym in kpatch_mark_ignored_sections
kpatch_mark_ignored_sections include .rodata.str1.1 section but does
not include its section symbol, causing its section symbol can not be
included any more in kpatch_include_standard_elements. After the
section symbol is freed in kpatch_elf_teardown, we got a segmentation
fault in kpatch_create_intermediate_sections.

Signed-off-by: Zhipeng Xie <xiezhipeng1@huawei.com>
2018-11-23 10:50:21 +08:00
Joe Lawrence
019c1fb3eb
Merge pull request #919 from jpoimboe/__FUNCTION__-special-static
create-diff-object: add __FUNCTION__ variables to the special static …
2018-10-18 09:46:03 -04:00
Josh Poimboeuf
72103a178c create-diff-object: add __FUNCTION__ variables to the special static list
As discovered in #918, the `__FUNCTION__` static local variable is
similar to the `__func__` variable, in that it refers to the current
function name.

Signed-off-by: Josh Poimboeuf <jpoimboe@redhat.com>
2018-10-17 21:08:30 -05:00