Re: [PATCH RFC] x86: check for and defend against BIOS memory corruption

Jeremy Fitzhardinge
2008-09-08T19:45:19Z

Hugh Dickins wrote:
> On Mon, 8 Sep 2008, Jeremy Fitzhardinge wrote:
>
>> Well, if we never want the direct map to be non-executable (which I
>>
>
> One too many negatives?
>

Yes. I over-corrected in my pre-send proofread.

>> think would be OK, since all the code is either core kernel or modules
>> which are mapped elsewhere),
>>
>
> I had thought so until just before replying.
>
>
>> then we can set NX on the level4 for the
>> linear mapping which will make everything below non-executable.
>>
>
> That would be much the neatest answer: I hadn't realized
> that inheritance (perhaps I'm still living in early-i386 days,
> when IIRC there was a bug in inheriting WP from higher levels).
>

In PAE mode the top-level doesn't let you set inheritable permissions
flags. On i386 it only works on levels 2 and 1, but 64-bit fixes it to
work on all levels.

> But then I stumbled across static_protections() in pageattr.c
> (takes both addr and pfn, latter seems weird),

Well, it wants to set the mappings for some physical memory in the case
of BIOS and RO data, but only affect the kernel mappings in a particular
virtual range (so the linear mapping of the kernel is still NX). (No
need for it to be inline though.)

But it does seem it wants the BIOS to be executable when you're using it
for PCI space. A possible fix in that case is to create a separate
executable mapping of BIOS space.

That said, PCI_GOBIOS is only used on 32-bit anyway, so it's moot (its
only use is to define CONFIG_PCI_BIOS, and it depends on X86_32).

> A simple answer might be to go the way you suggest, but remove
> the special casing from pageattr.c and ioremap.c; but I fear
> that will slow something down, or introduce further bugs.
>

It looks to me like it won't matter either way.

J