From mboxrd@z Thu Jan 1 00:00:00 1970 From: Laura Abbott Date: Wed, 23 Dec 2015 00:36:09 +0000 Subject: Re: [PATCH v2] ARM: mm: flip priority of CONFIG_DEBUG_RODATA Message-Id: <5679EC79.1080003@redhat.com> List-Id: References: <20151202202725.GA794@www.outflux.net> In-Reply-To: MIME-Version: 1.0 Content-Type: text/plain; charset="us-ascii" Content-Transfer-Encoding: 7bit To: Geert Uytterhoeven , Kees Cook , Russell King Cc: Arnd Bergmann , Ard Biesheuvel , Linux-sh list , Catalin Marinas , Nicolas Pitre , Will Deacon , LKML , Linux-MM , kernel-hardening@lists.openwall.com, "linux-arm-kernel@lists.infradead.org" , Laura Abbott On 12/22/2015 02:37 AM, Geert Uytterhoeven wrote: > Hi Kees, Russell, > > On Wed, Dec 2, 2015 at 9:27 PM, Kees Cook wrote: >> The use of CONFIG_DEBUG_RODATA is generally seen as an essential part of >> kernel self-protection: >> http://www.openwall.com/lists/kernel-hardening/2015/11/30/13 >> Additionally, its name has grown to mean things beyond just rodata. To >> get ARM closer to this, we ought to rearrange the names of the configs >> that control how the kernel protects its memory. What was called >> CONFIG_ARM_KERNMEM_PERMS is really doing the work that other architectures >> call CONFIG_DEBUG_RODATA. > > [...] > > This broke s2ram with shmobile_defconfig on r8a7791/koelsch: > > Freezing user space processes ... (elapsed 0.002 seconds) done. > Freezing remaining freezable tasks ... (elapsed 0.003 seconds) done. > PM: suspend of devices complete after 112.157 msecs > PM: late suspend of devices complete after 1.605 msecs > PM: noirq suspend of devices complete after 13.098 msecs > Disabling non-boot CPUs ... > s---[ end Kernel panic - not syncing: Attempted to kill the idle task! > CPU0: stopping > CPU: 0 PID: 2412 Comm: s2ram Tainted: G D > 4.4.0-rc6-00003-g1bb20571dcf0edfc #470 > Hardware name: Generic R8A7791 (Flattened Device Tree) > Backtrace: > [] (dump_backtrace) from [] (show_stack+0x18/0x1c) > r6:00000000 r5:00000000 r4:00000000 r3:80404000 > [] (show_stack) from [] (dump_stack+0x78/0x94) > [] (dump_stack) from [] (handle_IPI+0xf4/0x19c) > r4:c09313f0 r3:c09091ec > [] (handle_IPI) from [] (gic_handle_irq+0x7c/0x98) > r7:c0910b80 r6:ee1d5c30 r5:c0902754 r4:f0802000 > [] (gic_handle_irq) from [] (__irq_svc+0x54/0x70) > Exception stack(0xee1d5c30 to 0xee1d5c78) > 5c20: c0955484 00000002 > 00000000 60070013 > 5c40: c0942718 c093916c 00000005 0000000f 00000000 00000000 > c0943088 ee1d5cd4 > 5c60: ee1d5c08 ee1d5c80 c033fc20 c0158120 60070013 ffffffff > r8:00000000 r7:ee1d5c64 r6:ffffffff r5:60070013 r4:c0158120 r3:c033fc20 > [] (console_unlock) from [] (vprintk_emit+0x448/0x4a4) > r10:c09450a6 r9:00000000 r8:0000000e r7:00000005 r6:00000006 r5:c0932758 > r4:00000001 > [] (vprintk_emit) from [] (vprintk_default+0x28/0x30) > r10:c09055e0 r9:00000001 r8:c09055e0 r7:00000010 r6:00000000 r5:00000000 > r4:00000001 > [] (vprintk_default) from [] (printk+0x34/0x40) > [] (printk) from [] (__cpu_die+0x34/0x78) > r3:00000003 r2:c0906808 r1:00000001 r0:c0710af6 > [] (__cpu_die) from [] (_cpu_down+0x168/0x290) > r4:00000001 r3:00000005 > [] (_cpu_down) from [] (disable_nonboot_cpus+0x70/0xf0) > r10:00000051 r9:c0932734 r8:c0902528 r7:00000000 r6:c090245c r5:c0931b40 > r4:00000001 > [] (disable_nonboot_cpus) from [] > (suspend_devices_and_enter+0x290/0x3f8) > r8:c0714bb5 r7:eebac300 r6:00000003 r5:c0932734 r4:00000000 r3:00000000 > [] (suspend_devices_and_enter) from [] > (pm_suspend+0xb4/0x1c8) > r9:c093273c r8:c0714bb5 r7:eebac300 r6:00000003 r5:c09576fc r4:00000000 > [] (pm_suspend) from [] (state_store+0xb0/0xc4) > r6:00000004 r5:00000003 r4:00000003 r3:0000006d > [] (state_store) from [] (kobj_attr_store+0x1c/0x28) > r9:000cdc08 r8:ee1d5f80 r7:eebacb0c r6:eebacb00 r5:eebac300 r4:eebac300 > [] (kobj_attr_store) from [] (sysfs_kf_write+0x44/0x50) > [] (sysfs_kf_write) from [] > (kernfs_fop_write+0x13c/0x1a0) > r4:00000004 r3:c02223f4 > [] (kernfs_fop_write) from [] (__vfs_write+0x34/0xdc) > r10:00000000 r9:ee1d4000 r8:c0106fa4 r7:00000004 r6:ee1d5f80 r5:c02219a4 > r4:edf85d00 > [] (__vfs_write) from [] (vfs_write+0xb8/0x140) > r7:ee1d5f80 r6:000cdc08 r5:edf85d00 r4:00000004 > [] (vfs_write) from [] (SyS_write+0x50/0x90) > r9:ee1d4000 r8:c0106fa4 r7:000cdc08 r6:00000004 r5:edf85d00 r4:edf85d00 > [] (SyS_write) from [] (ret_fast_syscall+0x0/0x3c) > > Before commit 1bb20571dcf0edfc ("ARM: 8470/1: mm: flip priority of > CONFIG_DEBUG_RODATA"): > > # CONFIG_ARM_KERNMEM_PERMS is not set > > Freezing user space processes ... (elapsed 0.001 seconds) done. > Freezing remaining freezable tasks ... (elapsed 0.003 seconds) done. > PM: suspend of devices complete after 112.163 msecs > PM: late suspend of devices complete after 1.610 msecs > PM: noirq suspend of devices complete after 13.109 msecs > Disabling non-boot CPUs ... > CPU1: shutdown > > After the offending commit: > > CONFIG_DEBUG_RODATA=y > CONFIG_DEBUG_ALIGN_RODATA=y > > The "problem" is that DEBUG_RODATA now defaults to y on CPU_V7, so it gets > enabled for shmobile_defconfig. If I manually disable DEBUG_RODATA again, > s2ram does work. > > The real problem is something else, though. I can trigger the same panic > without the offending commit by enabling: > > CONFIG_ARM_KERNMEM_PERMS=y > CONFIG_DEBUG_RODATA=y > > I never enabled those options before, so I have no idea if this is a recent > regression. I've just tried a few older versions: on v4.4-rc1 I see the same > panic, on v4.3 (and v4.3.3) I don't see the panic, and the "CPU1: shutdown" > line, but the system doesn't wake up. > > Thanks for your suggestions! > > Gr{oetje,eeting}s, > > Geert > At a thought I think the RO/NX persmission are working as expected and something in the suspend code is writing or executing from where it shouldn't. I hit similar problems when working on RO/NX support for arm64. Looking in arch/arm/mach-shmobile/headsmp.S, it looks like shmobile_boot_fn, shmobile_boot_arg, shmobile_smp_mpdir, shmobile_smp_fn, and shmobile_smp_arg are ending up in the the text section which is going to be read_only. Assuming I understand the code flow, it looks like those are modified at suspend time which isn't going to work. I would say just throw those objects in the .data section but I notice shmobile_boot_size is there as well which seems to be calculated based off of the boot vector so you might need to do some re-working there. Thanks, Laura