Re: [RFC PATCH 1/3] efi/x86: Use symbolic constants in PE header instead of bare numbers

From: Arvind Sankar <nivedita@alum.mit.edu>
To: Ard Biesheuvel <ardb@kernel.org>
Cc: Arvind Sankar <nivedita@alum.mit.edu>,
	linux-efi <linux-efi@vger.kernel.org>,
	Laszlo Ersek <lersek@redhat.com>,
	Leif Lindholm <leif@nuviainc.com>,
	Peter Jones <pjones@redhat.com>,
	Matthew Garrett <mjg59@google.com>,
	Alexander Graf <agraf@csgraf.de>,
	Ilias Apalodimas <ilias.apalodimas@linaro.org>,
	Heinrich Schuchardt <xypron.glpk@gmx.de>,
	Daniel Kiper <daniel.kiper@oracle.com>,
	James Bottomley <James.Bottomley@hansenpartnership.com>
Subject: Re: [RFC PATCH 1/3] efi/x86: Use symbolic constants in PE header instead of bare numbers
Date: Thu, 20 Feb 2020 14:33:43 -0500	[thread overview]
Message-ID: <20200220193342.GA2459390@rani.riverdale.lan> (raw)
In-Reply-To: <CAKv+Gu_hVszLmChFvrv5MU5aS5-Vuc4AXB7R0XNXn+Amgx=Ogg@mail.gmail.com>

On Thu, Feb 20, 2020 at 07:14:27PM +0100, Ard Biesheuvel wrote:
> On Thu, 20 Feb 2020 at 19:05, Arvind Sankar <nivedita@alum.mit.edu> wrote:
> >
> > On Thu, Feb 20, 2020 at 06:32:39PM +0100, Ard Biesheuvel wrote:
> > > 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?
> >
> > commit 743628e868c5 ("x86, efi stub: Add .reloc section back into
> > image") says that
> >         Some UEFI firmware will not load a .efi with a .reloc section
> >         with a size of 0.
> >
> > Is that the issue you're refering to? It is a bit odd, since we actually
> > leave base relocation table at a size of zero with an RVA of zero, so it
> > shouldn't even look at the .reloc section according to the spec. At
> > least current EKD2 code doesn't seem to -- I think it would even work if
> > you specify fewer tables than 6 so that the base relocation table is
> > missing altogether.
> 
> I can only agree with that, and I have never experienced any issues
> loading PE/COFF images without .reloc sections on any architecture.
> 
> But looking at that patch, it seems it only changes the .reloc
> section's size, it doesn't actually add it back. The .reloc section
> has been there from the beginning, with a note saying that it is
> required, which doesn't seem to be the case today, and looking at the
> history of EDK2, I can't really spot any changes in that regard
> either.

Yeah I thought given that it caused a problem some firmware must have
been looking at the reloc section, but on the other hand it might just
have been that the firmware barfed because it didn't expect any section
to be zero size?