On Fri, 31 Mar 2023 at 10:29, Marvin Häuser <mhaeu...@posteo.de> wrote: > > > > On 31. Mar 2023, at 09:39, Ard Biesheuvel <a...@kernel.org> wrote: > > Hi Marvin, > > > > Thanks for the context. > > > > > > On Thu, 30 Mar 2023 at 23:54, Marvin Häuser <mhaeu...@posteo.de> 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] -=-=-=-=-=-=-=-=-=-=-=-
