Re: [GIT PULL] x86/mm changes for v4.4

From	Ard Biesheuvel <ard.biesheuvel@linaro.org>
Newsgroups	linux.kernel
Subject	Re: [GIT PULL] x86/mm changes for v4.4
Date	2015-11-07 08:40 +0100
Message-ID	<qs6gp-7Jo-3@gated-at.bofh.it> (permalink)
References	(4 earlier) <qrijD-mC-1@gated-at.bofh.it> <qrAgx-3yX-3@gated-at.bofh.it> <qrJaa-13t-7@gated-at.bofh.it> <qrOtc-4v4-35@gated-at.bofh.it> <qs5Nn-7zp-3@gated-at.bofh.it>
Organization	linux.* mail to news gateway

Show all headers | View raw

On 7 November 2015 at 08:09, Ingo Molnar <mingo@kernel.org> wrote:
>
> * Matt Fleming <matt@codeblueprint.co.uk> wrote:
>
>> On Fri, 06 Nov, at 07:55:50AM, Ingo Molnar wrote:
>> >
>> >  3) We should fix the EFI permission problem without relying on the firmware: it
>> >     appears we could just mark everything R-X optimistically, and if a write fault
>> >     happens (it's pretty rare in fact, only triggers when we write to an EFI
>> >     variable and so), we can mark the faulting page RW- on the fly, because it
>> >     appears that writable EFI sections, while not enumerated very well in 'old'
>> >     firmware, are still supposed to be page granular. (Even 'new' firmware I
>> >     wouldn't automatically trust to get the enumeration right...)
>>
>> Sorry, this isn't true. I misled you with one of my earlier posts on
>> this topic. Let me try and clear things up...
>>
>> Writing to EFI regions has to do with every invocation of the EFI
>> runtime services - it's not limited to when you read/write/delete EFI
>> variables. In fact, EFI variables really have nothing to do with this
>> discussion, they're a completely opaque concept to the OS, we have no
>> idea how the firmware implements them. Everything is done via the EFI
>> boot/runtime services.
>>
>> The firmware itself will attempt to write to EFI regions when we
>> invoke the EFI services because that's where the PE/COFF ".data" and
>> ".bss" sections live along with the heap. There's even some relocation
>> fixups that occur as SetVirtualAddressMap() time so it'll write to
>> ".text" too.
>>
>> Now, the above PE/COFF sections are usually (always?) contained within
>> EFI regions of type EfiRuntimeServicesCode. We know this is true
>> because the firmware folks have told us so, and because stopping that
>> is the motivation behind the new EFI_PROPERTIES_TABLE feature in UEFI
>> V2.5.
>>
>> The data sections within the region are also *not* guaranteed to be
>> page granular because work was required in Tianocore for emitting
>> sections with 4k alignment as part of the EFI_PROPERTIES_TABLE
>> support.
>>
>> Ultimately, what this means is that if you were to attempt to
>> dynamically fixup those regions that required write permission, you'd
>> have to modify the mappings for the majority of the EFI regions
>> anyway. And if you're blindly allowing write permission as a fixup,
>> there's not much security to be had.
>
> I think you misunderstood my suggestion: the 'fixup' would be changing it from R-X
> to RW-, i.e. it would add 'write' permission but remove 'execute' permission.
>
> Note that there would be no 'RWX' permission at any given moment - which is the
> dangerous combination.
>

The problem with that is that /any/ page in the UEFI runtime region
may intersect with both .text and .data of any of the PE/COFF images
that make up the runtime firmware (since the PE/COFF sections are not
necessarily page aligned). Such pages require RWX permissions. The
UEFI memory map does not provide the information to identify those
pages a priori (the entire region containing several PE/COFF images
could be covered by a single entry) so it is hard to guess which pages
should be allowed these RWX permissions.

>> >     If that 'supposed to be' turns out to be 'not true' (not unheard of in
>> >     firmware land), then plan B would be to mark pages that generate write faults
>> >     RWX as well, to not break functionality. (This 'mark it RWX' is not something
>> >     that exploits would have easy access to, and we could also generate a warning
>> >     [after the EFI call has finished] if it ever triggers.)
>> >
>> >     Admittedly this approach might not be without its own complications, but it
>> >     looks reasonably simple (I don't think we need per EFI call page tables,
>> >     etc.), and does not assume much about the firmware being able to enumerate its
>> >     permissions properly. Were we to merge EFI support today I'd have insisted on
>> >     trying such an approach from day 1 on.
>>
>> We already have separate EFI page tables, though with the caveat that
>> we share some of swapper_pg_dir's PGD entries. The best solution would
>> be to stop sharing entries and isolate the EFI mappings from every
>> other page table structure, so that they're only used during the EFI
>> service calls.
>
> Absolutely. Can you try to fix this for v4.3?
>
> Thanks,
>
>         Ingo
--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majordomo@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/

Thread

[GIT PULL] x86/mm changes for v4.4 Ingo Molnar <mingo@kernel.org> - 2015-11-03 12:20 +0100
  Re: [GIT PULL] x86/mm changes for v4.4 Linus Torvalds <torvalds@linux-foundation.org> - 2015-11-04 20:30 +0100
    Re: [GIT PULL] x86/mm changes for v4.4 Dave Jones <davej@codemonkey.org.uk> - 2015-11-05 00:40 +0100
      Re: [GIT PULL] x86/mm changes for v4.4 Linus Torvalds <torvalds@linux-foundation.org> - 2015-11-05 02:40 +0100
        Re: [GIT PULL] x86/mm changes for v4.4 Dave Jones <davej@codemonkey.org.uk> - 2015-11-05 03:20 +0100
          Re: [GIT PULL] x86/mm changes for v4.4 Linus Torvalds <torvalds@linux-foundation.org> - 2015-11-05 22:30 +0100
            Re: [GIT PULL] x86/mm changes for v4.4 Linus Torvalds <torvalds@linux-foundation.org> - 2015-11-05 22:40 +0100
              Re: [GIT PULL] x86/mm changes for v4.4 Matt Fleming <matt@codeblueprint.co.uk> - 2015-11-06 12:40 +0100
                Re: [GIT PULL] x86/mm changes for v4.4 Ingo Molnar <mingo@kernel.org> - 2015-11-07 08:10 +0100
                Re: [GIT PULL] x86/mm changes for v4.4 Matt Fleming <matt@codeblueprint.co.uk> - 2015-11-07 11:10 +0100
            Re: [GIT PULL] x86/mm changes for v4.4 Linus Torvalds <torvalds@linux-foundation.org> - 2015-11-05 23:10 +0100
              Re: [GIT PULL] x86/mm changes for v4.4 Borislav Petkov <bp@alien8.de> - 2015-11-05 23:30 +0100
            Re: [GIT PULL] x86/mm changes for v4.4 Ingo Molnar <mingo@kernel.org> - 2015-11-06 08:00 +0100
              Re: [GIT PULL] x86/mm changes for v4.4 Andy Lutomirski <luto@amacapital.net> - 2015-11-06 08:10 +0100
                Re: [GIT PULL] x86/mm changes for v4.4 Matt Fleming <matt@codeblueprint.co.uk> - 2015-11-06 14:10 +0100
                Re: [GIT PULL] x86/mm changes for v4.4 Borislav Petkov <bp@alien8.de> - 2015-11-06 14:30 +0100
                Re: [GIT PULL] x86/mm changes for v4.4 Ingo Molnar <mingo@kernel.org> - 2015-11-07 08:10 +0100
              Re: [GIT PULL] x86/mm changes for v4.4 Ingo Molnar <mingo@kernel.org> - 2015-11-06 08:50 +0100
              Re: [GIT PULL] x86/mm changes for v4.4 Matt Fleming <matt@codeblueprint.co.uk> - 2015-11-06 13:40 +0100
                Re: [GIT PULL] x86/mm changes for v4.4 Ingo Molnar <mingo@kernel.org> - 2015-11-07 08:10 +0100
                Re: [GIT PULL] x86/mm changes for v4.4 Ard Biesheuvel <ard.biesheuvel@linaro.org> - 2015-11-07 08:40 +0100
                Re: [GIT PULL] x86/mm changes for v4.4 Kees Cook <keescook@chromium.org> - 2015-11-08 08:00 +0100
                Re: [GIT PULL] x86/mm changes for v4.4 Ard Biesheuvel <ard.biesheuvel@linaro.org> - 2015-11-08 09:00 +0100
                Re: [GIT PULL] x86/mm changes for v4.4 Kees Cook <keescook@chromium.org> - 2015-11-09 22:10 +0100
                Re: [GIT PULL] x86/mm changes for v4.4 Ard Biesheuvel <ard.biesheuvel@linaro.org> - 2015-11-10 08:10 +0100
                Re: [GIT PULL] x86/mm changes for v4.4 Kees Cook <keescook@chromium.org> - 2015-11-10 21:20 +0100

csiph-web