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: 9275807 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 84C8E600CB for ; Thu, 11 Aug 2016 18:47:45 +0000 (UTC) Received: from mail.wl.linuxfoundation.org (localhost [127.0.0.1]) by mail.wl.linuxfoundation.org (Postfix) with ESMTP id 7845228777 for ; Thu, 11 Aug 2016 18:47:45 +0000 (UTC) Received: by mail.wl.linuxfoundation.org (Postfix, from userid 486) id 6CC9728779; Thu, 11 Aug 2016 18:47:45 +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=-4.1 required=2.0 tests=BAYES_00, DKIM_ADSP_CUSTOM_MED, DKIM_SIGNED, RCVD_IN_DNSWL_MED, T_DKIM_INVALID autolearn=ham version=3.3.1 Received: from mother.openwall.net (mother.openwall.net [195.42.179.200]) by mail.wl.linuxfoundation.org (Postfix) with SMTP id 5C98F28777 for ; Thu, 11 Aug 2016 18:47:43 +0000 (UTC) Received: (qmail 23586 invoked by uid 550); 11 Aug 2016 18:47:40 -0000 Mailing-List: contact kernel-hardening-help@lists.openwall.com; run by ezmlm Precedence: bulk List-Post: List-Help: List-Unsubscribe: List-Subscribe: List-ID: Reply-To: kernel-hardening@lists.openwall.com Delivered-To: mailing list kernel-hardening@lists.openwall.com Received: (qmail 23568 invoked from network); 11 Aug 2016 18:47:40 -0000 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=SLvW39mOwrzzgMZlkO0o5SiGmDoZYPlH7Bws6E7HeCmYGGyTFONG4tZuZ4R1D1EuKz 6pfEG/rIsQ+kqPW7pa7DZTdTnfsqsdmr3P2HkCnjASwz9I2K2FUsKdDx3gqb8GDXIy+l VYAZh26G9x3IzhxGRbGPm4k3ftrlXVFZOnoOKNKEbZ89rilj1fqvcovRAMCwvDfsa/vZ FeRRwlYn22XtZOFStFznBxhVl8HliYedXfP8/zQRs2J0MmFytmATcle1fcr23Tx4CIgT lJtn4Qh2JHilZjvgkRVjpJCiovGlQjUGfYzs7qfLB+iWDLtB4m5aebaf/L8slhS9HAWn DkYQ== X-Gm-Message-State: AEkoouvit4/0Yy1mYekl5btzjF0QDG+cGwm1t6jiL7V+nofl4SzYMiDhnXYID3Oh0blS9LAqdiaJcH95c54x7px+ 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 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: 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 Subject: [kernel-hardening] Re: [Resend][PATCH] x86/power/64: Always create temporary identity mapping correctly 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 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;