From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: X-Spam-Checker-Version: SpamAssassin 3.4.0 (2014-02-07) on aws-us-west-2-korg-lkml-1.web.codeaurora.org X-Spam-Level: X-Spam-Status: No, score=-9.2 required=3.0 tests=BAYES_00,DKIM_SIGNED, DKIM_VALID,FREEMAIL_FORGED_FROMDOMAIN,FREEMAIL_FROM, HEADER_FROM_DIFFERENT_DOMAINS,INCLUDES_PULL_REQUEST,MAILING_LIST_MULTI, NICE_REPLY_A,SPF_HELO_NONE,SPF_PASS,URIBL_BLOCKED,USER_AGENT_SANE_1 autolearn=ham autolearn_force=no version=3.4.0 Received: from mail.kernel.org (mail.kernel.org [198.145.29.99]) by smtp.lore.kernel.org (Postfix) with ESMTP id 8AAC2C433EF for ; Thu, 9 Sep 2021 11:21:29 +0000 (UTC) Received: from phobos.denx.de (phobos.denx.de [85.214.62.61]) (using TLSv1.2 with cipher ECDHE-RSA-AES128-GCM-SHA256 (128/128 bits)) (No client certificate requested) by mail.kernel.org (Postfix) with ESMTPS id A303861100 for ; Thu, 9 Sep 2021 11:21:28 +0000 (UTC) DMARC-Filter: OpenDMARC Filter v1.4.1 mail.kernel.org A303861100 Authentication-Results: mail.kernel.org; dmarc=fail (p=none dis=none) header.from=gmx.de Authentication-Results: mail.kernel.org; spf=pass smtp.mailfrom=lists.denx.de Received: from h2850616.stratoserver.net (localhost [IPv6:::1]) by phobos.denx.de (Postfix) with ESMTP id 738FC832C8; Thu, 9 Sep 2021 13:21:26 +0200 (CEST) Authentication-Results: phobos.denx.de; dmarc=fail (p=none dis=none) header.from=gmx.de Authentication-Results: phobos.denx.de; spf=pass smtp.mailfrom=u-boot-bounces@lists.denx.de Authentication-Results: phobos.denx.de; dkim=pass (1024-bit key; secure) header.d=gmx.net header.i=@gmx.net header.b="Ipzpngi2"; dkim-atps=neutral Received: by phobos.denx.de (Postfix, from userid 109) id 2C10A832DB; Thu, 9 Sep 2021 13:21:25 +0200 (CEST) Received: from mout.gmx.net (mout.gmx.net [212.227.17.22]) (using TLSv1.3 with cipher TLS_AES_256_GCM_SHA384 (256/256 bits)) (No client certificate requested) by phobos.denx.de (Postfix) with ESMTPS id 4D24E832A1 for ; Thu, 9 Sep 2021 13:21:20 +0200 (CEST) Authentication-Results: phobos.denx.de; dmarc=pass (p=none dis=none) header.from=gmx.de Authentication-Results: phobos.denx.de; spf=pass smtp.mailfrom=xypron.glpk@gmx.de DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/simple; d=gmx.net; s=badeba3b8450; t=1631186478; bh=9/ko+PpLYD3Lqq12TD9FAbM/rwS/tO40mBIjJglc5WU=; h=X-UI-Sender-Class:Subject:To:Cc:References:From:Date:In-Reply-To; b=Ipzpngi2a2KKqbxB5yEqrPeTxsKZfkZTzGVyuyS5/z+gx/H3N87NOSY0GqqAx9E54 e00RZ5nR44Ov2AGnDJNZHoC6/tWUnzq16uYtZ0nn5wtkDHp2sh1XFySzM9xju3liZQ 9nXc/Lpcoy5fYoYkXGpNg7S/7mt5z0JcNTJv/az4= X-UI-Sender-Class: 01bb95c1-4bf8-414a-932a-4f6e2808ef9c Received: from [192.168.123.55] ([88.152.144.157]) by mail.gmx.net (mrgmx104 [212.227.17.168]) with ESMTPSA (Nemesis) id 1MLR1f-1mg5z43X3T-00IQcZ; Thu, 09 Sep 2021 13:21:18 +0200 Subject: Re: Pull request for efi-2021-10-rc4 To: Simon Glass Cc: Tom Rini , U-Boot Mailing List , Alexander Graf , Masahisa Kojima , AKASHI Takahiro , Ilias Apalodimas References: <61e2e1cc-b474-84dd-eceb-bb86a59972be@gmx.de> <20210904130111.GA12964@bill-the-cat> <20210904143722.GD12964@bill-the-cat> <46CAD3CD-41FA-43B9-9099-225711B754E1@gmx.de> <20210904173949.GI12964@bill-the-cat> <8FD4F99B-E1A5-4842-8BB8-EA2749E4761B@gmx.de> <20210904180825.GJ12964@bill-the-cat> From: Heinrich Schuchardt Message-ID: Date: Thu, 9 Sep 2021 13:21:17 +0200 User-Agent: Mozilla/5.0 (X11; Linux x86_64; rv:78.0) Gecko/20100101 Thunderbird/78.13.0 MIME-Version: 1.0 In-Reply-To: Content-Type: text/plain; charset=utf-8; format=flowed Content-Language: en-US Content-Transfer-Encoding: quoted-printable X-Provags-ID: V03:K1:VBl3HBt/bInj+uELaIVg7hF4pB7gaXBoEnArV2DBSjhGwEPYhgq t8+jeBji0F35qW9t6+yWAkx+HeAU+/B8un3yX6lz7g4MH1/MTg6fx0bKSm6kLwWF5H2kaOh PD9Wuq8ACewHLPa6XBjRTmrejNvVOREDLUB3uibCOfP2S6NEDx+XSEnNnhIc6SYkY0mxbjV 0OobVb7fML0vJJeTX63gA== X-UI-Out-Filterresults: notjunk:1;V03:K0:iGd2Taj4Wrk=:QOGOupoVrzuOYAi1XHnLMG u9LNQf+uez9wjn2Cb2Ma/6X2al1iY5UGkpzAjWODMrbtDMKCzf3O5FOxyP9yQicu0WPUUp53X kb2895T3jvp5eCd1Zu8hqeeJa4PfcgSrnHEpwzO/fpKazNeVa1RBfR62uaFqCSX5nekFpZGKw jnUL7y5kMVJfthzfj5hRorEE13Jd36bf2iLkap+qGVSk3fsBCnXN4VNrIkD/dUSDWvYz7B2ZX XAsXnP+xPKHjtaJ4e10ptgBuY96Y1ow8fCtx8ZznY7ZeQFJsykOkgx+EvtbUgQCF1VN4qRrLg pQ/8X9kyDCYaOWHjWyDSP7FtBJtZJBqLIwIjlCvoWU4xbDw3XDJIsEqwTfawPq8+DX6QsooTF J3O58Z2wu+dFaavn7HwSlGPFIU4t4RXXHD+7+5DQgybeaqWfGkiBS4E8ipXfeJg1Gw0x8Sxlo 0PfXgJm0lzUabQKJUFrLhfZTvatf6EY+rVvSu+nI7NdFujiOlopfsmoD2C5YxVRPkXj0LGZkR mgxOK3zFAMSkWJOGbBwMcQyUey9PsnnWh5rqSEaWFrXJg5QR7fTb8CI6iOEOq3QAacoCFEMw2 RCUU+84Kxaye4HWyNPPrw1HYmKbmuuoCcwegZ7T00HV9/Y1fWSMqAwWPzWcZLVXPWSrevHiue c8sr0M51e6rDPCiEq9t4rl2BbaE/B3p6J2Z06pcLjk9sA25mIZsGCUfkJaobrV6zqLfa/HbkB M76er9oBp3wT9RtdHk0UUmeBSoqbTlKrh32bQ5loDae18oPFwSPG05h6Tga1dN2w6+YLCqNYb 4xXsXFm2dhL4yTCkcrXUKNX6fIdxCM+KGCBdf1Pi8xi+LnevoSWeu4fRPBb/6BBD8HIVngNML HNhS1Z2nY34DuiV3XeSwMFWyeF8S7w58AM1nn9djDP5lPpThV6vn1wP8P1mfK5QoKF7xP++4H mGL+jX2wsust0awB7eL6Jix5IimVRuoQTOr29fXjTPelOOnR6y90jbAvMpipB/QFb382I7pjM AolPaFUk6mbAf1EvXiQhync8G2OtJDxqhHXXVJwX6SMfat0aXWcy+8a//XM+1r+1QujUJUZke J/f1zo6yqNeQ2A= X-BeenThere: u-boot@lists.denx.de X-Mailman-Version: 2.1.34 Precedence: list List-Id: U-Boot discussion List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , Errors-To: u-boot-bounces@lists.denx.de Sender: "U-Boot" X-Virus-Scanned: clamav-milter 0.103.2 at phobos.denx.de X-Virus-Status: Clean On 9/9/21 10:56 AM, Simon Glass wrote: > Hi Ilias, > > On Sat, 4 Sept 2021 at 14:09, Ilias Apalodimas > wrote: >> >> Hi Tom, >> >> On Sat, 4 Sept 2021 at 21:08, Tom Rini wrote: >>> >>> On Sat, Sep 04, 2021 at 08:02:49PM +0200, Heinrich Schuchardt wrote: >>>> >>>> >>>> Am 4. September 2021 19:39:49 MESZ schrieb Tom Rini : >>>>> On Sat, Sep 04, 2021 at 07:03:48PM +0200, Heinrich Schuchardt wrote: >>>>>> >>>>>> >>>>>> Am 4. September 2021 16:37:22 MESZ schrieb Tom Rini : >>>>>>> On Sat, Sep 04, 2021 at 03:08:38PM +0200, Heinrich Schuchardt wrot= e: >>>>>>>> >>>>>>>> >>>>>>>> Am 4. September 2021 15:01:11 MESZ schrieb Tom Rini : >>>>>>>>> On Sat, Sep 04, 2021 at 11:56:47AM +0200, Heinrich Schuchardt wr= ote: >>>>>>>>> >>>>>>>>>> Dear Tom, >>>>>>>>>> >>>>>>>>>> The following changes since commit 94509b79b13e69c209199af0757a= fbde8d2ebd6d: >>>>>>>>>> >>>>>>>>>> btrfs: Use default subvolume as filesystem root (2021-09-01 = 10:11:24 >>>>>>>>>> -0400) >>>>>>>>>> >>>>>>>>>> are available in the Git repository at: >>>>>>>>>> >>>>>>>>>> https://source.denx.de/u-boot/custodians/u-boot-efi.git >>>>>>>>>> tags/efi-2021-10-rc4 >>>>>>>>>> >>>>>>>>>> for you to fetch changes up to 1dfa494610c5469cc28cf1f8538abf4b= e6c00324: >>>>>>>>>> >>>>>>>>>> efi_loader: fix efi_tcg2_hash_log_extend_event() parameter c= heck >>>>>>>>>> (2021-09-04 09:15:09 +0200) >>>>>>>>>> >>>>>>>>>> ---------------------------------------------------------------= - >>>>>>>>>> Pull request for efi-2021-10-rc4 >>>>>>>>>> >>>>>>>>>> Documentation: >>>>>>>>>> >>>>>>>>>> Remove invalid reference to configuration variable in UEFI= doc >>>>>>>>>> >>>>>>>>>> UEFI: >>>>>>>>>> >>>>>>>>>> Parameter checks for the EFI_TCG2_PROTOCOL >>>>>>>>>> Improve support of preseeding UEFI variables. >>>>>>>>>> Correct the calculation of the size of loaded images. >>>>>>>>>> Allow for UEFI images with zero VirtualSize >>>>>>>>>> >>>>>>>>>> ---------------------------------------------------------------= - >>>>>>>>>> Heinrich Schuchardt (5): >>>>>>>>>> efi_loader: sections with zero VirtualSize >>>>>>>>>> efi_loader: rounding of image size >>>>>>>>>> efi_loader: don't load signature database from file >>>>>>>>>> efi_loader: efi_auth_var_type for AuditMode, DeployedMod= e >>>>>>>>>> efi_loader: correct determination of secure boot state >>>>>>>>>> >>>>>>>>>> Masahisa Kojima (3): >>>>>>>>>> efi_loader: add missing parameter check for EFI_TCG2_PRO= TOCOL api >>>>>>>>>> efi_loader: fix boot_service_capability_min calculation >>>>>>>>>> efi_loader: fix efi_tcg2_hash_log_extend_event() paramet= er check >>>>>>>>> >>>>>>>>> And I don't see Simon's revert in here either. And he asked you= about >>>>>>>>> that yesterday: >>>>>>>>> https://lore.kernel.org/r/CAPnjgZ3eRdjF0jb9S-cJK6y+feuyRyWf0hNkf= 2triB4DR4UFBQ@mail.gmail.com/ >>>>>>>>> >>>>>>>>> So at this point, are you asserting there is nothing to revert? >>>>>>>> >>>>>>>> Never. Simons "revert" is breaking functionality. The concept for= suporting blobs in devicetrees supplied by a prior bootstage has not been= defined yet. >>>>>>> >>>>>>> And to be clearer, reverting something that was introduced in one = rc in >>>>>>> a later rc isn't breaking functionality. U-Boot releases (well, t= he >>>>>>> non-rc ones for sure) are on a very regular schedule. External pr= ojects >>>>>>> may not depend on some feature introduced at -rcN unless they're w= illing >>>>>>> to accept that some changes could happen before release. >>>>>>> >>>>>> >>>>>> There is no value delivered by Simon's series. Neither does the ima= ge get smaller nor does it fix anything. If he wants to enforce a design, = it must work for all use cases. But this requires some conceptual work. >>>>> >>>>> Yes, and what's the rush to not do the conceptual work? If I recall >>>>> part of the thread correctly, yes, Simon didn't get his objections i= n >>>>> before the patches were merged, but it was early enough in the relea= se >>>>> cycle that taking a step back and reverting was a reasonable request= . >>>>> What he had said wouldn't have changed if he had gotten the email ou= t a >>>>> few days earlier. >>>>> >>>>> So yes, please merge Simon's revert, or post and merge new more mini= mal >>>>> revert that brings things to the same functional end. There are >>>>> objections to this implementation, and thus far Simon has been >>>>> responding all of the requests to better clarify all of the related = code >>>>> and concepts that have been asked of him, so that in the end an >>>>> implementation that fulfills all of the technical requirements can b= e >>>>> created, that hopefully leaves all parties satisfied. >>>> >>>> There is nothing wrong with the current code. >>>> >>>> It is Simon's concept of blobs in devicetrees that is borked in that >>>> it ignores QEMU and any board that gets the DT from a prior boot >>>> stage. >>> >>> Then it should be pretty easy to get Simon to withdraw his objections, >>> if there's such a fundamental "this is the only possible way, no >>> changes" path forward. >>> >>>> Simon's patches have no functional end. So what do you mean by "same = functional end"? >>> >>> I mean the state of the EFI subsystem, prior to the code in question >>> being merged, without breaking the other assorted EFI changes that hav= e >>> come in since then. >>> >>> -- >>> Tom >> I'll sum this up since there's many emails on the topic. >> >> The current changes move the public needed for capsule updates in >> U-Boot's .rodata section. >> When I sent this, I assumed u-boot was mapping .rodata as read only. >> Since it doesn't the protection I was hoping for isn't there. So >> security wise the two different proposals are on par. Arguably it's >> easier to fix .rodata instead of copying the key from the dtb and >> switching the pages to RO, but that's really minor. >> >> However keeping the key on the DTB has some of limitations, with the >> most notable being that you *must* only use CONFIG_OF_SEPARATE for >> your DTB, while there's four different ways available in the Kconfig >> (and 3 are usable on production). I've repeated enough times, that I >> don't mind changing the code and keeping the key on the DTB, as long >> as the limitations are lifted. If that means reverting the patch now >> and fixing it in the future, I am fine with that as well. >> To be honest I don't understand why this has to be set in stone. Even >> if we keep the current patchset and change it to the dtb in the >> future, that will have zero effect on the users. Once they upgrade to >> the newer, shinier version, their key will just be read from a >> different location, but that's all hidden from the user. The only they >> will have to change, is how they include that key on the final binary. > > This is a reasonable summary I think. > > The devicetree series I sent explains how to deal with the limitations > here. Heinrich requested that I clarify this which I did. I fully > expected that the revert would then be applied. But instead it seems > there is not even agreement about the status quo (of use of devicetree > in U-Boot). > > OF_SEPARATE is used by the vast majority of boards, including most > qemu builds. I think the OF_BOARD thing should probably be deleted. > The OF_EMBED thing should not be used in production. It is needed with > the EFI app though and I recently sent a series to support updating > the DT there. > > For OF_PRIOR_STAGE the prior state is responsible for supplying the > DT, and needs to do so and meet U-Boot's requirements. I have clearly > set that out in the devicetree patch. Yes, and this was wrong. You cannot impose U-Boot's requirements on other projects. Instead I suggested to use an overlay tree to add our stuff on the devicetree from the prior stage once U-Boot is invoked. Best regards Heinrich > > https://patchwork.ozlabs.org/project/uboot/list/?series=3D260032&state= =3D* > > So what has happened here is a short-cut and not the correct approach. > It needs to be reverted before the release, since otherwise people > will rely on it and we will be stuck with two ways to do signatures. > > Regards, > Simon >