linux-kernel - Re: [PATCH] riscv: CONFIG_EFI should not depend on CONFIG_RISCV_ISA

lists.openwall.net		lists / announce owl-users owl-dev john-users john-dev passwdqc-users yescrypt popa3d-users / oss-security kernel-hardening musl sabotage tlsify passwords / crypt-dev xvendor / Bugtraq Full-Disclosure linux-kernel linux-netdev linux-ext4 linux-hardening linux-cve-announce PHC
Open Source and information security mailing list archives

Hash Suite: Windows password security audit tool. GUI, reports in PDF.

[<prev] [next>] [<thread-prev] [thread-next>] [day] [month] [year] [list]

Message-ID: <87ttpyu1r3.fsf@all.your.base.are.belong.to.us>
Date:   Mon, 06 Nov 2023 20:21:04 +0100
From:   Björn Töpel <bjorn@...nel.org>
To:     Palmer Dabbelt <palmer@...belt.com>
Cc:     aou@...s.berkeley.edu, anup@...infault.org, atishp@...shpatra.org,
        Atish Patra <atishp@...osinc.com>,
        Paul Walmsley <paul.walmsley@...ive.com>,
        Bjorn Topel <bjorn@...osinc.com>,
        Ard Biesheuvel <ardb@...nel.org>, sunilvl@...tanamicro.com,
        linux-kernel@...r.kernel.org, linux-riscv@...ts.infradead.org
Subject: Re: [PATCH] riscv: CONFIG_EFI should not depend on CONFIG_RISCV_ISA_C

Palmer Dabbelt <palmer@...belt.com> writes:

> On Tue, 24 Oct 2023 12:26:48 PDT (-0700), bjorn@...nel.org wrote:
>> From: Björn Töpel <bjorn@...osinc.com>
>>
>> UEFI/PE mandates that the kernel Image starts with "MZ" ASCII
>> (0x5A4D). Convenient enough, "MZ" is a valid compressed RISC-V
>> instruction. This means that a non-UEFI loader can simply jump to
>> "code0" in the Image header [1] and start executing.
>>
>> The Image specification [1] says the following about "code0":
>>   |   This header is also reused to support EFI stub for RISC-V. EFI
>>   |   specification needs PE/COFF image header in the beginning of the
>>   |   kernel image in order to load it as an EFI application. In order
>>   |   to support EFI stub, code0 is replaced with "MZ" magic string
>>   |   and res3(at offset 0x3c) points to the rest of the PE/COFF
>>   |   header.
>>
>> "MZ" is not a valid instruction for implementations without the C
>> extension.
>>
>> A non-UEFI loader, loading a non-C UEFI image have the following
>> options:
>>   1. Trap and emulate "code0"
>>   2. Avoid "code0" if it is "MZ", and have the kernel entry at
>>      "code1".
>>
>> Replace the compressed instruction with a hex code variant, that works
>> for CONFIG_RISCV_ISA_C=n builds. Further, this change also make sure
>> that the "code0" instruction is 32b aligned.
>
> IMO this breaks the Image format on non-C systems: they'll jump straight 
> to the start (as there's no symbols left or anything) and then just fail 
> to execute the C instructions.

That's right! My idea, which was probably really poor articulated, was
to add trap/emulate in OpenSBI/non-C for the MZ instructions.

> We could amend the Image format to require bootloaders to handle this 
> (ie, look at the first instructions and emulate them if there's no C), 
> that would require some bootloader updates but given this doesn't work 
> maybe that's fine.
>
> We could also just stop producing Image+PE binaries on non-C systems 
> (ie, just produce PE).

Yes, or make the Image loader in Qemu et al more robust, by e.g.
checking code0/code1. The Image spec does say that code0 is 'MZ' for
PE+Image builds, and for non-C capable systems one could argue that it
should be checked/skipped by the loader.

Does the arguments above make sense for you? I.e.:
  1.   Merge this patch
  2a.  Add the trap/emu to OpenSBI
  (2b. Improve the image loader in Qemu?)


Björn