Received-SPF: pass (google.com: domain of linux-kernel+bounces-52161-linux.lists.archive=gmail.com@vger.kernel.org designates 147.75.48.161 as permitted sender) client-ip=147.75.48.161;
Precedence: bulk
MIME-Version: 1.0
References: <CAHzAAWQ6srV6LVNdmfbJhOwhBw5ZzxxZZ07aHt9oKkfYAdvuQQ@mail.gmail.com>
 <ZcBCiqOroolz1hoh@archie.me> <CAHzAAWQ2snWppQK_D3cSR6m35htVqM+4Su9f1JT9oWZ2A9Vznw@mail.gmail.com>
In-Reply-To: <CAHzAAWQ2snWppQK_D3cSR6m35htVqM+4Su9f1JT9oWZ2A9Vznw@mail.gmail.com>
From: Ard Biesheuvel <ardb@kernel.org>
Date: Mon, 5 Feb 2024 08:47:03 +0100
Message-ID: <CAMj1kXFyVKRg0DTTrZGPGfVPxqY5zsXbK+Sxp0GBid-msqe46w@mail.gmail.com>
Subject: Re: Broken section alignment in 6.7 and 6.8rc EFI stub
To: Mike Beaton <mjsbeaton@gmail.com>
Cc: Bagas Sanjaya <bagasdotme@gmail.com>, Linux EFI <linux-efi@vger.kernel.org>, 
	Linux Kernel Mailing List <linux-kernel@vger.kernel.org>, Ivan Hu <ivan.hu@canonical.com>, 
	Jeremy Kerr <jk@ozlabs.org>, Peter Jones <pjones@redhat.com>
Content-Type: text/plain; charset="UTF-8"

On Mon, 5 Feb 2024 at 08:37, Mike Beaton <mjsbeaton@gmail.com> wrote:
>
> On Mon, 5 Feb 2024 at 02:06, Bagas Sanjaya <bagasdotme@gmail.com> wrote:
> >
> > So v6.7 onwards misses .reloc section, right?
> >
> > Confused...
>
> Reloc info is still present as normal in data directories, e.g.
> `llvm-objdump -p` shows NumberOfRvaAndSizes = 6. Reloc info is taken
> from index 5 https://github.com/tianocore/edk2/blob/master/MdePkg/Include/IndustryStandard/PeImage.h#L128
> . (I've been told a dummy .reloc section was dumped recently?)
>

So there are a couple of things going on here:
- the .reloc section was indeed dropped because we could not find any
evidence anywhere that the reason it was added is still valid;
- the .compat section uses a non-1:1 RVA mapping, to avoid padding,
but reading the PE/COFF spec again, I suppose this is not compliant.

Note that objdump looks broken too, on an image I have locally, I get

(llvm-readelf -a)

  Section {
    Number: 2
    Name: .compat (2E 63 6F 6D 70 61 74 00)
    VirtualSize: 0x8
    VirtualAddress: 0xB82000
    RawDataSize: 4096
    PointerToRawData: 0x4000

(objdump -h)

  1 .compat       00000008  0000000000b82000  0000000000b82000  00004000  2**2
                  CONTENTS, ALLOC, LOAD, READONLY, DATA

So the .compat section is in the correct spot in the file view but not
in the memory view.

Given that the .setup section is not relevant to EFI boot anyway, we
could try to use the same file mapping as the virtual mapping, and
just split the content preceding .text across the .setup and .compat
sections arbitrarily.

Could you try the below?


diff --git a/arch/x86/boot/header.S b/arch/x86/boot/header.S
index b2771710ed98..f8b48ff86dd9 100644
--- a/arch/x86/boot/header.S
+++ b/arch/x86/boot/header.S
@@ -106,8 +106,7 @@ extra_header_fields:
        .word   0                               # MinorSubsystemVersion
        .long   0                               # Win32VersionValue

-       .long   setup_size + ZO__end + pecompat_vsize
-                                               # SizeOfImage
+       .long   setup_size + ZO__end            # SizeOfImage

        .long   salign                          # SizeOfHeaders
        .long   0                               # CheckSum
@@ -143,7 +142,7 @@ section_table:
        .ascii  ".setup"
        .byte   0
        .byte   0
-       .long   setup_size - salign             # VirtualSize
+       .long   pecompat_fstart  - salign       # VirtualSize
        .long   salign                          # VirtualAddress
        .long   pecompat_fstart - salign        # SizeOfRawData
        .long   salign                          # PointerToRawData
@@ -157,7 +156,7 @@ section_table:
        .asciz  ".compat"

        .long   8                               # VirtualSize
-       .long   setup_size + ZO__end            # VirtualAddress
+       .long   pecompat_fstart                 # VirtualAddress
        .long   pecompat_fsize                  # SizeOfRawData
        .long   pecompat_fstart                 # PointerToRawData

@@ -173,7 +172,6 @@ section_table:
         */
        .pushsection ".pecompat", "a", @progbits
        .balign falign
-       .set    pecompat_vsize, salign
        .globl  pecompat_fstart
 pecompat_fstart:
        .byte   0x1                             # Version
@@ -182,7 +180,6 @@ pecompat_fstart:
        .long   setup_size + ZO_efi32_pe_entry  # Entrypoint
        .popsection
 #else
-       .set    pecompat_vsize, 0
        .set    pecompat_fstart, setup_size
 #endif
        .ascii  ".text"