On Thu, Feb 20, 2020 at 06:32:39PM +0100, Ard Biesheuvel wrote: > On Thu, 20 Feb 2020 at 18:28, Arvind Sankar <nivedita@xxxxxxxxxxxx> wrote: > > > > The .text section contains data as well. Shouldn't it be marked > > IMAGE_SCN_MEM_WRITE also? > > > > Another good point. For ARM and arm64, I actually changed this into > split R-X and RW- sections, in case the loader decides to honour these > attributes (which it should imo) > > So yes, we should either add IMAGE_SCN_MEM_WRITE, or add a .data > section (although we probably don't have the space for that). > > Another thing I wondered was whether we really need the .reloc > section. We don't have one on ARM, and it works fine with the existing > EDK2 loader. > Peter: any idea whether the issue with .reloc you pointed out to me > the other day still exists in EDK2 today? Is there any reason we couldn't combine .setup + .text as R-X, then .reloc, then .data + .bss as RW-? That would leave room for the new .compat section then even without removing .reloc. Although if the loader actually honored the non-writable setting, efi_relocate_kernel has to be done unconditionally. It currently happens to be unconditional if you come in via PE loader, since we don't set ImageBase to tell it our preferred address, and even if we did, the existence of .setup means startup32 isn't at that address.
