linux-kernel - Re: [PATCH v2 2/2] efi/fdt: ignore dtb when acpi option is used with force

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: <CAMj1kXHysummq5dG==gOmQr2DgtU5Ee0_tJbjGagRJniAee1vA@mail.gmail.com>
Date: Mon, 25 Nov 2024 19:32:26 +0100
From: Ard Biesheuvel <ardb@...nel.org>
To: Levi Yun <yeoreum.yun@....com>
Cc: broonie@...nel.org, sami.mujawar@....com, sudeep.holla@....com, 
	pierre.gondois@....com, hagarhem@...zon.com, catalin.marinas@....com, 
	will@...nel.org, guohanjun@...wei.com, Jonathan.Cameron@...wei.com, 
	linux-arm-kernel@...ts.infradead.org, linux-kernel@...r.kernel.org, 
	linux-efi@...r.kernel.org
Subject: Re: [PATCH v2 2/2] efi/fdt: ignore dtb when acpi option is used with force

On Mon, 25 Nov 2024 at 19:16, Levi Yun <yeoreum.yun@....com> wrote:
>
> Hi Ard.
>
> >
> > The DT is not stored in a variable.
> >
> > If CONFIG_EFI_ARMSTUB_DTB_LOADER is enabled, it may be provided via
> > dtb= on the command line, but I have little sympathy for a user that
> > passes both dtb= *and* acpi=force, so this is a scenario that we can
> > ignore.
> >
> > Otherwise, it is taken from a EFI config table, which is just a
> > <guid,addr> tuple describing a location in physical memory where the
> > firmware has placed a DT. If the firmware puts a corrupted DT there,
> > the firmware should be fixed instead.
> >
> > acpi=force is intended to force the use of ACPI tables on a system
> > that provides both.
> >
> > > also, although acpi= could differ from architecture, the force option's menaing
> > > seems the same over architecture (ignore DT boot with ACPI tables).
> > >
> > > So I think the check the "acpi=force" option to prevent loading DT seems
> > > good.
> > >
> >
> > The EFI stub does not care about ACPI vs DT boot, and I'd prefer to
> > keep it that way unless there is a good reason.
> >
> > Which real-world problem does this patch aim to solve?
>
> Well. I had lack of explaination. In case of Juno platform, it loads
> FDT from "Fdt" variable from the storage and install it into
> configuration table with corrupted Fdt because of FDT stored in variable
> storage was corrupted.
>
> In that siutation, If it loads corrupted fdt, it prints error message
> while sanity check in early_init_dt_scan().
> This kind of error message would be confused to user because
> user already specifies to boot with acpi table only with acpi=force
> option.
>
> anyway, what kind of way to install fdt into configuraiton table is not
> matter. but when the dt installed in configuration table isn't valid,
> it could produce the error message which seems violate user specified
> option.
>
> unless check the acpi=force to ignore DT, I think it would require to
> check the installed DT in configuration table or passed should have
> simple sanity check doen in early_init_dt_scan() so that error messsage
> which makes some confusion for this situation.
>

Thanks for explaining the issue in more detail. Juno is a development
platform with a highly unusual boot stack so I don't think we need to
accommodate its quirks in the upstream kernel.

And I still don't think this is something worth fixing in general.
Even if the machine description should be taken from ACPI tables only,
the DT /chosen node is always used as a conduit by the EFI stub, and
there are cases, e.g., for initrd info or the kaslr seed, where this
information might come from the bootloader, such as older GRUB builds.