From patchwork Thu Aug 11 18:47:27 2016 Content-Type: text/plain; charset="utf-8" MIME-Version: 1.0 Content-Transfer-Encoding: 7bit X-Patchwork-Submitter: Thomas Garnier X-Patchwork-Id: 9275803 Return-Path: Received: from mail.wl.linuxfoundation.org (pdx-wl-mail.web.codeaurora.org [172.30.200.125]) by pdx-korg-patchwork.web.codeaurora.org (Postfix) with ESMTP id 7A04C600CB for ; Thu, 11 Aug 2016 18:47:33 +0000 (UTC) Received: from mail.wl.linuxfoundation.org (localhost [127.0.0.1]) by mail.wl.linuxfoundation.org (Postfix) with ESMTP id 6C40428777 for ; Thu, 11 Aug 2016 18:47:33 +0000 (UTC) Received: by mail.wl.linuxfoundation.org (Postfix, from userid 486) id 6062228779; Thu, 11 Aug 2016 18:47:33 +0000 (UTC) X-Spam-Checker-Version: SpamAssassin 3.3.1 (2010-03-16) on pdx-wl-mail.web.codeaurora.org X-Spam-Level: X-Spam-Status: No, score=-6.8 required=2.0 tests=BAYES_00, DKIM_ADSP_CUSTOM_MED, DKIM_SIGNED, RCVD_IN_DNSWL_HI, T_DKIM_INVALID autolearn=ham version=3.3.1 Received: from vger.kernel.org (vger.kernel.org [209.132.180.67]) by mail.wl.linuxfoundation.org (Postfix) with ESMTP id D1C0D28777 for ; Thu, 11 Aug 2016 18:47:32 +0000 (UTC) Received: (majordomo@vger.kernel.org) by vger.kernel.org via listexpand id S932544AbcHKSra (ORCPT ); Thu, 11 Aug 2016 14:47:30 -0400 Received: from mail-yb0-f180.google.com ([209.85.213.180]:36152 "EHLO mail-yb0-f180.google.com" rhost-flags-OK-OK-OK-OK) by vger.kernel.org with ESMTP id S932219AbcHKSr3 (ORCPT ); Thu, 11 Aug 2016 14:47:29 -0400 Received: by mail-yb0-f180.google.com with SMTP id v8so1491912ybe.3 for ; Thu, 11 Aug 2016 11:47:28 -0700 (PDT) DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=google.com; s=20120113; h=mime-version:in-reply-to:references:from:date:message-id:subject:to :cc; bh=Nl8IqBEals4XPtprJBkHEDe9IRC1HB1G8QnXjWZAp5w=; b=R3yzX0K8Pi1V0C/0GD+Ry2iK1WWIOJYUOEE2+Q5EX2X4Ij+JdVA0ow91x4X8GiD6Y+ e2T4eK+dGRRPBU3DjEZllNDS3yjGnP2Qq0FB9miz3vxjqpGvF9f1qlyfS7svRGIK3hFI b4DPW72d5HSk0z5qHE9KweH4HXeK71SLNXZSpY2HasURsg+gxL48ritK8u6fLmjhD3HI oN+jOP9jhU3fbMsBSeLgVvILFxUbGOzEk6MshtFnXshzCddSwvnhBGRsVmoJBTJlV9Yb DaxZLXHPlpxqi81KJAX1mJ2Klro9aHuODdMcPHdxHUWlB/8yF44mXSqFynCrAZC9QduS Y55g== X-Google-DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=1e100.net; s=20130820; h=x-gm-message-state:mime-version:in-reply-to:references:from:date :message-id:subject:to:cc; bh=Nl8IqBEals4XPtprJBkHEDe9IRC1HB1G8QnXjWZAp5w=; b=EfDsB5oSyBIWMd/euhrlK4m0ZWyeLH+jUJwAA3uLabyGwjseILjpCBSlF+UVQXkSu8 vyKUdQH7D1dUPSzCReHah+J1BfAoSZTDaV1mieEHBXOnwqLNDLFPESKD93b+SBC79b34 KM4Bq+gb4FDsGUfTUK65sGHIa7dUfUB8hXtFAJ3blEfJdGGa8GD+KSNz/xsUT9TNwHgM +n/AdH0Ff/suVt6ecUe9tX0m+wv5fw6FczVUGDsn308tdmWSAaiJy4gXCMJaPYvdyY0b SX/A1alyIlNh7eOSHuMG3/qtrKmhhoZSywZ3fHp/gNCoLt193s55mxIBSB8rxFQyy5bh cAIA== X-Gm-Message-State: AEkoouspZE86N82wokRejBrC6LCAl+yXKp8EJ6WQ4zV6AlYi094NucGtwrjqcRbzc8Ry4zy8mQUp0sW42GsimeOy X-Received: by 10.37.210.197 with SMTP id j188mr7329447ybg.52.1470941248112; Thu, 11 Aug 2016 11:47:28 -0700 (PDT) MIME-Version: 1.0 Received: by 10.37.12.4 with HTTP; Thu, 11 Aug 2016 11:47:27 -0700 (PDT) In-Reply-To: References: <2206547.eDj3RJQyE5@vostro.rjw.lan> <2010434.TyoHiO9eal@vostro.rjw.lan> <1779906.DOHeuFCiDy@vostro.rjw.lan> <20160810163500.GA9424@nazgul.tnic> From: Thomas Garnier Date: Thu, 11 Aug 2016 11:47:27 -0700 Message-ID: Subject: Re: [Resend][PATCH] x86/power/64: Always create temporary identity mapping correctly To: "Rafael J. Wysocki" Cc: Jiri Kosina , Borislav Petkov , "Rafael J. Wysocki" , Linux PM list , "the arch/x86 maintainers" , Linux Kernel Mailing List , Yinghai Lu , Thomas Gleixner , Ingo Molnar , "H . Peter Anvin" , Kees Cook , Pavel Machek , Kernel Hardening Sender: linux-pm-owner@vger.kernel.org Precedence: bulk List-ID: X-Mailing-List: linux-pm@vger.kernel.org X-Virus-Scanned: ClamAV using ClamSMTP On Wed, Aug 10, 2016 at 6:35 PM, Rafael J. Wysocki wrote: > On Thu, Aug 11, 2016 at 3:17 AM, Thomas Garnier wrote: >> On Wed, Aug 10, 2016 at 5:35 PM, Rafael J. Wysocki wrote: >>> On Wed, Aug 10, 2016 at 11:59 PM, Jiri Kosina wrote: >>>> On Wed, 10 Aug 2016, Rafael J. Wysocki wrote: >>>> >>>>> So I used your .config to generate one for my test machine and with >>>>> that I can reproduce. >>>> >>>> Was that the config I've sent, or did Boris provide one as well? Which one >>>> are you able to reproduce with please? >>> >>> It's the Boris' one. >>> >>> Moreover, I have found the options that make the difference: unsetting >>> CONFIG_PROVE_LOCKING and CONFIG_DEBUG_LOCK_ALLOC (which also will >>> unset CONFIG_LOCKDEP AFAICS) in it makes hibernation work again with >>> CONFIG_RANDOMIZE_MEMORY set and with the $subject patch applied. >>> >>> Unbelievable, but that's what I'm seeing. >> >> Nice find! >> >>> >>> Now, that leads to a few questions: >>> >>> - How does lockdep change the picture so it matters for hibernation? >>> - Why is hibernation the only piece that's affected? >>> - Why is RANDOMIZE_MEMORY necessary to make this breakage show up? >>> >>> Thomas, any ideas? >> >> No idea so far. I will investigate though. >> >> We had an unrelated issue with CONFIG_DEBUG_PAGEALLOC on early boot. I >> don't think it was related because it was on early boot and with >> certain e820 memory layout (and PUD randomization that I disabled on >> the previous patch test). The fix is on tip: >> http://git.kernel.org/cgit/linux/kernel/git/tip/tip.git/commit/?id=fb754f958f8e46202c1efd7f66d5b3db1208117d > > Well, I don't think this is related. > > In the meantime, I went back to my original .config and verified that > setting CONFIG_DEBUG_LOCK_ALLOC in it caused hibernation to fail (with > CONFIG_RANDOMIZE_MEMORY set and with the $subject patch applied), so > this really matters somehow. > > Besides, now that I have a reproducer, I can check various other > things and for example this change (sorry for broken whitespace): > > Index: linux-pm/arch/x86/mm/kaslr.c > =================================================================== > --- linux-pm.orig/arch/x86/mm/kaslr.c > +++ linux-pm/arch/x86/mm/kaslr.c > @@ -122,7 +122,7 @@ void __init kernel_randomize_memory(void > prandom_bytes_state(&rand_state, &rand, sizeof(rand)); > entropy = (rand % (entropy + 1)) & PUD_MASK; > vaddr += entropy; > - *kaslr_regions[i].base = vaddr; > + *kaslr_regions[i].base += PUD_SIZE; > I think it works because the address is fixed now (just PUD aligned). > /* > * Jump the region and add a minimum padding based on > > makes hibernation work for me again in the above configuration. To > me, this means that the $subject patch works as expected and the > problem really is related to the vaddr value being too big. > I managed to debug the restoration and found that a first access violation happens here: (gdb) x/20i 0xffffffffb20a46de 0xffffffffb20a46de : mov eax,DWORD PTR gs:[rip+0x4df65a4b] # 0xa130 Handled by do_async_page_fault which will fault as well on this instruction: => 0xffffffffb2047ca1 : mov eax,DWORD PTR gs:[rip+0x4dfc4e58] # 0xcb00 So there is a problem with the gs register not being restored correctly at this stage. In create_image, there is tracing (trace_suspend_resume) before and after the suspend. Except at this stage, gs was not yet restored. It uses the old gs leading to the double fault. I tested this fix to be correct: Let me know if it works for you. Note that I don't know why this issue popup with the different config. > Thanks, > Rafael --- To unsubscribe from this list: send the line "unsubscribe linux-pm" in the body of a message to majordomo@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html diff --git a/kernel/power/hibernate.c b/kernel/power/hibernate.c index a881c6a..33c79b6 100644 --- a/kernel/power/hibernate.c +++ b/kernel/power/hibernate.c @@ -300,12 +300,12 @@ static int create_image(int platform_mode) save_processor_state(); trace_suspend_resume(TPS("machine_suspend"), PM_EVENT_HIBERNATE, true); error = swsusp_arch_suspend(); + /* Restore control flow magically appears here */ + restore_processor_state(); trace_suspend_resume(TPS("machine_suspend"), PM_EVENT_HIBERNATE, false); if (error) printk(KERN_ERR "PM: Error %d creating hibernation image\n", error); - /* Restore control flow magically appears here */ - restore_processor_state(); if (!in_suspend) events_check_enabled = false;