Groups | Search | Server Info | Keyboard shortcuts | Login | Register [http] [https] [nntp] [nntps]
Groups > linux.kernel > #1261410 > unrolled thread
| Started by | Ingo Molnar <mingo@kernel.org> |
|---|---|
| First post | 2015-11-03 12:20 +0100 |
| Last post | 2015-11-10 21:20 +0100 |
| Articles | 6 on this page of 26 — 8 participants |
Back to article view | Back to linux.kernel
[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
Page 2 of 2 — ← Prev page 1 [2]
| From | Ard Biesheuvel <ard.biesheuvel@linaro.org> |
|---|---|
| Date | 2015-11-07 08:40 +0100 |
| Message-ID | <qs6gp-7Jo-3@gated-at.bofh.it> |
| In reply to | #1264745 |
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/
[toc] | [prev] | [next] | [standalone]
| From | Kees Cook <keescook@chromium.org> |
|---|---|
| Date | 2015-11-08 08:00 +0100 |
| Message-ID | <qss7g-57a-27@gated-at.bofh.it> |
| In reply to | #1264752 |
On Fri, Nov 6, 2015 at 11:39 PM, Ard Biesheuvel <ard.biesheuvel@linaro.org> wrote: > 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. I'm sad that UEFI was designed without even the most basic of memory protections in mind. UEFI _itself_ should be setting up protective page mappings. :( For a boot firmware, it seems to me that safe page table layout would be a top priority bug. The "reporting issues" page for TianoCore doesn't actually seem to link to the "Project Tracker": https://github.com/tianocore/tianocore.github.io/wiki/Reporting-Issues Does anyone know how to get this correctly reported so future UEFI releases don't suffer from this? -Kees > >>> > 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 -- Kees Cook Chrome OS Security -- 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/
[toc] | [prev] | [next] | [standalone]
| From | Ard Biesheuvel <ard.biesheuvel@linaro.org> |
|---|---|
| Date | 2015-11-08 09:00 +0100 |
| Message-ID | <qst3k-5HG-3@gated-at.bofh.it> |
| In reply to | #1264990 |
On 8 November 2015 at 07:58, Kees Cook <keescook@chromium.org> wrote: > On Fri, Nov 6, 2015 at 11:39 PM, Ard Biesheuvel > <ard.biesheuvel@linaro.org> wrote: >> 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. > > I'm sad that UEFI was designed without even the most basic of memory > protections in mind. UEFI _itself_ should be setting up protective > page mappings. :( > Well, the 4 KB alignment of sections was considered prohibitive at the time from code size pov. But this was a long time ago, obviously. > For a boot firmware, it seems to me that safe page table layout would > be a top priority bug. The "reporting issues" page for TianoCore > doesn't actually seem to link to the "Project Tracker": > https://github.com/tianocore/tianocore.github.io/wiki/Reporting-Issues > > Does anyone know how to get this correctly reported so future UEFI > releases don't suffer from this? > Ugh. Don't get me started on that topic. I have been working with the UEFI forum since July to get a fundamentally broken implementation of memory protections fixed. UEFI v2.5 defines a memory protection scheme that is based on splitting PE/COFF images into separate memory regions so that R-X and RW- permissions can be applied. Unfortunately, that broke every OS in existence (including Windows 8), since the OS is allowed to reorder memory regions when it lays out the virtual remapping of the UEFI regions, resulting in PE/COFF .data and .text potentially appearing out of order. The good news is that we fixed it for the upcoming release (v2.6). I can't disclose any specifics, though :-( -- Ard. >>>> > 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 > > > > -- > Kees Cook > Chrome OS Security -- 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/
[toc] | [prev] | [next] | [standalone]
| From | Kees Cook <keescook@chromium.org> |
|---|---|
| Date | 2015-11-09 22:10 +0100 |
| Message-ID | <qt1Rn-3jI-5@gated-at.bofh.it> |
| In reply to | #1265034 |
On Sat, Nov 7, 2015 at 11:55 PM, Ard Biesheuvel <ard.biesheuvel@linaro.org> wrote: > On 8 November 2015 at 07:58, Kees Cook <keescook@chromium.org> wrote: >> On Fri, Nov 6, 2015 at 11:39 PM, Ard Biesheuvel >> <ard.biesheuvel@linaro.org> wrote: >>> 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. >> >> I'm sad that UEFI was designed without even the most basic of memory >> protections in mind. UEFI _itself_ should be setting up protective >> page mappings. :( >> > > Well, the 4 KB alignment of sections was considered prohibitive at the > time from code size pov. But this was a long time ago, obviously. Heh, yeah, I'd expect max 4K padding to get code/data correctly aligned on a 2MB binary to not be an issue. :) > >> For a boot firmware, it seems to me that safe page table layout would >> be a top priority bug. The "reporting issues" page for TianoCore >> doesn't actually seem to link to the "Project Tracker": >> https://github.com/tianocore/tianocore.github.io/wiki/Reporting-Issues >> >> Does anyone know how to get this correctly reported so future UEFI >> releases don't suffer from this? >> > > Ugh. Don't get me started on that topic. I have been working with the > UEFI forum since July to get a fundamentally broken implementation of > memory protections fixed. UEFI v2.5 defines a memory protection scheme > that is based on splitting PE/COFF images into separate memory regions > so that R-X and RW- permissions can be applied. Unfortunately, that > broke every OS in existence (including Windows 8), since the OS is > allowed to reorder memory regions when it lays out the virtual > remapping of the UEFI regions, resulting in PE/COFF .data and .text > potentially appearing out of order. > > The good news is that we fixed it for the upcoming release (v2.6). I > can't disclose any specifics, though :-( As long as there's motion to getting it fixed, that makes me happy! :) Does 2.6 get rid of the (AIUI) 2MB limit too? -Kees > > -- > Ard. > > >>>>> > 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 >> >> >> >> -- >> Kees Cook >> Chrome OS Security -- Kees Cook Chrome OS Security -- 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/
[toc] | [prev] | [next] | [standalone]
| From | Ard Biesheuvel <ard.biesheuvel@linaro.org> |
|---|---|
| Date | 2015-11-10 08:10 +0100 |
| Message-ID | <qtbe1-1T5-3@gated-at.bofh.it> |
| In reply to | #1266005 |
On 9 November 2015 at 22:08, Kees Cook <keescook@chromium.org> wrote: > On Sat, Nov 7, 2015 at 11:55 PM, Ard Biesheuvel > <ard.biesheuvel@linaro.org> wrote: >> On 8 November 2015 at 07:58, Kees Cook <keescook@chromium.org> wrote: >>> On Fri, Nov 6, 2015 at 11:39 PM, Ard Biesheuvel >>> <ard.biesheuvel@linaro.org> wrote: >>>> 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. >>> >>> I'm sad that UEFI was designed without even the most basic of memory >>> protections in mind. UEFI _itself_ should be setting up protective >>> page mappings. :( >>> >> >> Well, the 4 KB alignment of sections was considered prohibitive at the >> time from code size pov. But this was a long time ago, obviously. > > Heh, yeah, I'd expect max 4K padding to get code/data correctly > aligned on a 2MB binary to not be an issue. :) > This is not about section sizes on ARM. The PE/COFF format does not use segments, like ELF, so the payload (the sections) needs to be completely disjoint from the header. This means, when using 4 KB alignment, that every PE/COFF image wastes ~4 KB in the header and 4 KB on average in the section padding (assuming a .text/.data/.reloc layout, as is common with PE/COFF) Considering that a typical UEFI firmware image consists of numerous (around 50 on average, I think) PE/COFF images, and some of them execute from NOR flash, the Tianocore tooling (which is the reference implementation) has always been geared towards keeping the alignment as small as possible, typically 32 bytes unless data objects need more. Since the UEFI runtime services are typically implemented by several of these PE/COFF images, and since the memory they occupy may be described by a single UEFI memory map entry, there is simply no easy way to decide which pages need R-X, RW- or RWX. Even looking for PE/COFF headers in the memory region is not guaranteed to work, since the PE/COFF header is part of the file format, not the memory format (i.e., since the header is disjoint from the payload, a PE/COFF loader is not required to copy the header to memory) >> >>> For a boot firmware, it seems to me that safe page table layout would >>> be a top priority bug. The "reporting issues" page for TianoCore >>> doesn't actually seem to link to the "Project Tracker": >>> https://github.com/tianocore/tianocore.github.io/wiki/Reporting-Issues >>> >>> Does anyone know how to get this correctly reported so future UEFI >>> releases don't suffer from this? >>> >> >> Ugh. Don't get me started on that topic. I have been working with the >> UEFI forum since July to get a fundamentally broken implementation of >> memory protections fixed. UEFI v2.5 defines a memory protection scheme >> that is based on splitting PE/COFF images into separate memory regions >> so that R-X and RW- permissions can be applied. Unfortunately, that >> broke every OS in existence (including Windows 8), since the OS is >> allowed to reorder memory regions when it lays out the virtual >> remapping of the UEFI regions, resulting in PE/COFF .data and .text >> potentially appearing out of order. >> >> The good news is that we fixed it for the upcoming release (v2.6). I >> can't disclose any specifics, though :-( > > As long as there's motion to getting it fixed, that makes me happy! :) > Does 2.6 get rid of the (AIUI) 2MB limit too? > No, there is no such limit in UEFI. If there is a limit like that, it is an implementation detail of the UEFI support in the OS. For arm64 (and the upcoming ARM support), the UEFI runtime services regions are remapped into a virtual userland range that is only active during the time runtime services are being invoked. (x86 does something similar, but it shares the page tables with the suspend/resume code afaiu) These mappings could be page granularity (since they don't require splitting PUDs or PMDs in the linear region), with the side note that arm64 mandates 64 KB alignment (to interoperate with 64 KB pages OSes). This requirement has been added to the UEFI spec, i.e., a v2.5 compliant arm64 firmware should not expose UEFI runtime regions that are not 64 KB aligned. -- Ard. -- 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/
[toc] | [prev] | [next] | [standalone]
| From | Kees Cook <keescook@chromium.org> |
|---|---|
| Date | 2015-11-10 21:20 +0100 |
| Message-ID | <qtnyy-1ks-7@gated-at.bofh.it> |
| In reply to | #1266284 |
On Mon, Nov 9, 2015 at 11:08 PM, Ard Biesheuvel <ard.biesheuvel@linaro.org> wrote: > On 9 November 2015 at 22:08, Kees Cook <keescook@chromium.org> wrote: >> On Sat, Nov 7, 2015 at 11:55 PM, Ard Biesheuvel >> <ard.biesheuvel@linaro.org> wrote: >>> On 8 November 2015 at 07:58, Kees Cook <keescook@chromium.org> wrote: >>>> On Fri, Nov 6, 2015 at 11:39 PM, Ard Biesheuvel >>>> <ard.biesheuvel@linaro.org> wrote: >>>>> 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. >>>> >>>> I'm sad that UEFI was designed without even the most basic of memory >>>> protections in mind. UEFI _itself_ should be setting up protective >>>> page mappings. :( >>>> >>> >>> Well, the 4 KB alignment of sections was considered prohibitive at the >>> time from code size pov. But this was a long time ago, obviously. >> >> Heh, yeah, I'd expect max 4K padding to get code/data correctly >> aligned on a 2MB binary to not be an issue. :) >> > > This is not about section sizes on ARM. The PE/COFF format does not > use segments, like ELF, so the payload (the sections) needs to be > completely disjoint from the header. This means, when using 4 KB > alignment, that every PE/COFF image wastes ~4 KB in the header and 4 > KB on average in the section padding (assuming a .text/.data/.reloc > layout, as is common with PE/COFF) > > Considering that a typical UEFI firmware image consists of numerous > (around 50 on average, I think) PE/COFF images, and some of them Oooh, that's no fun. So the linker can't produce merged .text and .data sections? > execute from NOR flash, the Tianocore tooling (which is the reference > implementation) has always been geared towards keeping the alignment > as small as possible, typically 32 bytes unless data objects need > more. Since the UEFI runtime services are typically implemented by > several of these PE/COFF images, and since the memory they occupy may > be described by a single UEFI memory map entry, there is simply no > easy way to decide which pages need R-X, RW- or RWX. Even looking for > PE/COFF headers in the memory region is not guaranteed to work, since > the PE/COFF header is part of the file format, not the memory format > (i.e., since the header is disjoint from the payload, a PE/COFF loader > is not required to copy the header to memory) > >>> >>>> For a boot firmware, it seems to me that safe page table layout would >>>> be a top priority bug. The "reporting issues" page for TianoCore >>>> doesn't actually seem to link to the "Project Tracker": >>>> https://github.com/tianocore/tianocore.github.io/wiki/Reporting-Issues >>>> >>>> Does anyone know how to get this correctly reported so future UEFI >>>> releases don't suffer from this? >>>> >>> >>> Ugh. Don't get me started on that topic. I have been working with the >>> UEFI forum since July to get a fundamentally broken implementation of >>> memory protections fixed. UEFI v2.5 defines a memory protection scheme >>> that is based on splitting PE/COFF images into separate memory regions >>> so that R-X and RW- permissions can be applied. Unfortunately, that >>> broke every OS in existence (including Windows 8), since the OS is >>> allowed to reorder memory regions when it lays out the virtual >>> remapping of the UEFI regions, resulting in PE/COFF .data and .text >>> potentially appearing out of order. >>> >>> The good news is that we fixed it for the upcoming release (v2.6). I >>> can't disclose any specifics, though :-( >> >> As long as there's motion to getting it fixed, that makes me happy! :) >> Does 2.6 get rid of the (AIUI) 2MB limit too? >> > > No, there is no such limit in UEFI. If there is a limit like that, it > is an implementation detail of the UEFI support in the OS. > > For arm64 (and the upcoming ARM support), the UEFI runtime services > regions are remapped into a virtual userland range that is only active > during the time runtime services are being invoked. (x86 does > something similar, but it shares the page tables with the > suspend/resume code afaiu) These mappings could be page granularity > (since they don't require splitting PUDs or PMDs in the linear > region), with the side note that arm64 mandates 64 KB alignment (to > interoperate with 64 KB pages OSes). This requirement has been added > to the UEFI spec, i.e., a v2.5 compliant arm64 firmware should not > expose UEFI runtime regions that are not 64 KB aligned. Cool, thanks for the details! -Kees -- Kees Cook Chrome OS Security -- 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/
[toc] | [prev] | [standalone]
Page 2 of 2 — ← Prev page 1 [2]
Back to top | Article view | linux.kernel
csiph-web