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=-10.8 required=3.0 tests=BAYES_00,DKIM_SIGNED, DKIM_VALID,DKIM_VALID_AU,HEADER_FROM_DIFFERENT_DOMAINS,INCLUDES_PULL_REQUEST, MAILING_LIST_MULTI,SPF_HELO_NONE,SPF_PASS 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 216A7C433F5 for ; Sat, 4 Sep 2021 20:09:16 +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 45A5E600D4 for ; Sat, 4 Sep 2021 20:09:15 +0000 (UTC) DMARC-Filter: OpenDMARC Filter v1.4.1 mail.kernel.org 45A5E600D4 Authentication-Results: mail.kernel.org; dmarc=fail (p=none dis=none) header.from=linaro.org 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 0884682ECC; Sat, 4 Sep 2021 22:09:13 +0200 (CEST) Authentication-Results: phobos.denx.de; dmarc=pass (p=none dis=none) header.from=linaro.org Authentication-Results: phobos.denx.de; spf=pass smtp.mailfrom=u-boot-bounces@lists.denx.de Authentication-Results: phobos.denx.de; dkim=pass (2048-bit key; unprotected) header.d=linaro.org header.i=@linaro.org header.b="NTDiecpX"; dkim-atps=neutral Received: by phobos.denx.de (Postfix, from userid 109) id 6891C83230; Sat, 4 Sep 2021 22:09:11 +0200 (CEST) Received: from mail-yb1-xb35.google.com (mail-yb1-xb35.google.com [IPv6:2607:f8b0:4864:20::b35]) (using TLSv1.3 with cipher TLS_AES_128_GCM_SHA256 (128/128 bits)) (No client certificate requested) by phobos.denx.de (Postfix) with ESMTPS id 23FF582DE4 for ; Sat, 4 Sep 2021 22:09:07 +0200 (CEST) Authentication-Results: phobos.denx.de; dmarc=pass (p=none dis=none) header.from=linaro.org Authentication-Results: phobos.denx.de; spf=pass smtp.mailfrom=ilias.apalodimas@linaro.org Received: by mail-yb1-xb35.google.com with SMTP id z5so5242345ybj.2 for ; Sat, 04 Sep 2021 13:09:07 -0700 (PDT) DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=linaro.org; s=google; h=mime-version:references:in-reply-to:from:date:message-id:subject:to :cc; bh=r8gamDvhcVApUzEi8B4eczMwzIgeevaZa9gUyXA1low=; b=NTDiecpXfNqLOoLvGeSXG2x+FMoMeClMwqJLFyyKBhwQI4Pr8lECbVu005fPEyuZoL r+7VZqybL/gP6gc5UDpT7u+NYhv7j3Ud6mNvF1Mdh76mJHeExxEs1KhkbzZ2ZPo4iJjV rh8aaHg/w1HMQIMnupC03tBRlKmM7NxlNVNEAN44yq80maa7Femn0MNNwE6sBd+pnN1z ShtW2IXDpT1C9osyE0s7tjFxaYn52hIw/Xf7L23o085wAmda1vNRylGZKiwelioNSMiG HasE4bA2+kHUO1X61+OUJCC2UXPwyOySzDk5qkhKj/cmZQgEf474eLox0PYCsFc0HHC5 TpXw== X-Google-DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=1e100.net; s=20161025; h=x-gm-message-state:mime-version:references:in-reply-to:from:date :message-id:subject:to:cc; bh=r8gamDvhcVApUzEi8B4eczMwzIgeevaZa9gUyXA1low=; b=RBaPeyojzrB7T26WrazJ6E/uMj3xYZ0+N26jp5AkBidwAV/khju4ti2nwz8r4l+uSg QKdqN5m183PgW90mAKrdjJFp9ZIZPd2+JelJo6g0Kkx4FjzkyqArRGMA9c3NbBsYRvcU Rz7/1aapafVX48ogVm/4Bq8wC0Q+oX1e/fQLA+GQvYn1R/YTYEY1FqC/4MTATBMCXiIe QSPK1NL9Y76hKeXtitGYrW6HBM26aWRM6qlDvU8RUH2V9eiOXbVHdx4FEiLkhGBeCpye dB/lOGIwGoctmzUuqizAEyoZWNtDRE+7nMOCE/sOKzWNj+cE35xt1SUCRUIc6Gv4G4y9 AFeQ== X-Gm-Message-State: AOAM533inytEq5fwIW7R81vEci5EgocyC7U+O1BMgLIyedcgW70PZp/M gBCNH0bjJJzZVWYSu3L370CEHlTVz8SDth5K65ngTQ== X-Google-Smtp-Source: ABdhPJw58PZFyBWG8/9qiNPPTHGBke6BLFQmcrHmShZvrD2BAdOklnYzbfaYn2mJDfK28LOPrDcdvaBl+DzQrsMBZA4= X-Received: by 2002:a25:4902:: with SMTP id w2mr7031253yba.42.1630786145741; Sat, 04 Sep 2021 13:09:05 -0700 (PDT) MIME-Version: 1.0 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> In-Reply-To: <20210904180825.GJ12964@bill-the-cat> From: Ilias Apalodimas Date: Sat, 4 Sep 2021 23:08:28 +0300 Message-ID: Subject: Re: Pull request for efi-2021-10-rc4 To: Tom Rini Cc: Heinrich Schuchardt , U-Boot Mailing List , Alexander Graf , Masahisa Kojima , AKASHI Takahiro , Simon Glass Content-Type: text/plain; charset="UTF-8" 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 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 wrote: > > >> >> > > >> >> > > >> >> Am 4. September 2021 15:01:11 MESZ schrieb Tom Rini : > > >> >> >On Sat, Sep 04, 2021 at 11:56:47AM +0200, Heinrich Schuchardt wrote: > > >> >> > > > >> >> >> Dear Tom, > > >> >> >> > > >> >> >> The following changes since commit 94509b79b13e69c209199af0757afbde8d2ebd6d: > > >> >> >> > > >> >> >> 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 1dfa494610c5469cc28cf1f8538abf4be6c00324: > > >> >> >> > > >> >> >> efi_loader: fix efi_tcg2_hash_log_extend_event() parameter check > > >> >> >> (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, DeployedMode > > >> >> >> efi_loader: correct determination of secure boot state > > >> >> >> > > >> >> >> Masahisa Kojima (3): > > >> >> >> efi_loader: add missing parameter check for EFI_TCG2_PROTOCOL api > > >> >> >> efi_loader: fix boot_service_capability_min calculation > > >> >> >> efi_loader: fix efi_tcg2_hash_log_extend_event() parameter 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+feuyRyWf0hNkf2triB4DR4UFBQ@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, the > > >> >non-rc ones for sure) are on a very regular schedule. External projects > > >> >may not depend on some feature introduced at -rcN unless they're willing > > >> >to accept that some changes could happen before release. > > >> > > > >> > > >> There is no value delivered by Simon's series. Neither does the image 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 in > > >before the patches were merged, but it was early enough in the release > > >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 out a > > >few days earlier. > > > > > >So yes, please merge Simon's revert, or post and merge new more minimal > > >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 be > > >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 have > 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. Regards /Ilias