Re: [PATCH] x86/ldt: Fix support_pte_mask filtering in map_ldt_struct()
On 04/19/2018 07:11 AM, Joerg Roedel wrote: > On Thu, Apr 19, 2018 at 03:00:11PM +0200, Borislav Petkov wrote: >> fb43d6cb91ef x86/mm: Do not auto-massage page protections <--- NOT OK > Hmm, that hunk from above patch looks suspicious: > > - set_pgd(pgd + pgd_index(restore_jump_address), > __pgd(__pa(pud) | _KERNPG_TABLE)); > + pgd_t new_pgd = __pgd(__pa(p4d) | pgprot_val(pgtable_prot)); > + set_pgd(pgd + pgd_index(restore_jump_address), new_pgd); > > The old code used __pa(pud) while the new one uses __pa(p4d). > > Boris, can you change that back to __pa(pud) and test please? That does look like the culprit. I probably copy/pasted the p4d line from above and missed that 'p4d' when converting it to pud. Thanks for finding this, guys.
Re: [PATCH] x86/ldt: Fix support_pte_mask filtering in map_ldt_struct()
On Thu, Apr 19, 2018 at 05:11:27PM +0200, Joerg Roedel wrote: > Cool, thanks for testing! I prepared a patch and will send it out if > Boris also reports success. I'll also added your Tested-by :) Boris reports success too. Very good catch, thanks! -- Regards/Gruss, Boris. SUSE Linux GmbH, GF: Felix Imendörffer, Jane Smithard, Graham Norton, HRB 21284 (AG Nürnberg) --
Re: [PATCH] x86/ldt: Fix support_pte_mask filtering in map_ldt_struct()
Hi Michal, On Thu, Apr 19, 2018 at 04:58:32PM +0200, Michal Kubecek wrote: > I had the same problem with v4.17-rc1 and fb43d6cb91ef and the change > you suggested applied on top of > > (a) fb43d6cb91ef with cherry-picked e6f39e87b643 > (b) current master (87ef12027b9b) > > fixes the resume on my system. Cool, thanks for testing! I prepared a patch and will send it out if Boris also reports success. I'll also added your Tested-by :) Thanks, Joerg
Re: [PATCH] x86/ldt: Fix support_pte_mask filtering in map_ldt_struct()
On Thu, Apr 19, 2018 at 04:11:34PM +0200, Joerg Roedel wrote: > On Thu, Apr 19, 2018 at 03:00:11PM +0200, Borislav Petkov wrote: > > fb43d6cb91ef x86/mm: Do not auto-massage page protections <--- NOT OK > > Hmm, that hunk from above patch looks suspicious: > > - set_pgd(pgd + pgd_index(restore_jump_address), > __pgd(__pa(pud) | _KERNPG_TABLE)); > + pgd_t new_pgd = __pgd(__pa(p4d) | pgprot_val(pgtable_prot)); > + set_pgd(pgd + pgd_index(restore_jump_address), new_pgd); > > The old code used __pa(pud) while the new one uses __pa(p4d). > > Boris, can you change that back to __pa(pud) and test please? I had the same problem with v4.17-rc1 and fb43d6cb91ef and the change you suggested applied on top of (a) fb43d6cb91ef with cherry-picked e6f39e87b643 (b) current master (87ef12027b9b) fixes the resume on my system. Michal Kubecek
Re: [PATCH] x86/ldt: Fix support_pte_mask filtering in map_ldt_struct()
On Thu, Apr 19, 2018 at 03:00:11PM +0200, Borislav Petkov wrote: > fb43d6cb91ef x86/mm: Do not auto-massage page protections <--- NOT OK Hmm, that hunk from above patch looks suspicious: - set_pgd(pgd + pgd_index(restore_jump_address), __pgd(__pa(pud) | _KERNPG_TABLE)); + pgd_t new_pgd = __pgd(__pa(p4d) | pgprot_val(pgtable_prot)); + set_pgd(pgd + pgd_index(restore_jump_address), new_pgd); The old code used __pa(pud) while the new one uses __pa(p4d). Boris, can you change that back to __pa(pud) and test please? Thanks, Joerg
Re: [PATCH] x86/ldt: Fix support_pte_mask filtering in map_ldt_struct()
On Wed, Apr 18, 2018 at 02:18:57PM +0200, Borislav Petkov wrote: > Btw, even with Jörg's fix, > > fb43d6cb91ef ("x86/mm: Do not auto-massage page protections") Some updates. 1. current linus master with Joerg's fix triple-faults on the IVB laptop during resume from disk. 2. current linus master with Joerg's fix + those two: C Apr 17 Dave Hansen ( : 67) [PATCH 1/2] x86, pti: fix boot problems from Global-bit setting C Apr 17 Dave Hansen ( : 81) └─>[PATCH 2/2] x86, pti: fix boot warning from Global-bit setting triplefaults too. Bisection confirms again fb43d6cb91ef is the first bad: fb43d6cb91ef x86/mm: Do not auto-massage page protections <--- NOT OK 6baf4bec02db x86/espfix: Document use of _PAGE_GLOBAL <--- OK -- Regards/Gruss, Boris. SUSE Linux GmbH, GF: Felix Imendörffer, Jane Smithard, Graham Norton, HRB 21284 (AG Nürnberg) --
Re: [PATCH] x86/ldt: Fix support_pte_mask filtering in map_ldt_struct()
+ Rafael. On Mon, Apr 16, 2018 at 08:39:37AM -0700, Dave Hansen wrote: > On 04/16/2018 08:16 AM, Andy Lutomirski wrote: > > On Mon, Apr 16, 2018 at 2:43 AM, Joerg Roedel wrote: > >> From: Joerg Roedel > >> > >> The |= operator will let us end up with an invalid PTE. Use > >> the correct &= instead. > > D'oh! Looks good. > > Yes, agreed. Thanks for finding that, Joerg! Btw, even with Jörg's fix, fb43d6cb91ef ("x86/mm: Do not auto-massage page protections") is still broken. In my case # CONFIG_MODIFY_LDT_SYSCALL is not set so Jörg's patch doesn't have any effect. I tried the patch before fb43d6cb91ef: 6baf4bec02db ("x86/espfix: Document use of _PAGE_GLOBAL") and the machine's fine. But with fb43d6cb91ef I can't resume from disk properly and I'm seeing is the below splat: [5.417480] PM: Image loading progress: 0% [5.631174] PM: Image loading progress: 10% [5.716705] PM: Image loading progress: 20% [5.805258] PM: Image loading progress: 30% [5.884919] random: crng init done [5.899245] PM: Image loading progress: 40% [5.980752] PM: Image loading progress: 50% [6.058269] PM: Image loading progress: 60% [6.138994] PM: Image loading progress: 70% [6.219384] PM: Image loading progress: 80% [6.299277] PM: Image loading progress: 90% [6.382403] PM: Image loading progress: 100% [6.386971] PM: Image loading done [6.390424] PM: Read 598112 kbytes in 1.03 seconds (580.69 MB/s) [6.397450] PM: Image successfully loaded [6.631431] ACPI: EC: interrupt blocked [6.635383] Disabling non-boot CPUs ... [6.821452] [ cut here ] [6.826115] kernel BUG at arch/x86/mm/physaddr.c:27! [6.831126] invalid opcode: [#1] PREEMPT SMP [6.835872] Modules linked in: [6.838976] CPU: 0 PID: 1 Comm: swapper/0 Not tainted 4.16.0+ #6 [6.845026] Hardware name: To be filled by O.E.M. To be filled by O.E.M./M5A97 EVO R2.0, BIOS 1503 01/16/2013 [6.854996] RIP: 0010:__phys_addr+0x38/0x50 [6.859220] RSP: 0018:c9017dc8 EFLAGS: 00010087 [6.864490] RAX: 7800 RBX: 880426f5b000 RCX: 0030 [6.871667] RDX: 8000 RSI: 1fe46a1b RDI: [6.878841] RBP: c9017e48 R08: R09: 0002 [6.886018] R10: 5ad731e5 R11: 02de R12: 0063 [6.893221] R13: R14: 016001e3 R15: [6.900396] FS: () GS:88043dc0() knlGS: [6.908544] CS: 0010 DS: ES: CR0: 80050033 [6.914333] CR2: 88043efff000 CR3: 02209000 CR4: 000406f0 [6.921507] Call Trace: [6.924007] swsusp_arch_resume+0x112/0x3c0 [6.928235] ? hibernate_resume_nonboot_cpu_disable+0x30/0x30 [6.934024] ? save_processor_state+0xc9/0x250 [6.938514] ? set_debug_rodata+0x11/0x11 [6.942570] hibernation_restore+0x8d/0x130 [6.946797] ? hibernation_restore+0x130/0x130 [6.951286] load_image_and_restore+0x5e/0x99 [6.955690] software_resume+0x20f/0x2a0 [6.959661] do_one_initcall+0x5c/0x1b0 [6.963544] kernel_init_freeable+0x123/0x1a5 [6.967945] ? rest_init+0xc0/0xc0 [6.971394] kernel_init+0xa/0x100 [6.974869] ret_from_fork+0x27/0x50 [6.978518] Code: 48 89 c2 72 28 48 b8 00 00 00 00 00 78 00 00 48 01 f8 48 39 c2 72 14 0f b6 0d fe e6 22 01 48 89 c2 48 d3 ea 48 85 d2 75 02 f3 c3 <0f> 0b 48 8b 05 0f 71 16 01 48 01 d0 48 81 fa ff ff ff 1f 76 e9 [6.997454] RIP: __phys_addr+0x38/0x50 RSP: c9017dc8 [7.003159] ---[ end trace 4b9d4661c2110f43 ]--- [7.007820] note: swapper/0[1] exited with preempt_count 1 [7.013353] Kernel panic - not syncing: Attempted to kill init! exitcode=0x000b [7.013353] [7.022563] Kernel Offset: disabled [7.026098] ---[ end Kernel panic - not syncing: Attempted to kill init! exitcode=0x000b [7.026098] ]--- -- Regards/Gruss, Boris. SUSE Linux GmbH, GF: Felix Imendörffer, Jane Smithard, Graham Norton, HRB 21284 (AG Nürnberg) --
Re: [PATCH] x86/ldt: Fix support_pte_mask filtering in map_ldt_struct()
On 04/16/2018 08:16 AM, Andy Lutomirski wrote: > On Mon, Apr 16, 2018 at 2:43 AM, Joerg Roedel wrote: >> From: Joerg Roedel >> >> The |= operator will let us end up with an invalid PTE. Use >> the correct &= instead. > D'oh! Looks good. Yes, agreed. Thanks for finding that, Joerg!
Re: [PATCH] x86/ldt: Fix support_pte_mask filtering in map_ldt_struct()
On Mon, Apr 16, 2018 at 2:43 AM, Joerg Roedel wrote: > From: Joerg Roedel > > The |= operator will let us end up with an invalid PTE. Use > the correct &= instead. D'oh! Looks good.