linux-kernel - Re: [PATCH v2] x86/mm: Fix PAT bit missing from page protection modify mask

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] [thread-next>] [day] [month] [year] [list]

Message-ID: <8bcfad6697316e200f78bd13e737345dc0436404.camel@intel.com>
Date:   Wed, 7 Jun 2023 22:47:36 +0000
From:   "Edgecombe, Rick P" <rick.p.edgecombe@...el.com>
To:     "Hansen, Dave" <dave.hansen@...el.com>,
        "janusz.krzysztofik@...ux.intel.com" 
        <janusz.krzysztofik@...ux.intel.com>, "bp@...en8.de" <bp@...en8.de>
CC:     "Gross, Jurgen" <jgross@...e.com>,
        "dave.hansen@...ux.intel.com" <dave.hansen@...ux.intel.com>,
        "x86@...nel.org" <x86@...nel.org>,
        "intel-gfx@...ts.freedesktop.org" <intel-gfx@...ts.freedesktop.org>,
        "hpa@...or.com" <hpa@...or.com>,
        "mingo@...hat.com" <mingo@...hat.com>,
        "tglx@...utronix.de" <tglx@...utronix.de>,
        "andi.shyti@...ux.intel.com" <andi.shyti@...ux.intel.com>,
        "linux-kernel@...r.kernel.org" <linux-kernel@...r.kernel.org>,
        "hannes@...xchg.org" <hannes@...xchg.org>
Subject: Re: [PATCH v2] x86/mm: Fix PAT bit missing from page protection
 modify mask

On Wed, 2023-06-07 at 23:33 +0200, Janusz Krzysztofik wrote:
> > So since _PAGE_PSE is actually the same value as _PAGE_PAT, you
> > don't
> > actually need to have _PAGE_PSE in _HPAGE_CHG_MASK in order to get
> > functional correctness. Is that right?
> 
> As soon as we add _PAGE_PAT to _PAGE_CHG_MASK -- yes, that's right. 
> But we 
> may still want to add _PAGE_PSE to _HPAGE_CHG_MASK to have the need
> for that 
> bit explicitly documented.

_PAGE_PSE is already in _HPAGE_CHG_MASK though, right? I'm confused.

> 
> > 
> > I think it is still a little hidden (even before this) and I wonder
> > about separating out the common bits into, like,
> > _COMMON_PAGE_CHG_MASK
> > or something. Then setting specific PAGE and HPAGE bits (like
> > _PAGE_PAT, _PAGE_PSE and _PAGE_PAT_LARGE) in their specific define.
> > Would it be more readable that way?
> 
> Yes, I think that's a good idea, and I can use it in my patch.
> 
> The question if _PAGE_PAT vel _PAGE_PSE added to _PAGE_CHG_MASK is
> really 
> harmless for pte_modify() and its users is still open for me though.

When you say "vel", this is similar to the english acronym "AKA" I
think?

So I think you mean, when you add _PAGE_PAT to _PAGE_CHG_MASK, you are
also adding _PAGE_PSE to it. So does that cause any problems? I see,
good question... 

vm_page_prot is used when creating PTEs and huge PMDs, and the setter
only uses _PAGE_CHG_MASK, even though it won't actually know where that
prot is going to end up.

Having _PAGE_PAT/PSE in _PAGE_CHG_MASK certainly doesn't make it easier
to think about. One thing it's favor though is vm_page_prot is not
applied to page table entries that are pointing to other page table
entries (PSE = 0). So you shouldn't accidentally set PSE=1. And
_PAGE_PSE shouldn't be being set in there, so you also shouldn't
accidentally be setting PAT=1.

But yea, I see why you are concerned. I would /guess/ it would be ok
functionally. That probably doesn't help much...