On 10 October 2014 12:49, Mark Rutland <mark.rutland@xxxxxxx> wrote: > On Thu, Oct 09, 2014 at 08:03:52PM +0100, Ard Biesheuvel wrote: >> On 9 October 2014 19:23, Mark Rutland <mark.rutland@xxxxxxx> wrote: >> > Hi Ard, >> > >> > On Wed, Oct 08, 2014 at 03:11:27PM +0100, Ard Biesheuvel wrote: >> >> After the EFI stub has done its business, it jumps into the kernel by >> >> branching to offset #0 of the loaded Image, which is where it expects >> >> to find the header containing a 'branch to stext' instruction. >> >> >> >> However, the UEFI spec 2.1.1 states the following regarding PE/COFF >> >> image loading: >> >> "A UEFI image is loaded into memory through the LoadImage() Boot >> >> Service. This service loads an image with a PE32+ format into memory. >> >> This PE32+ loader is required to load all sections of the PE32+ image >> >> into memory." >> >> >> >> In other words, it is /not/ required to load parts of the image that are >> >> not covered by a PE/COFF section, so it may not have loaded the header >> >> at the expected offset, as it is not covered by any PE/COFF section. >> > >> > What does this mean for handle_kernel_image? Given we might not have >> > _text through to _stext mapped, do we not need to take that into >> > account? >> > >> >> Actually, handle_kernel_image() does not interpret the header, it just >> copies it along with the rest of the image if it needs to be >> relocated, so I don't see an issue there. > > Sorry, I wasn't clear enough with my concern. My concern was whether we > had any guarantee _something_ was mapped for the address range covering > efi_head to stext. > > So long as _something_ is mapped there, we're ok -- handle_kernel_image > will just copy some garbage along with the usable portion of the kernel. > Indeed. > But if the EFI loader is allowed to load stext at the precise start of > RAM (or anywhere not in the idmap), in attempting the copy we'd try to > access unmapped addresses. > > So if that's a possibility, we need to shrink the copy to cover stext > to _edata rather than _text to edata. > > Does that make sense? > That cannot happen. The PE/COFF .text section's positive relative virtual offset ensures that the memory image has room for the header, it's just not guaranteed that anything gets copied there. >> However, I do remember Mark >> Salter mentioning that there is at least one other location that needs >> to be fixed up if this concern is valid. Mark? >> >> > Also, have we seen problems on any systems yet? >> > >> >> No, I am not aware of any occurrences of this exact issue, this is >> just one of the things I spotted while working on this code. > > Ok. I was just curious as to how urgent this was. > >> But I think we mostly agree that branching through the header relies >> on behavior of the PE/COFF loader that is not covered by the spec. > > Yes. We should not rely on unspecified behaviour. > > Mark. -- To unsubscribe from this list: send the line "unsubscribe linux-efi" in the body of a message to majordomo@xxxxxxxxxxxxxxx More majordomo info at http://vger.kernel.org/majordomo-info.html
