Re: [PATCH] x86/mm: Unbreak modules that rely on external PAGE_KERNEL availability

[Tom Lendacky]

On 11/8/2017 3:45 PM, Borislav Petkov wrote:
> On Wed, Nov 08, 2017 at 01:23:37PM -0800, Linus Torvalds wrote:
> > I was thinking that maybe we could have a fixed "encrypt" bit in our
> > PTE, and then replace that "software bit" with whatever the real
> > hardware mask is (if any).
>
> Right, I don't think that should be hard, unless I'm missing anything.
> We read that bit from CPUID and that's bit 47 of the physical address
> right now.
>
> Do you think we could reuse one of those _PAGE_BIT_SOFTW*?
>
> Right, and then set the proper *hardware* bit everytime we set a
> pteval_t.
>
> > Because it's nasty to have these constants that _used_ to be
> > constants, and still _look_ like constants, suddely do stupid memory
> > reads from random kernel data.
> >
> > So _this_ is the underflying problem:
> >
> >    #define _PAGE_ENC  (_AT(pteval_t, sme_me_mask))
> >
> > because that is simply not how the _PAGE_xyz macros should work!
>
> Yeah, I still have a funny feeling when looking at that but modulo
> better solutions... :-\
>
> > So it should have been a fixed bit to begin with, and the dynamic part
> > should have been elsewhere.
>
> Right, Tom, whaddya think? Do you see any issues with doing a software,
> "mirror" bit of sorts and then converting to the C-bit when needed?

I think that should be doable. It will take some investigation to see what
bit can be safely used and to find all of the areas where we would have to
translate the software bit to the hardware bit (as you mention below).

Thanks,
Tom

> > The whole EXPORT_SYMBOL() thing is just a symptom of that fundamental
> > error. Modules - GPL or not - should _never_ have to know or care
> > about this _PAGE_ENC bit madness, simply because it shouldn't have
> > been there.
>
> Right, so every user of the PAGE_* macros needs to set the C-bit when
> SME is enabled and everytime it creates a PTE so that the memory
> controller knows how to do the access. I certainly like your idea but
> we'd have to audit all the places where we need to convert to the C-bit
> from the software encryption bit and how ugly that would get.
>
> Btw, this is the other reason why the _PAGE_ENC bit is in the PAGE_*
> macros: for full encryption, everything that deals with PTEs needs to
> set the C-bit.