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: <Z0S+zP3u4781zNwo@e129823.arm.com>
Date: Mon, 25 Nov 2024 18:15:40 +0000
From: Levi Yun <yeoreum.yun@....com>
To: Ard Biesheuvel <ardb@...nel.org>
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

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.

Am I missing?

Thanks.