From e8f7f2dfe80a3f5953f56893cd80fb03ef9b081c Mon Sep 17 00:00:00 2001 From: Josh Poimboeuf Date: Tue, 23 Jul 2019 17:35:08 -0500 Subject: [PATCH 1/2] 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 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 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 [: 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 --- kpatch-build/create-diff-object.c | 9 +++++++++ 1 file changed, 9 insertions(+) diff --git a/kpatch-build/create-diff-object.c b/kpatch-build/create-diff-object.c index 1812fec24..6d43fef83 100644 --- a/kpatch-build/create-diff-object.c +++ b/kpatch-build/create-diff-object.c @@ -1282,6 +1282,15 @@ static void kpatch_replace_sections_syms(struct kpatch_elf *kelf) if (rela->sym->sec && rela->sym->sec->sym) { rela->sym = rela->sym->sec->sym; + /* + * On ppc64le with GCC6+, the function symbol + * starts 8 bytes past the beginning of the + * section, because of localentry. So even + * though the symbol is bundled, we can't + * assume it's at offset 0 in the section. + */ + rela->addend -= rela->sym->sym.st_value; + continue; } From cef336093601fcbed673d099fd8e37ee2b8bfacf Mon Sep 17 00:00:00 2001 From: Josh Poimboeuf Date: Tue, 23 Jul 2019 18:11:47 -0500 Subject: [PATCH 2/2] test/unit: Add unit test for ppc64le bundle replace_sections_syms() Signed-off-by: Josh Poimboeuf --- test/unit/objs | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/test/unit/objs b/test/unit/objs index 8ef8049ca..0b34cbb7e 160000 --- a/test/unit/objs +++ b/test/unit/objs @@ -1 +1 @@ -Subproject commit 8ef8049ca314b5f9ffd19832992d4d4899b044a2 +Subproject commit 0b34cbb7e835706615d2295f8c3129a23e19f6e6