linux-kernel - Re: [PATCH v3] x86/hibernate: Use fixmap for saving unmapped pages

lists.openwall.net		lists / announce owl-users owl-dev john-users john-dev passwdqc-users yescrypt popa3d-users / oss-security kernel-hardening musl sabotage tlsify passwords / crypt-dev xvendor / Bugtraq Full-Disclosure linux-kernel linux-netdev linux-ext4 linux-hardening linux-cve-announce PHC
Open Source and information security mailing list archives

Hash Suite: Windows password security audit tool. GUI, reports in PDF.

[<prev] [next>] [<thread-prev] [day] [month] [year] [list]

Message-ID: <6dcd7d8117549fa06240af17ae86ff96c3b91807.camel@intel.com>
Date:   Mon, 23 Jan 2023 18:38:32 +0000
From:   "Edgecombe, Rick P" <rick.p.edgecombe@...el.com>
To:     "peterz@...radead.org" <peterz@...radead.org>,
        "rppt@...nel.org" <rppt@...nel.org>,
        "tglx@...utronix.de" <tglx@...utronix.de>,
        "dave.hansen@...ux.intel.com" <dave.hansen@...ux.intel.com>,
        "pavel@....cz" <pavel@....cz>, "x86@...nel.org" <x86@...nel.org>,
        "hpa@...or.com" <hpa@...or.com>,
        "mingo@...hat.com" <mingo@...hat.com>,
        "Lutomirski, Andy" <luto@...nel.org>,
        "bp@...en8.de" <bp@...en8.de>,
        "Hansen, Dave" <dave.hansen@...el.com>,
        "rafael@...nel.org" <rafael@...nel.org>,
        "Brown, Len" <len.brown@...el.com>
CC:     "linux-kernel@...r.kernel.org" <linux-kernel@...r.kernel.org>,
        "linux-pm@...r.kernel.org" <linux-pm@...r.kernel.org>
Subject: Re: [PATCH v3] x86/hibernate: Use fixmap for saving unmapped pages

On Mon, 2023-01-23 at 10:15 -0800, Dave Hansen wrote:
> On 1/19/23 15:51, Rick Edgecombe wrote:
> > Hibernate uses the direct map to read memory it saves to disk.
> > Since
> > sometimes pages are not accessible on the direct map ("not present"
> > on
> > x86), it has special case logic to temporarily make a page present.
> > On x86
> > these direct map addresses can be mapped at various page sizes, but
> > the
> > logic works ok as long as the not present pages are always mapped
> > as
> > PAGE_SIZE such that they don't require a split to map the region as
> > present. If the address was mapped not present by a larger page
> > size, the
> > split may fail and hibernate would then try to read an address
> > mapped not
> > present.
> 
> The "split" thing here kinda threw me a bit.
> 
> First, this code depends on having a 'struct page'.  On 64-bit, that
> means that the pages at least have an address in the direct map.
> 
> But, that doesn't mean that there's an actual mapping in the direct
> map
> for the page.  Lots of things zap the direct map.  To make up for
> this,
> the hibernate code tries to temporarily restore a zapped mapping with
> hibernate_map_page()->set_direct_map_default_noflush().
> 
> What's the actual failure mode here, though?  

It's a theoretical failure that was raised in the past. Last time, Mike
Rapoport added the warnings in hibernate. They have not been seen
AFAIK, but I don't *think* there can be any huge NP mappings on the
direct map today either.

The failure scenario is there is a 2MB region "mapped" with an NP
mapping (by that I mean the region of the direct map where the page is
mapped is marked as not present). Then when hibernate is saving the
page, it tries to set a 4k page in that region as present. CPA will try
to create a 4k region marked present and the rest remaining NP. So it
should try to split the huge mapping, which requires an allocation for
the new table which could fail.

> Does __change_page_attr()
> just fail to find an existing PTE and fall over?  Or, does it
> actually
> try to and fail to allocate the PTE page?

In CPA, if the allocation for the new table fails it would abort and
return an error to hibernate, which would print a warning then crash
trying to copy it as it reads from an NP mapping.