1. 21 Sep, 2014 5 commits
    • Peter Jones's avatar
      Generate a sane PE header on shim, fallback, and MokManager. · 2c59a1a0
      Peter Jones authored
      It turns out a7249a65 was masking a second problem - on some binaries,
      when we actually don't have any base relocations at all, binutils'
      "objcopy --target efi-app-x86_64" is generating a PE header with a base
      relocations pointer that happily points into the middle of our text
      section.  So with shim processing base relocations correctly, it refuses
      to load those binaries.
      For example, on one binary I just built:
      00000130  00 a0 00 00 0a 00 00 00  00 00 00 00 00 00 00 00 |................|
      which says there's a Base Relocation Table at 0xa000 that's 0xa bytes long.
      That's here:
      0000a000  58 00 29 00 00 00 00 00  48 00 44 00 28 00 50 00 |X.).....H.D.(.P.|
      0000a010  61 00 72 00 74 00 25 00  64 00 2c 00 53 00 69 00 |a.r.t.%.d.,.S.i.|
      0000a020  67 00 25 00 67 00 29 00  00 00 00 00 00 00 00 00 |g.%.g.).........|
      0000a030  48 00 44 00 28 00 50 00  61 00 72 00 74 00 25 00 |H.D.(.P.a.r.t.%.|
      So the table is:
      0000a000  58 00 29 00 00 00 00 00  48 00                   |X.).....H.      |
      That wouldn't be so bad, except those binaries are MokManager.efi,
      fallback.efi, and shim.efi, and sometimes they're .reloc, which we're
      actually trying to handle correctly now because grub builds with a real
      and valid .reloc table.  So though I didn't think there was any hair
      left on this yak, more shaving ensues.
      With this change, instead of letting objcopy do whatever it likes, we
      switch to "-O binary" and merely link in a header that's appropriate for
      our binaries.  This is the same method Ard wrote for aarch64, and it
      seems to work fine in either place (modulo some minor changes.)
      At some point this should be merged into gnu-efi instead of carrying our
      own crt0-efi-x86_64.S, but that's a less immediate problem.
      I did not need this problem.
      Signed-off-by: default avatarPeter Jones <pjones@redhat.com>
    • Peter Jones's avatar
      Fix our "in_protocol" printing. · 631225fb
      Peter Jones authored
      When I merged 4bfb13d8 and fixed the conflicts, I managed to make the
      in_protocol test exactly backwards, so that's why we don't currently see
      error messages.
      Signed-off-by: default avatarPeter Jones <pjones@redhat.com>
    • Peter Jones's avatar
      Don't call AuthenticodeVerify if vendor_cert_size is 0. · 213e29e2
      Peter Jones authored
      Actually check the size of our vendor cert quite early, so that there's
      no confusion as to what's going on.
      This isn't strictly necessary, in that in all cases if vendor_cert_size
      is 0, then AuthenticodeVerify -> Pkcs7Verify() -> d2i_X509() will result
      in a NULL "Cert", and it will return FALSE, and we'll reject the
      signature, but better to avoid all that code in the first place.  Belt
      and suspenders and whatnot.
      Based on a patch from https://github.com/TBOpen .
      Signed-off-by: default avatarPeter Jones <pjones@redhat.com>
    • Peter Jones's avatar
      Validate computed hash bases/hash sizes more thoroughly. · 0dcd5a8e
      Peter Jones authored
      I screwed one of these up when working on 750584c2, and it's a real pain
      to figure out, so that means we should be validating them.
      Signed-off-by: default avatarPeter Jones <pjones@redhat.com>
    • Peter Jones's avatar
      Make 64-on-32 maybe work on x86_64. · afec82ac
      Peter Jones authored
      This is mostly based on a patch (https://github.com/mjg59/shim/issues/30)
      from https://github.com/TBOpen , which refactors our __LP64__
      tests to be tests of the header magic instead.  I've simplified things
      by using what we've pre-loaded into "context" and making some helper
      functions so the conditionals in most of the code say what they do,
      instead of how they work.
      Note that we're only allowing that from in_protocol's loader - that is,
      we'll let 64-bit grub load a 32-bit kernel or 32-bit grub load a 64-bit
      kernel, but 32-bit shim isn't loading a 64-bit grub.
      Signed-off-by: default avatarPeter Jones <pjones@redhat.com>
  2. 19 Sep, 2014 1 commit
    • Peter Jones's avatar
      Actually refer to the base relocation table of our loaded image. · 486bf03e
      Peter Jones authored
      Currently when we process base relocations, we get the correct Data
      Directory pointer from the headers (context->RelocDir), and that header
      has been copied into our pristine allocated image when we copied up to
      SizeOfHeaders.  But the data it points to has not been mirrored in to
      the new image, so it is whatever data AllocPool() gave us.
      This patch changes relocate_coff() to refer to the base relocation table
      from the image we loaded from disk, but apply the fixups to the new
      I have no idea how x86_64 worked without this, but I can't make aarch64
      work without it.  I also don't know how Ard or Leif have seen aarch64
      work.  Maybe they haven't?  Leif indicated on irc that they may have
      only tested shim with simple "hello world" applications from gnu-efi;
      they are certainly much less complex than grub.efi, and are generated
      through a different linking process.
      My only theory is that we're getting recycled data there pretty reliably
      that just makes us /not/ process any relocations, but since our
      ImageBase is 0, and I don't think we ever load grub with 0 as its base
      virtual address, that doesn't follow.  I'm open to any other ideas
      anybody has.
      I do know that on x86_64 (and presumably aarch64 as well), we don't
      actually start seeing *symptoms* of this bug until the first chunk[0] of
      94c9a77f is applied[1].  Once that is applied, relocate_coff() starts
      seeing zero[2] for both RelocBase->VirtualAddress and
      RelocBase->SizeOfBlock, because RelocBase is a (generated, relative)
      pointer that only makes sense in the context of the original binary, not
      our partial copy.  Since RelocBase->SizeOfBlock is tested first,
      relocate_base() gives us "Reloc block size is invalid"[3] and returns
      EFI_UNSUPPORTED.  At that point shim exits with an error.
      [0] The second chunk of 94c9a77f patch makes no difference on this
      [1] I don't see why at all.
      [2] Which could really be any value since it's AllocatePool() and not
          AllocateZeroPool() results, but 0 is all I've observed; I think
          AllocatePool() has simply never recycled any memory in my test
      [3] which is silent because perror() tries to avoid talking because that
          has caused much crashing in the past; work needs to go in to 0.9 for
      Signed-off-by: default avatarPeter Jones <pjones@redhat.com>
  3. 27 Aug, 2014 4 commits
  4. 19 Aug, 2014 1 commit
  5. 12 Aug, 2014 5 commits
  6. 21 Jul, 2014 1 commit
  7. 14 Jul, 2014 1 commit
  8. 25 Jun, 2014 14 commits
  9. 13 May, 2014 3 commits
  10. 11 Apr, 2014 3 commits
  11. 14 Feb, 2014 2 commits