Messages in this thread | | | From | Andy Lutomirski <> | Date | Thu, 30 Jun 2016 10:23:14 -0700 | Subject | Re: [PATCH v4] x86/power/64: Fix kernel text mapping corruption during image restoration |
| |
On Thu, Jun 30, 2016 at 8:29 AM, Rafael J. Wysocki <rafael@kernel.org> wrote: > On Thu, Jun 30, 2016 at 5:24 PM, Andy Lutomirski <luto@amacapital.net> wrote: >> On Thu, Jun 30, 2016 at 8:17 AM, Rafael J. Wysocki <rafael@kernel.org> wrote: >>> On Thu, Jun 30, 2016 at 5:05 PM, Borislav Petkov <bp@alien8.de> wrote: >>>> On Thu, Jun 30, 2016 at 03:17:20PM +0200, Rafael J. Wysocki wrote: >>>>> From: Rafael J. Wysocki <rafael.j.wysocki@intel.com> >>>>> >>>>> Logan Gunthorpe reports that hibernation stopped working reliably for >>>>> him after commit ab76f7b4ab23 (x86/mm: Set NX on gap between __ex_table >>>>> and rodata). >>>> >>>> ... >>>> >>>>> +static int relocate_restore_code(void) >>>>> +{ >>>>> + pgd_t *pgd; >>>>> + pud_t *pud; >>>>> + >>>>> + relocated_restore_code = get_safe_page(GFP_ATOMIC); >>>>> + if (!relocated_restore_code) >>>>> + return -ENOMEM; >>>>> + >>>>> + memcpy((void *)relocated_restore_code, &core_restore_code, PAGE_SIZE); >>>>> + >>>>> + /* Make the page containing the relocated code executable */ >>>>> + pgd = (pgd_t *)__va(read_cr3()) + pgd_index(relocated_restore_code); >>>>> + pud = pud_offset(pgd, relocated_restore_code); >>>>> + if (pud_large(*pud)) { >>>>> + set_pud(pud, __pud(pud_val(*pud) & ~_PAGE_NX)); >>>>> + } else { >>>>> + pmd_t *pmd = pmd_offset(pud, relocated_restore_code); >>>>> + >>>>> + if (pmd_large(*pmd)) { >>>>> + set_pmd(pmd, __pmd(pmd_val(*pmd) & ~_PAGE_NX)); >>>>> + } else { >>>>> + pte_t *pte = pte_offset_kernel(pmd, relocated_restore_code); >>>>> + >>>>> + set_pte(pte, __pte(pte_val(*pte) & ~_PAGE_NX)); >>>>> + } >>>>> + } >>>>> + flush_tlb_all(); >>>> >>>> I know you want to flush TLBs but this causes the splat below on the >>>> resume kernel. >>>> >>>> Most likely because: >>>> >>>> resume_target_kernel() does local_irq_disable() and then >>>> >>>> swsusp_arch_resume() -> relocate_restore_code() -> flush_tlb_all() >>>> >>>> and smp_call_function_many() doesn't like it when IRQs are disabled. >>> >>> Right. >>> >>> Can I invoke __flush_tlb_all() from here to flush the TLB on the local >>> CPU? That should be sufficient IMO. >> >> Yes, unless there's another CPU already up at the time, but that seems unlikely. > > They are all offline at that time. > > OK, let me do that then. > >> FWIW, the CPU can and will cache the NX bit and the CPU will *not* >> refresh the TLB before sending a page fault. You definitely need to >> flush. > > OK > >> I wonder if this is also why you're seeing a certain amount of >> random corruption. > > Well, the corruption we saw was not really random. I can give you all > of the details if you're interested, maybe you'll be able to figure > out what might be happening. :-)
Sure, or at least in abbreviated form. No guarantee that I'll have any clue, though.
--Andy
| |