On Sat, Aug 14, 2021 at 8:59 AM Fabiano Rosas [off-list ref] wrote:
Laurent Vivier [off-list ref] writes:
quoted
since this patch is merged my VM is experiencing a crash at boot (20% of the time):
[ 8.496850] kernel tried to execute exec-protected page (c008000004073278) - exploit
attempt? (uid: 0)
[ 8.496921] BUG: Unable to handle kernel instruction fetch
[ 8.496954] Faulting instruction address: 0xc008000004073278
[ 8.496994] Oops: Kernel access of bad area, sig: 11 [#1]
[ 8.497028] LE PAGE_SIZE=64K MMU=Radix SMP NR_CPUS=2048 NUMA pSeries
[ 8.497071] Modules linked in: drm virtio_console fuse drm_panel_orientation_quirks xfs
libcrc32c virtio_net net_failover virtio_blk vmx_crypto failover dm_mirror dm_region_hash
dm_log dm_mod
[ 8.497186] CPU: 3 PID: 44 Comm: kworker/3:1 Not tainted 5.14.0-rc4+ #12
[ 8.497228] Workqueue: events control_work_handler [virtio_console]
[ 8.497272] NIP: c008000004073278 LR: c008000004073278 CTR: c0000000001e9de0
[ 8.497320] REGS: c00000002e4ef7e0 TRAP: 0400 Not tainted (5.14.0-rc4+)
[ 8.497361] MSR: 800000004280b033 <SF,VEC,VSX,EE,FP,ME,IR,DR,RI,LE> CR: 24002822
XER: 200400cf
[ 8.497426] CFAR: c0000000001e9e44 IRQMASK: 1
[ 8.497426] GPR00: c008000004073278 c00000002e4efa80 c000000002a26b00 c000000042c39520
[ 8.497426] GPR04: 0000000000000001 0000000000000000 0000000000000000 00000000000000ff
[ 8.497426] GPR08: 0000000000000001 c000000042c39520 0000000000000001 c008000004076008
[ 8.497426] GPR12: c0000000001e9de0 c0000001fffccb00 c00000000018ba88 c00000002c91d400
[ 8.497426] GPR16: 0000000000000000 0000000000000000 0000000000000000 0000000000000000
[ 8.497426] GPR20: 0000000000000000 0000000000000000 0000000000000000 c008000004080340
[ 8.497426] GPR24: c0080000040a01e8 0000000000000000 0000000000000000 c00000002e0975c0
[ 8.497426] GPR28: c00000002ce72940 c000000042c39520 0000000000000048 0000000000000038
[ 8.497891] NIP [c008000004073278] fill_queue+0xf0/0x210 [virtio_console]
[ 8.497934] LR [c008000004073278] fill_queue+0xf0/0x210 [virtio_console]
[ 8.497976] Call Trace:
[ 8.497993] [c00000002e4efa80] [c00800000407323c] fill_queue+0xb4/0x210
[virtio_console] (unreliable)
[ 8.498052] [c00000002e4efae0] [c008000004073a90] add_port+0x1a8/0x470 [virtio_console]
[ 8.498102] [c00000002e4efbb0] [c0080000040750f4] control_work_handler+0xbc/0x1e8
[virtio_console]
[ 8.498160] [c00000002e4efc60] [c00000000017f4f0] process_one_work+0x290/0x590
[ 8.498212] [c00000002e4efd00] [c00000000017f878] worker_thread+0x88/0x620
102 more quoted lines
Thanks for looking at this. I agree that this is the problem.
This avoids the crash (not a proper solution):
--- a/kernel/module.c
+++ b/kernel/module.c
@@ -2011,13 +2011,15 @@ static void module_enable_ro(const struct
module *mod, bool after_init)
if (!rodata_enabled)
return;
- set_vm_flush_reset_perms(mod->core_layout.base);
- set_vm_flush_reset_perms(mod->init_layout.base);
- frob_text(&mod->core_layout, set_memory_ro);
-
- frob_rodata(&mod->core_layout, set_memory_ro);
- frob_text(&mod->init_layout, set_memory_ro);
- frob_rodata(&mod->init_layout, set_memory_ro);
+ if (!after_init) {
+ set_vm_flush_reset_perms(mod->core_layout.base);
+ set_vm_flush_reset_perms(mod->init_layout.base);
+ frob_text(&mod->core_layout, set_memory_ro);
+
+ frob_rodata(&mod->core_layout, set_memory_ro);
+ frob_text(&mod->init_layout, set_memory_ro);
+ frob_rodata(&mod->init_layout, set_memory_ro);
+ }
I'm ignorant of strict rwx in general so I don't see why we need to
clear the bits before setting them to their final value, but as I
understand it, the set_pte_at + flush_tlb_kernel_range satisfy the ISA
requirement of [ptesync; tlbie; eieio; tlbsync; ptesync;] so it seems
like the patch should work.
I believe it's done like that because ISA 6.10.1.2 Modifying a
Translation Table Entry -
"The sequence is equivalent to deleting the PTE and then adding a new one".
In that sequence, I think it is [set pte to 0; ptesync; tlbie; eieio;
tlbsync; ptesync;]
But it does seem like we need to do something like your patch for
change_page_attr() to work as expected.
Now, I cannot explain why the crash always happens around the code
that does the module's symbols relocation (the NIP in Laurent's trace
is the TOC reload from module_64.c:restore_r2). Maybe because
instructions are already in icache until the first branch into the
stub?
Yeah, mpe thought it might be some cache effect of a branch instruction.
From time to time I did see Oopses like this (not "kernel tried to
execute exec-protected page")
[ 124.986964][ T52] Oops: Kernel access of bad area, sig: 11 [#1]
[ 124.987043][ T52] LE PAGE_SIZE=64K MMU=Radix SMP NR_CPUS=2048 NUMA pSeries
[ 124.987095][ T52] Modules linked in: virtio_console binfmt_misc
virtiofs fuse virtio_net virtio_blk virtio_pci virtio_pci_modern_dev
virtio_ring virtio crc32c_vpmsum [last unloaded: virtio_console]
[ 124.987209][ T52] CPU: 2 PID: 52 Comm: kworker/2:1 Not tainted
5.14.0-rc4 #4
[ 124.987259][ T52] Workqueue: events control_work_handler [virtio_console]
[ 124.987307][ T52] NIP: c008000001a86044 LR: c008000001a82cf8
CTR: c00000000042a6c0
[ 124.987358][ T52] REGS: c00000000b273770 TRAP: 0300 Not tainted
(5.14.0-rc4)
[ 124.987406][ T52] MSR: 800000000280b033
<SF,VEC,VSX,EE,FP,ME,IR,DR,RI,LE> CR: 82002882 XER: 00000004
[ 124.987475][ T52] CFAR: c008000001a82cf4 DAR: c008000001a86058
DSISR: 40000000 IRQMASK: 0
[ 124.987475][ T52] GPR00: c008000001a82ce8 c00000000b273a10
c008000001ab8000 c000000010731320
[ 124.987475][ T52] GPR04: 0000000000000001 0000000000000000
0000000000000000 0000000000000000
[ 124.987475][ T52] GPR08: 0000000000000000 0000000000000000
0000000000000000 c008000001a86038
[ 124.987475][ T52] GPR12: 0000000022002884 c0000001ffffdf00
c000000000169ae8 c0000000037700c0
[ 124.987475][ T52] GPR16: 0000000000000000 0000000000000000
c000000021af0000 c000000003313900
[ 124.987475][ T52] GPR20: 0000000000000001 0000000000000000
0000000000000000 c000000003316780
[ 124.987475][ T52] GPR24: c008000001a90288 0000000000000000
0000000000000000 c000000010731320
[ 124.987475][ T52] GPR28: c000000003316900 0000000000000018
0000000000000068 c00000000379c5a0
[ 124.988028][ T52] NIP [c008000001a86044] fini+0x824/0xa7e0 [virtio_console]
[ 124.988085][ T52] LR [c008000001a82cf8] fill_queue+0xb0/0x230
[virtio_console]
[ 124.988141][ T52] Call Trace:
21 more lines
Anyway, this is what Murilo and I found out over our debugging session
in the past couple of days. I hope it helps. =)
Yes it does, thanks heaps.