Re: [edk2-devel] [RFT PATCH v2 1/6] BaseTools/tools_def XCODE: Link X64 with -read_only_relocs suppress
> On 31. Mar 2023, at 16:58, Rebecca Cran wrote: > > On 3/31/23 2:29 AM, Marvin Häuser wrote: >> I agree if there’s an actual plan on doing that. We depend on XCODE5 >> downstream, but I think it would literally be easier for us if the upstream >> version was dropped than rebasing against hacks that our slightly modded >> variant does not require. >> >> Cc Andrew and Rebecca. I don’t know anyone else who might still be using >> XCODE5. Any objections to dropping it? If so, any plans to pick up my >> proposed changes instead? > > I've only been using XCODE5 to test if it still works. I do all of my > development work on Linux or FreeBSD. I checked the list and most of the traffic regarding XCODE5 is basically Andrew and you. So if you don't need this toolchain... @Andrew Would you agree it's fair for Apple to maintain XCODE5 and its oddities downstream? I know someone mentioned the ARM transition, but I think you still support UEFI for HV things, don't you? The alternative would be we revamp the upstream XCODE5 toolchain and mtoc with our changes from AUDK and ocmtoc, but this isn't feasible without strong support from Apple (our previous patches to mtoc were ignored). I don't think the burden easily fixable XCODE5 oddities put on the general codebase are acceptable going forward. Thanks! Best regards, Marvin > > > -- > > Rebecca Cran > > -=-=-=-=-=-=-=-=-=-=-=- Groups.io Links: You receive all messages sent to this group. View/Reply Online (#102320): https://edk2.groups.io/g/devel/message/102320 Mute This Topic: https://groups.io/mt/97960758/21656 Group Owner: devel+ow...@edk2.groups.io Unsubscribe: https://edk2.groups.io/g/devel/unsub [arch...@mail-archive.com] -=-=-=-=-=-=-=-=-=-=-=-
Re: [edk2-devel] [RFT PATCH v2 1/6] BaseTools/tools_def XCODE: Link X64 with -read_only_relocs suppress
On 3/31/23 2:29 AM, Marvin Häuser wrote: I agree if there’s an actual plan on doing that. We depend on XCODE5 downstream, but I think it would literally be easier for us if the upstream version was dropped than rebasing against hacks that our slightly modded variant does not require. Cc Andrew and Rebecca. I don’t know anyone else who might still be using XCODE5. Any objections to dropping it? If so, any plans to pick up my proposed changes instead? I've only been using XCODE5 to test if it still works. I do all of my development work on Linux or FreeBSD. -- Rebecca Cran -=-=-=-=-=-=-=-=-=-=-=- Groups.io Links: You receive all messages sent to this group. View/Reply Online (#102317): https://edk2.groups.io/g/devel/message/102317 Mute This Topic: https://groups.io/mt/97960758/21656 Group Owner: devel+ow...@edk2.groups.io Unsubscribe: https://edk2.groups.io/g/devel/unsub [arch...@mail-archive.com] -=-=-=-=-=-=-=-=-=-=-=-
Re: [edk2-devel] [RFT PATCH v2 1/6] BaseTools/tools_def XCODE: Link X64 with -read_only_relocs suppress
Hi Gerd, > On 31. Mar 2023, at 12:53, Gerd Hoffmann wrote: > > Hi, > >>> However, those issues might have been fixed and it’s not impossible >>> Vitaly will give it another try eventually. In any case, I think our >>> downstream variant of XCODE5 doesn’t require any level of special >>> care, so it doesn’t really matter to us. >>> >>> (Another thing to consider is despite the bugs are fixed, mtoc has a >>> much higher overall code quality and more safety checks than GenFw, >>> which is used for CLANGDWARF.) >>> >>> The upstream toolchain has no future in my opinion, as mtoc has been >>> deprecated and already failed to compile certain things (like it >>> lacked Standalone MM types). The reason it still “worked” was >>> because homebrew silently shipped a variant with a subset of our >>> ocmtoc patches. So as I see it, taking our changes or dropping it >>> entirely are the only sane options, even regardless of this >>> particular issue you’re trying to fix. Personally, I have no >>> preference. >> >> I think both GenFw and mtoc are horrible hacks that should be phased >> out once we can - with good cross-architecture Clang support for >> native PE binaries, I'd hope macOS could move to CLANGPDB for all >> targets. > > What is the difference between CLANGPDB and CLANGDWARF? Just the debug > info format? DWARF generates an ELF (which is utilized by UefiPayloadPkg) and PDB generates a PE directly. Best regards, Marvin > > What is the support status? Is CLANGDWARF expected to build edk2 on all > platforms? Including cross-builds? Or will that work only after > Rebecca's toolchain fix/cleanup series being merged? > > Should we eventually switch from gcc to clang on linux too? > > take care, > Gerd > -=-=-=-=-=-=-=-=-=-=-=- Groups.io Links: You receive all messages sent to this group. View/Reply Online (#102298): https://edk2.groups.io/g/devel/message/102298 Mute This Topic: https://groups.io/mt/97960758/21656 Group Owner: devel+ow...@edk2.groups.io Unsubscribe: https://edk2.groups.io/g/devel/unsub [arch...@mail-archive.com] -=-=-=-=-=-=-=-=-=-=-=-
Re: [edk2-devel] [RFT PATCH v2 1/6] BaseTools/tools_def XCODE: Link X64 with -read_only_relocs suppress
On Fri, 31 Mar 2023 at 12:53, Gerd Hoffmann wrote: > > Hi, > > > > However, those issues might have been fixed and it’s not impossible > > > Vitaly will give it another try eventually. In any case, I think our > > > downstream variant of XCODE5 doesn’t require any level of special > > > care, so it doesn’t really matter to us. > > > > > > (Another thing to consider is despite the bugs are fixed, mtoc has a > > > much higher overall code quality and more safety checks than GenFw, > > > which is used for CLANGDWARF.) > > > > > > The upstream toolchain has no future in my opinion, as mtoc has been > > > deprecated and already failed to compile certain things (like it > > > lacked Standalone MM types). The reason it still “worked” was > > > because homebrew silently shipped a variant with a subset of our > > > ocmtoc patches. So as I see it, taking our changes or dropping it > > > entirely are the only sane options, even regardless of this > > > particular issue you’re trying to fix. Personally, I have no > > > preference. > > > > I think both GenFw and mtoc are horrible hacks that should be phased > > out once we can - with good cross-architecture Clang support for > > native PE binaries, I'd hope macOS could move to CLANGPDB for all > > targets. > > What is the difference between CLANGPDB and CLANGDWARF? Just the debug > info format? > No, it uses the LLVM tools to generate PE binaries directly. > What is the support status? Is CLANGDWARF expected to build edk2 on all > platforms? Including cross-builds? Or will that work only after > Rebecca's toolchain fix/cleanup series being merged? > Yes, that what I was hoping for - LLD supports all architectures, which is why I insisted that CLANGDWARF should use LLD on ARM/AARCH64 as well. That way, anyone can build all targets on any host. > Should we eventually switch from gcc to clang on linux too? > When using ELF to PE/COFF conversion, it doesn't make that much of a difference. -=-=-=-=-=-=-=-=-=-=-=- Groups.io Links: You receive all messages sent to this group. View/Reply Online (#102297): https://edk2.groups.io/g/devel/message/102297 Mute This Topic: https://groups.io/mt/97960758/21656 Group Owner: devel+ow...@edk2.groups.io Unsubscribe: https://edk2.groups.io/g/devel/unsub [arch...@mail-archive.com] -=-=-=-=-=-=-=-=-=-=-=-
Re: [edk2-devel] [RFT PATCH v2 1/6] BaseTools/tools_def XCODE: Link X64 with -read_only_relocs suppress
Hi, > > However, those issues might have been fixed and it’s not impossible > > Vitaly will give it another try eventually. In any case, I think our > > downstream variant of XCODE5 doesn’t require any level of special > > care, so it doesn’t really matter to us. > > > > (Another thing to consider is despite the bugs are fixed, mtoc has a > > much higher overall code quality and more safety checks than GenFw, > > which is used for CLANGDWARF.) > > > > The upstream toolchain has no future in my opinion, as mtoc has been > > deprecated and already failed to compile certain things (like it > > lacked Standalone MM types). The reason it still “worked” was > > because homebrew silently shipped a variant with a subset of our > > ocmtoc patches. So as I see it, taking our changes or dropping it > > entirely are the only sane options, even regardless of this > > particular issue you’re trying to fix. Personally, I have no > > preference. > > I think both GenFw and mtoc are horrible hacks that should be phased > out once we can - with good cross-architecture Clang support for > native PE binaries, I'd hope macOS could move to CLANGPDB for all > targets. What is the difference between CLANGPDB and CLANGDWARF? Just the debug info format? What is the support status? Is CLANGDWARF expected to build edk2 on all platforms? Including cross-builds? Or will that work only after Rebecca's toolchain fix/cleanup series being merged? Should we eventually switch from gcc to clang on linux too? take care, Gerd -=-=-=-=-=-=-=-=-=-=-=- Groups.io Links: You receive all messages sent to this group. View/Reply Online (#102295): https://edk2.groups.io/g/devel/message/102295 Mute This Topic: https://groups.io/mt/97960758/21656 Group Owner: devel+ow...@edk2.groups.io Unsubscribe: https://edk2.groups.io/g/devel/unsub [arch...@mail-archive.com] -=-=-=-=-=-=-=-=-=-=-=-
Re: [edk2-devel] [RFT PATCH v2 1/6] BaseTools/tools_def XCODE: Link X64 with -read_only_relocs suppress
> On 31. Mar 2023, at 11:36, Ard Biesheuvel wrote: > > On Fri, 31 Mar 2023 at 11:27, Marvin Häuser wrote: >> >> On 31. Mar 2023, at 10:59, Ard Biesheuvel wrote: >>> >>> On Fri, 31 Mar 2023 at 10:29, Marvin Häuser wrote: >> On 31. Mar 2023, at 09:39, Ard Biesheuvel wrote: > Hi Marvin, > > Thanks for the context. > > > On Thu, 30 Mar 2023 at 23:54, Marvin Häuser wrote: >> >> Hi Ard, >> >> Sorry, I cannot preserve the CC list as the groups.io interface doesn't >> seem to allow it. Can you please CC me on future revisions? >> >> This patch will badly corrupt binaries. I cannot cite a source right now >> (if you want me to, please remind me in your response, so I can look it >> up tomorrow), but for X64 (but not IA32, which is why this is enabled >> there), relocs are relative to the first *writable* segment. In other >> words, any relocation to __TEXT will badly corrupt binaries this way. > > OMG. > > I can't believe how buggy all this stuff is. But I can confirm that > the resulting binaries don't look right, even though they appear to > boot fine. Codegen does not change from the suppress flag, so there will be no additional text relocs beyond those you introduced. As they target the exception handler, I guess you’d need to actively provoke the broken code paths (and may end up with a nice recursion :) ). >>> >>> I understand that the codegen is the same. I was specifically talking >>> about the PE relocations, which seem to be lacking entirely. >> >> Sure, I was just elaborating on the “appear to boot fine” part, which does >> make sense. When I last tried, the relocs were not absent but underflown. >> Might be mtoc drops them somehow, I think I inspected the Mach-O directly. >> Whatever, you reproduce the issue. :) >> > > Fair enough. > >>> > In particular, when I dump the PE relocations using > llvm-readobj --coff-basereloc, I don't see any relocations referring > to the .text section. > >> In AUDK, we support this with two essential changes. The first is that >> we always generate a writable dummy segment at the beginning of the >> address space [1], making the relocs relative to the image base. The >> second is that in ocmtoc, our fork of the abandoned (and pretty >> badly-bugged) Apple mtoc, we explicitly require this segment to be >> present and verify its virtual address is the minimum virtual address >> [2]. It is then omitted from the conversion process [3]. I suggest you >> replicate these changes and fully switch to ocmtoc for XCODE5 builds. > > I'm not going to do any of that. Instead, I am going to drop this > change, and do the following: > > - modify the SecPei version of CpuExceptionHandlerLib to put the > vector templates in .data, as I proposed before. This works around the > issue, and given that SEC/PEI is assumed to be read-only anyway (as it > may execute in place from flash) and does not use page alignment for > the sections due to size constraints, it is reasonable to assume that > .text and .data will be mapped executable anyway. Well, that assumption is more than fair to make for the status quo platforms, but this is just another rock in the way of doing things the right way (even if it’s just VMs). >>> >>> How so? SEC and PEI could be mapped read-only today, it's just that we >>> never bother. >> >> I’m not concerned about read-only but NX. >> > > We don't have writable data in SEC or PEI, so this would require SEC, > PEI_CORE and every PEIM to have split .text and .rodata, and round > them up to page size. Not sure this is worth it, especially given the > fact that CoCo targets seems to be skipping the PEI phase entirely. CoCo = Confidential Computing? Right, I actually hope that’s true. :) But there are also some plans for real hardware here. > >>> Cc Gerd for an OVMF security perspective. Is PEI-time memory protection something you’d be interested in in the future? >>> >>> My WXN series for ARM maps all PEIMs read-only, and turns off >>> shadowing entirely (which makes no sense for VMs). So we have most of >>> what we need to do that, and this change has no bearing on that. >> >> Well yes, if everything is read-only, you guarantee W^X implicitly, but >> downstream we have plans for the full deal including NX data. It’s however >> shelved for the distant future, so as long as this is changed with the >> intention of reverting it once XCODE5 is fixed or dropped, that sounds fine >> to me. I just don’t like the notion of intentionally breaking the memory >> permission model as a hack. I rather hope we’ll make some swift progress on >> removing XCODE5 as a source of frustration. :) >> > > Pardon my bluntness, but why should I care about the
Re: [edk2-devel] [RFT PATCH v2 1/6] BaseTools/tools_def XCODE: Link X64 with -read_only_relocs suppress
On Fri, 31 Mar 2023 at 11:27, Marvin Häuser wrote: > > > > On 31. Mar 2023, at 10:59, Ard Biesheuvel wrote: > > > > On Fri, 31 Mar 2023 at 10:29, Marvin Häuser wrote: > >> > >> > On 31. Mar 2023, at 09:39, Ard Biesheuvel wrote: > >>> Hi Marvin, > >>> > >>> Thanks for the context. > >>> > >>> > >>> On Thu, 30 Mar 2023 at 23:54, Marvin Häuser wrote: > > Hi Ard, > > Sorry, I cannot preserve the CC list as the groups.io interface doesn't > seem to allow it. Can you please CC me on future revisions? > > This patch will badly corrupt binaries. I cannot cite a source right now > (if you want me to, please remind me in your response, so I can look it > up tomorrow), but for X64 (but not IA32, which is why this is enabled > there), relocs are relative to the first *writable* segment. In other > words, any relocation to __TEXT will badly corrupt binaries this way. > >>> > >>> OMG. > >>> > >>> I can't believe how buggy all this stuff is. But I can confirm that > >>> the resulting binaries don't look right, even though they appear to > >>> boot fine. > >> > >> Codegen does not change from the suppress flag, so there will be no > >> additional text relocs beyond those you introduced. As they target the > >> exception handler, I guess you’d need to actively provoke the broken code > >> paths (and may end up with a nice recursion :) ). > >> > > > > I understand that the codegen is the same. I was specifically talking > > about the PE relocations, which seem to be lacking entirely. > > Sure, I was just elaborating on the “appear to boot fine” part, which does > make sense. When I last tried, the relocs were not absent but underflown. > Might be mtoc drops them somehow, I think I inspected the Mach-O directly. > Whatever, you reproduce the issue. :) > Fair enough. > > > >>> In particular, when I dump the PE relocations using > >>> llvm-readobj --coff-basereloc, I don't see any relocations referring > >>> to the .text section. > >>> > In AUDK, we support this with two essential changes. The first is that > we always generate a writable dummy segment at the beginning of the > address space [1], making the relocs relative to the image base. The > second is that in ocmtoc, our fork of the abandoned (and pretty > badly-bugged) Apple mtoc, we explicitly require this segment to be > present and verify its virtual address is the minimum virtual address > [2]. It is then omitted from the conversion process [3]. I suggest you > replicate these changes and fully switch to ocmtoc for XCODE5 builds. > >>> > >>> I'm not going to do any of that. Instead, I am going to drop this > >>> change, and do the following: > >>> > >>> - modify the SecPei version of CpuExceptionHandlerLib to put the > >>> vector templates in .data, as I proposed before. This works around the > >>> issue, and given that SEC/PEI is assumed to be read-only anyway (as it > >>> may execute in place from flash) and does not use page alignment for > >>> the sections due to size constraints, it is reasonable to assume that > >>> .text and .data will be mapped executable anyway. > >> > >> Well, that assumption is more than fair to make for the status quo > >> platforms, but this is just another rock in the way of doing things the > >> right way (even if it’s just VMs). > >> > > > > How so? SEC and PEI could be mapped read-only today, it's just that we > > never bother. > > I’m not concerned about read-only but NX. > We don't have writable data in SEC or PEI, so this would require SEC, PEI_CORE and every PEIM to have split .text and .rodata, and round them up to page size. Not sure this is worth it, especially given the fact that CoCo targets seems to be skipping the PEI phase entirely. > > > >> Cc Gerd for an OVMF security perspective. Is PEI-time memory protection > >> something you’d be interested in in the future? > >> > > > > My WXN series for ARM maps all PEIMs read-only, and turns off > > shadowing entirely (which makes no sense for VMs). So we have most of > > what we need to do that, and this change has no bearing on that. > > Well yes, if everything is read-only, you guarantee W^X implicitly, but > downstream we have plans for the full deal including NX data. It’s however > shelved for the distant future, so as long as this is changed with the > intention of reverting it once XCODE5 is fixed or dropped, that sounds fine > to me. I just don’t like the notion of intentionally breaking the memory > permission model as a hack. I rather hope we’ll make some swift progress on > removing XCODE5 as a source of frustration. :) > Pardon my bluntness, but why should I care about the shelved future plans of some downstream project? > > > >>> > >>> - update the version that performs the runtime fixups to only do so > >>> when using the XCODE toolchain - we can phase that out once we drop > >>> XCODE support. > >> > >> I agree if there’s an
Re: [edk2-devel] [RFT PATCH v2 1/6] BaseTools/tools_def XCODE: Link X64 with -read_only_relocs suppress
> On 31. Mar 2023, at 10:59, Ard Biesheuvel wrote: > > On Fri, 31 Mar 2023 at 10:29, Marvin Häuser wrote: >> >> On 31. Mar 2023, at 09:39, Ard Biesheuvel wrote: >>> Hi Marvin, >>> >>> Thanks for the context. >>> >>> >>> On Thu, 30 Mar 2023 at 23:54, Marvin Häuser wrote: Hi Ard, Sorry, I cannot preserve the CC list as the groups.io interface doesn't seem to allow it. Can you please CC me on future revisions? This patch will badly corrupt binaries. I cannot cite a source right now (if you want me to, please remind me in your response, so I can look it up tomorrow), but for X64 (but not IA32, which is why this is enabled there), relocs are relative to the first *writable* segment. In other words, any relocation to __TEXT will badly corrupt binaries this way. >>> >>> OMG. >>> >>> I can't believe how buggy all this stuff is. But I can confirm that >>> the resulting binaries don't look right, even though they appear to >>> boot fine. >> >> Codegen does not change from the suppress flag, so there will be no >> additional text relocs beyond those you introduced. As they target the >> exception handler, I guess you’d need to actively provoke the broken code >> paths (and may end up with a nice recursion :) ). >> > > I understand that the codegen is the same. I was specifically talking > about the PE relocations, which seem to be lacking entirely. Sure, I was just elaborating on the “appear to boot fine” part, which does make sense. When I last tried, the relocs were not absent but underflown. Might be mtoc drops them somehow, I think I inspected the Mach-O directly. Whatever, you reproduce the issue. :) > >>> In particular, when I dump the PE relocations using >>> llvm-readobj --coff-basereloc, I don't see any relocations referring >>> to the .text section. >>> In AUDK, we support this with two essential changes. The first is that we always generate a writable dummy segment at the beginning of the address space [1], making the relocs relative to the image base. The second is that in ocmtoc, our fork of the abandoned (and pretty badly-bugged) Apple mtoc, we explicitly require this segment to be present and verify its virtual address is the minimum virtual address [2]. It is then omitted from the conversion process [3]. I suggest you replicate these changes and fully switch to ocmtoc for XCODE5 builds. >>> >>> I'm not going to do any of that. Instead, I am going to drop this >>> change, and do the following: >>> >>> - modify the SecPei version of CpuExceptionHandlerLib to put the >>> vector templates in .data, as I proposed before. This works around the >>> issue, and given that SEC/PEI is assumed to be read-only anyway (as it >>> may execute in place from flash) and does not use page alignment for >>> the sections due to size constraints, it is reasonable to assume that >>> .text and .data will be mapped executable anyway. >> >> Well, that assumption is more than fair to make for the status quo >> platforms, but this is just another rock in the way of doing things the >> right way (even if it’s just VMs). >> > > How so? SEC and PEI could be mapped read-only today, it's just that we > never bother. I’m not concerned about read-only but NX. > >> Cc Gerd for an OVMF security perspective. Is PEI-time memory protection >> something you’d be interested in in the future? >> > > My WXN series for ARM maps all PEIMs read-only, and turns off > shadowing entirely (which makes no sense for VMs). So we have most of > what we need to do that, and this change has no bearing on that. Well yes, if everything is read-only, you guarantee W^X implicitly, but downstream we have plans for the full deal including NX data. It’s however shelved for the distant future, so as long as this is changed with the intention of reverting it once XCODE5 is fixed or dropped, that sounds fine to me. I just don’t like the notion of intentionally breaking the memory permission model as a hack. I rather hope we’ll make some swift progress on removing XCODE5 as a source of frustration. :) > >>> >>> - update the version that performs the runtime fixups to only do so >>> when using the XCODE toolchain - we can phase that out once we drop >>> XCODE support. >> >> I agree if there’s an actual plan on doing that. We depend on XCODE5 >> downstream, but I think it would literally be easier for us if the upstream >> version was dropped than rebasing against hacks that our slightly modded >> variant does not require. >> >> Cc Andrew and Rebecca. I don’t know anyone else who might still be using >> XCODE5. Any objections to dropping it? If so, any plans to pick up my >> proposed changes instead? >> > > I wouldn't mind dropping it. In fact, I'm wondering - given that you > need to install nasm and iasl anyway - if it wouldn't make more sense > to use the CLANGPDB toolchain on macOS, and avoid t
Re: [edk2-devel] [RFT PATCH v2 1/6] BaseTools/tools_def XCODE: Link X64 with -read_only_relocs suppress
Hi, > > - modify the SecPei version of CpuExceptionHandlerLib to put the > > vector templates in .data, as I proposed before. This works around the > > issue, and given that SEC/PEI is assumed to be read-only anyway (as it > > may execute in place from flash) and does not use page alignment for > > the sections due to size constraints, it is reasonable to assume that > > .text and .data will be mapped executable anyway. > > Well, that assumption is more than fair to make for the status quo > platforms, but this is just another rock in the way of doing things > the right way (even if it’s just VMs). > > Cc Gerd for an OVMF security perspective. Is PEI-time memory > protection something you’d be interested in in the future? Given that PEI is expected to be able to run from read-only storage the easiest way to apply X^W rules would be to just map the whole PEI firmware volume as R-X when executing from RAM (which is the case for OVMF). I've fixed OVMF PEI modules last year to *not* use global variables, so OVMF is not a special case any more and mapping OVMF PEI readonly should work just fine. So Ard's approach looks sane to me. take care, Gerd -=-=-=-=-=-=-=-=-=-=-=- Groups.io Links: You receive all messages sent to this group. View/Reply Online (#102261): https://edk2.groups.io/g/devel/message/102261 Mute This Topic: https://groups.io/mt/97960758/21656 Group Owner: devel+ow...@edk2.groups.io Unsubscribe: https://edk2.groups.io/g/devel/unsub [arch...@mail-archive.com] -=-=-=-=-=-=-=-=-=-=-=-
Re: [edk2-devel] [RFT PATCH v2 1/6] BaseTools/tools_def XCODE: Link X64 with -read_only_relocs suppress
On Fri, 31 Mar 2023 at 10:29, Marvin Häuser wrote: > > > > On 31. Mar 2023, at 09:39, Ard Biesheuvel wrote: > > Hi Marvin, > > > > Thanks for the context. > > > > > > On Thu, 30 Mar 2023 at 23:54, Marvin Häuser wrote: > >> > >> Hi Ard, > >> > >> Sorry, I cannot preserve the CC list as the groups.io interface doesn't > >> seem to allow it. Can you please CC me on future revisions? > >> > >> This patch will badly corrupt binaries. I cannot cite a source right now > >> (if you want me to, please remind me in your response, so I can look it up > >> tomorrow), but for X64 (but not IA32, which is why this is enabled there), > >> relocs are relative to the first *writable* segment. In other words, any > >> relocation to __TEXT will badly corrupt binaries this way. > > > > OMG. > > > > I can't believe how buggy all this stuff is. But I can confirm that > > the resulting binaries don't look right, even though they appear to > > boot fine. > > Codegen does not change from the suppress flag, so there will be no > additional text relocs beyond those you introduced. As they target the > exception handler, I guess you’d need to actively provoke the broken code > paths (and may end up with a nice recursion :) ). > I understand that the codegen is the same. I was specifically talking about the PE relocations, which seem to be lacking entirely. > > In particular, when I dump the PE relocations using > > llvm-readobj --coff-basereloc, I don't see any relocations referring > > to the .text section. > > > >> In AUDK, we support this with two essential changes. The first is that we > >> always generate a writable dummy segment at the beginning of the address > >> space [1], making the relocs relative to the image base. The second is > >> that in ocmtoc, our fork of the abandoned (and pretty badly-bugged) Apple > >> mtoc, we explicitly require this segment to be present and verify its > >> virtual address is the minimum virtual address [2]. It is then omitted > >> from the conversion process [3]. I suggest you replicate these changes and > >> fully switch to ocmtoc for XCODE5 builds. > > > > I'm not going to do any of that. Instead, I am going to drop this > > change, and do the following: > > > > - modify the SecPei version of CpuExceptionHandlerLib to put the > > vector templates in .data, as I proposed before. This works around the > > issue, and given that SEC/PEI is assumed to be read-only anyway (as it > > may execute in place from flash) and does not use page alignment for > > the sections due to size constraints, it is reasonable to assume that > > .text and .data will be mapped executable anyway. > > Well, that assumption is more than fair to make for the status quo platforms, > but this is just another rock in the way of doing things the right way (even > if it’s just VMs). > How so? SEC and PEI could be mapped read-only today, it's just that we never bother. > Cc Gerd for an OVMF security perspective. Is PEI-time memory protection > something you’d be interested in in the future? > My WXN series for ARM maps all PEIMs read-only, and turns off shadowing entirely (which makes no sense for VMs). So we have most of what we need to do that, and this change has no bearing on that. > > > > - update the version that performs the runtime fixups to only do so > > when using the XCODE toolchain - we can phase that out once we drop > > XCODE support. > > I agree if there’s an actual plan on doing that. We depend on XCODE5 > downstream, but I think it would literally be easier for us if the upstream > version was dropped than rebasing against hacks that our slightly modded > variant does not require. > > Cc Andrew and Rebecca. I don’t know anyone else who might still be using > XCODE5. Any objections to dropping it? If so, any plans to pick up my > proposed changes instead? > I wouldn't mind dropping it. In fact, I'm wondering - given that you need to install nasm and iasl anyway - if it wouldn't make more sense to use the CLANGPDB toolchain on macOS, and avoid the mtoc mess entirely? -=-=-=-=-=-=-=-=-=-=-=- Groups.io Links: You receive all messages sent to this group. View/Reply Online (#102254): https://edk2.groups.io/g/devel/message/102254 Mute This Topic: https://groups.io/mt/97960758/21656 Group Owner: devel+ow...@edk2.groups.io Unsubscribe: https://edk2.groups.io/g/devel/unsub [arch...@mail-archive.com] -=-=-=-=-=-=-=-=-=-=-=-
Re: [edk2-devel] [RFT PATCH v2 1/6] BaseTools/tools_def XCODE: Link X64 with -read_only_relocs suppress
> On 31. Mar 2023, at 09:39, Ard Biesheuvel wrote: > Hi Marvin, > > Thanks for the context. > > > On Thu, 30 Mar 2023 at 23:54, Marvin Häuser wrote: >> >> Hi Ard, >> >> Sorry, I cannot preserve the CC list as the groups.io interface doesn't seem >> to allow it. Can you please CC me on future revisions? >> >> This patch will badly corrupt binaries. I cannot cite a source right now (if >> you want me to, please remind me in your response, so I can look it up >> tomorrow), but for X64 (but not IA32, which is why this is enabled there), >> relocs are relative to the first *writable* segment. In other words, any >> relocation to __TEXT will badly corrupt binaries this way. > > OMG. > > I can't believe how buggy all this stuff is. But I can confirm that > the resulting binaries don't look right, even though they appear to > boot fine. Codegen does not change from the suppress flag, so there will be no additional text relocs beyond those you introduced. As they target the exception handler, I guess you’d need to actively provoke the broken code paths (and may end up with a nice recursion :) ). > In particular, when I dump the PE relocations using > llvm-readobj --coff-basereloc, I don't see any relocations referring > to the .text section. > >> In AUDK, we support this with two essential changes. The first is that we >> always generate a writable dummy segment at the beginning of the address >> space [1], making the relocs relative to the image base. The second is that >> in ocmtoc, our fork of the abandoned (and pretty badly-bugged) Apple mtoc, >> we explicitly require this segment to be present and verify its virtual >> address is the minimum virtual address [2]. It is then omitted from the >> conversion process [3]. I suggest you replicate these changes and fully >> switch to ocmtoc for XCODE5 builds. > > I'm not going to do any of that. Instead, I am going to drop this > change, and do the following: > > - modify the SecPei version of CpuExceptionHandlerLib to put the > vector templates in .data, as I proposed before. This works around the > issue, and given that SEC/PEI is assumed to be read-only anyway (as it > may execute in place from flash) and does not use page alignment for > the sections due to size constraints, it is reasonable to assume that > .text and .data will be mapped executable anyway. Well, that assumption is more than fair to make for the status quo platforms, but this is just another rock in the way of doing things the right way (even if it’s just VMs). Cc Gerd for an OVMF security perspective. Is PEI-time memory protection something you’d be interested in in the future? > > - update the version that performs the runtime fixups to only do so > when using the XCODE toolchain - we can phase that out once we drop > XCODE support. I agree if there’s an actual plan on doing that. We depend on XCODE5 downstream, but I think it would literally be easier for us if the upstream version was dropped than rebasing against hacks that our slightly modded variant does not require. Cc Andrew and Rebecca. I don’t know anyone else who might still be using XCODE5. Any objections to dropping it? If so, any plans to pick up my proposed changes instead? Best regards, Marvin -=-=-=-=-=-=-=-=-=-=-=- Groups.io Links: You receive all messages sent to this group. View/Reply Online (#102252): https://edk2.groups.io/g/devel/message/102252 Mute This Topic: https://groups.io/mt/97960758/21656 Group Owner: devel+ow...@edk2.groups.io Unsubscribe: https://edk2.groups.io/g/devel/unsub [arch...@mail-archive.com] -=-=-=-=-=-=-=-=-=-=-=-
Re: [edk2-devel] [RFT PATCH v2 1/6] BaseTools/tools_def XCODE: Link X64 with -read_only_relocs suppress
Hi Marvin, Thanks for the context. On Thu, 30 Mar 2023 at 23:54, Marvin Häuser wrote: > > Hi Ard, > > Sorry, I cannot preserve the CC list as the groups.io interface doesn't seem > to allow it. Can you please CC me on future revisions? > > This patch will badly corrupt binaries. I cannot cite a source right now (if > you want me to, please remind me in your response, so I can look it up > tomorrow), but for X64 (but not IA32, which is why this is enabled there), > relocs are relative to the first *writable* segment. In other words, any > relocation to __TEXT will badly corrupt binaries this way. > OMG. I can't believe how buggy all this stuff is. But I can confirm that the resulting binaries don't look right, even though they appear to boot fine. In particular, when I dump the PE relocations using llvm-readobj --coff-basereloc, I don't see any relocations referring to the .text section. > In AUDK, we support this with two essential changes. The first is that we > always generate a writable dummy segment at the beginning of the address > space [1], making the relocs relative to the image base. The second is that > in ocmtoc, our fork of the abandoned (and pretty badly-bugged) Apple mtoc, we > explicitly require this segment to be present and verify its virtual address > is the minimum virtual address [2]. It is then omitted from the conversion > process [3]. I suggest you replicate these changes and fully switch to ocmtoc > for XCODE5 builds. > I'm not going to do any of that. Instead, I am going to drop this change, and do the following: - modify the SecPei version of CpuExceptionHandlerLib to put the vector templates in .data, as I proposed before. This works around the issue, and given that SEC/PEI is assumed to be read-only anyway (as it may execute in place from flash) and does not use page alignment for the sections due to size constraints, it is reasonable to assume that .text and .data will be mapped executable anyway. - update the version that performs the runtime fixups to only do so when using the XCODE toolchain - we can phase that out once we drop XCODE support. -=-=-=-=-=-=-=-=-=-=-=- Groups.io Links: You receive all messages sent to this group. View/Reply Online (#102244): https://edk2.groups.io/g/devel/message/102244 Mute This Topic: https://groups.io/mt/97960758/21656 Group Owner: devel+ow...@edk2.groups.io Unsubscribe: https://edk2.groups.io/g/devel/unsub [arch...@mail-archive.com] -=-=-=-=-=-=-=-=-=-=-=-
Re: [edk2-devel] [RFT PATCH v2 1/6] BaseTools/tools_def XCODE: Link X64 with -read_only_relocs suppress
Hi Ard, Sorry, I cannot preserve the CC list as the groups.io interface doesn't seem to allow it. Can you please CC me on future revisions? This patch will badly corrupt binaries. I cannot cite a source right now (if you want me to, please remind me in your response, so I can look it up tomorrow), but for X64 (but not IA32, which is why this is enabled there), relocs are relative to the first *writable* segment. In other words, any relocation to __TEXT will badly corrupt binaries this way. In AUDK, we support this with two essential changes. The first is that we always generate a writable dummy segment at the beginning of the address space [1], making the relocs relative to the image base. The second is that in ocmtoc, our fork of the abandoned (and pretty badly-bugged) Apple mtoc, we explicitly require this segment to be present and verify its virtual address is the minimum virtual address [2]. It is then omitted from the conversion process [3]. I suggest you replicate these changes and fully switch to ocmtoc for XCODE5 builds. Best regards, Marvin [1] https://github.com/acidanthera/audk/blob/c382e9c571c7d5f39ba53b46a0c723c7943f33c5/BaseTools/Conf/tools_def.template#L2976-L2988 [2] https://github.com/acidanthera/ocmtoc/blob/b0152c51beae264770c3faf0d213f9594ee043be/efitools/mtoc.c#L1097-L1123 https://github.com/acidanthera/ocmtoc/blob/b0152c51beae264770c3faf0d213f9594ee043be/efitools/mtoc.c#L1204-L1214 [3] https://github.com/acidanthera/ocmtoc/blob/b0152c51beae264770c3faf0d213f9594ee043be/efitools/mtoc.c#L1307-L1311 -=-=-=-=-=-=-=-=-=-=-=- Groups.io Links: You receive all messages sent to this group. View/Reply Online (#102219): https://edk2.groups.io/g/devel/message/102219 Mute This Topic: https://groups.io/mt/97960758/21656 Group Owner: devel+ow...@edk2.groups.io Unsubscribe: https://edk2.groups.io/g/devel/unsub [arch...@mail-archive.com] -=-=-=-=-=-=-=-=-=-=-=-
[edk2-devel] [RFT PATCH v2 1/6] BaseTools/tools_def XCODE: Link X64 with -read_only_relocs suppress
Earlier XCODE versions did not support the -read_only_relocs suppress linker option, which suppresses errors resulting from absolute relocations emitted into read-only sections when building PIE executables. This requires a rather messy workaround in the CPU exception handler libraries, to permit absolute relocations in code that may get copied from a template in some cases. Fortunately, this seems to be permitted now, so add the option for X64 as well (it was already present for IA32). This will allows us to simplify the CPU exception handler libraries in subsequent patches. Signed-off-by: Ard Biesheuvel --- BaseTools/Conf/tools_def.template | 6 +++--- 1 file changed, 3 insertions(+), 3 deletions(-) diff --git a/BaseTools/Conf/tools_def.template b/BaseTools/Conf/tools_def.template index ae43101853870c6d..1855f1038b1571e4 100755 --- a/BaseTools/Conf/tools_def.template +++ b/BaseTools/Conf/tools_def.template @@ -3010,9 +3010,9 @@ RELEASE_XCODE5_IA32_CC_FLAGS = -arch i386 -c-Os -Wall -Werror -inclu ## # X64 definitions ## - DEBUG_XCODE5_X64_DLINK_FLAGS = -arch x86_64 -u _$(IMAGE_ENTRY_POINT) -e _$(IMAGE_ENTRY_POINT) -preload -segalign 0x20 -pie -all_load -dead_strip -seg1addr 0x240 -map $(DEST_DIR_DEBUG)/$(BASE_NAME).map - NOOPT_XCODE5_X64_DLINK_FLAGS = -arch x86_64 -u _$(IMAGE_ENTRY_POINT) -e _$(IMAGE_ENTRY_POINT) -preload -segalign 0x20 -pie -all_load -dead_strip -seg1addr 0x240 -map $(DEST_DIR_DEBUG)/$(BASE_NAME).map -RELEASE_XCODE5_X64_DLINK_FLAGS = -arch x86_64 -u _$(IMAGE_ENTRY_POINT) -e _$(IMAGE_ENTRY_POINT) -preload -segalign 0x20 -pie -all_load -dead_strip -seg1addr 0x240 -map $(DEST_DIR_DEBUG)/$(BASE_NAME).map + DEBUG_XCODE5_X64_DLINK_FLAGS = -arch x86_64 -u _$(IMAGE_ENTRY_POINT) -e _$(IMAGE_ENTRY_POINT) -preload -segalign 0x20 -pie -all_load -dead_strip -seg1addr 0x240 -read_only_relocs suppress -map $(DEST_DIR_DEBUG)/$(BASE_NAME).map + NOOPT_XCODE5_X64_DLINK_FLAGS = -arch x86_64 -u _$(IMAGE_ENTRY_POINT) -e _$(IMAGE_ENTRY_POINT) -preload -segalign 0x20 -pie -all_load -dead_strip -seg1addr 0x240 -read_only_relocs suppress -map $(DEST_DIR_DEBUG)/$(BASE_NAME).map +RELEASE_XCODE5_X64_DLINK_FLAGS = -arch x86_64 -u _$(IMAGE_ENTRY_POINT) -e _$(IMAGE_ENTRY_POINT) -preload -segalign 0x20 -pie -all_load -dead_strip -seg1addr 0x240 -read_only_relocs suppress -map $(DEST_DIR_DEBUG)/$(BASE_NAME).map *_XCODE5_X64_SLINK_FLAGS = -static -o DEBUG_XCODE5_X64_ASM_FLAGS = -arch x86_64 -g -- 2.39.2 -=-=-=-=-=-=-=-=-=-=-=- Groups.io Links: You receive all messages sent to this group. View/Reply Online (#102211): https://edk2.groups.io/g/devel/message/102211 Mute This Topic: https://groups.io/mt/97960758/21656 Group Owner: devel+ow...@edk2.groups.io Unsubscribe: https://edk2.groups.io/g/devel/unsub [arch...@mail-archive.com] -=-=-=-=-=-=-=-=-=-=-=-
