Received-SPF: pass (google.com: domain of linux-kernel-owner@vger.kernel.org designates 2620:137:e000::1:20 as permitted sender) client-ip=2620:137:e000::1:20;
MIME-Version: 1.0
References: <20230526010748.1222-1-masahisa.kojima@linaro.org>
 <CAC_iWj+E7-XK6dCeSn4205K0O3EZCLxCaC+adu-14ST6sdudfA@mail.gmail.com>
 <76da826f-b608-6add-5401-6de818b180e3@siemens.com> <CAFA6WYPCDRjFzsUMU=SNzEt88nT7Fcm1eOFL8z4HiQO+=2JeVA@mail.gmail.com>
 <cc6bd203-83ea-c247-0986-7fec6f327ee8@siemens.com> <CAC_iWjKZNHJxq4VMFnV7oQngwBBCQveh=s34u1LZ59YUqViPbw@mail.gmail.com>
 <CAC_iWjJMv68yLC606SBhMmBYkR4wVC8SvUcPvNM=RX_qL=9Bvw@mail.gmail.com>
 <b9b8c1d3-fc8e-df94-d12b-a9e3debf3418@siemens.com> <CAC_iWj+cP4RfDNu_n-ZOp7A62W34drLpPszN_hrkqF_aPTLtMg@mail.gmail.com>
 <871ece13-7d6e-44d4-3bda-317658202f6f@siemens.com> <CAC_iWjKgCJWgKU8tC3Nfn-0CgwGhw89B3JpTgsjkjDDOcWZEdw@mail.gmail.com>
 <CAMj1kXGbXdbDr6DbyuziSFuxMgAimjNnkhqy8C-S5AbxdnixuQ@mail.gmail.com>
 <CAC_iWjJ0uw_p9=6C+U+q8BNQgnDBgbDpcu8s8c80oB25i9y77w@mail.gmail.com>
 <3eb9bf84-4cf0-6cfe-9ad3-f7eef7d775fb@siemens.com> <CAC_iWjLdmmQ4+BVt_mOP5Ht9jDHQqoVV+-ATR-nDrmG37raGjQ@mail.gmail.com>
 <da7d3871-3b2f-1172-62a8-70f6856e4968@siemens.com>
In-Reply-To: <da7d3871-3b2f-1172-62a8-70f6856e4968@siemens.com>
From:   Ilias Apalodimas <ilias.apalodimas@linaro.org>
Date:   Fri, 9 Jun 2023 11:00:06 +0300
Message-ID: <CAC_iWj+67o2LF5ULffaDPUQ9FXZYpqK96Sb1_EhY40ppgk=X2A@mail.gmail.com>
Subject: Re: [PATCH v5 3/3] efi: Add tee-based EFI variable driver
To:     Jan Kiszka <jan.kiszka@siemens.com>
Cc:     Ard Biesheuvel <ardb@kernel.org>,
        Sumit Garg <sumit.garg@linaro.org>,
        Masahisa Kojima <masahisa.kojima@linaro.org>,
        Jens Wiklander <jens.wiklander@linaro.org>,
        linux-kernel@vger.kernel.org, op-tee@lists.trustedfirmware.org,
        Johan Hovold <johan+linaro@kernel.org>,
        Maxime Coquelin <mcoquelin.stm32@gmail.com>,
        Alexandre Torgue <alexandre.torgue@foss.st.com>,
        linux-efi@vger.kernel.org,
        linux-stm32@st-md-mailman.stormreply.com,
        linux-arm-kernel@lists.infradead.org,
        "Su, Bao Cheng (RC-CN DF FA R&D)" <baocheng.su@siemens.com>
Content-Type: text/plain; charset="UTF-8"
Precedence: bulk

Hi Jan,

On Fri, 9 Jun 2023 at 09:40, Jan Kiszka <jan.kiszka@siemens.com> wrote:
>
> On 09.06.23 08:34, Ilias Apalodimas wrote:
> > Hi Jan,
> >
> > [...]
> >
> >>> --- a/drivers/firmware/efi/vars.c
> >>> +++ b/drivers/firmware/efi/vars.c
> >>> @@ -123,7 +123,7 @@ EXPORT_SYMBOL_GPL(efivars_unregister);
> >>>
> >>>  bool efivar_supports_writes(void)
> >>>  {
> >>> -       return __efivars && __efivars->ops->set_variable;
> >>> +       return __efivars && __efivars->ops->set_variable != set_variable_int;
> >>>  }
> >>>  EXPORT_SYMBOL_GPL(efivar_supports_writes);
> >>>
> >>> diff --git a/fs/efivarfs/super.c b/fs/efivarfs/super.c
> >>> index e028fafa04f3..e40b5c4c5106 100644
> >>> --- a/fs/efivarfs/super.c
> >>> +++ b/fs/efivarfs/super.c
> >>> @@ -242,9 +242,6 @@ static int efivarfs_fill_super(struct super_block
> >>> *sb, struct fs_context *fc)
> >>>         sb->s_d_op              = &efivarfs_d_ops;
> >>>         sb->s_time_gran         = 1;
> >>>
> >>> -       if (!efivar_supports_writes())
> >>> -               sb->s_flags |= SB_RDONLY;
> >>> -
> >>>         inode = efivarfs_get_inode(sb, NULL, S_IFDIR | 0755, 0, true);
> >>>         if (!inode)
> >>>                 return -ENOMEM;
> >>> diff --git a/include/linux/efi.h b/include/linux/efi.h
> >>> index 58d1c271d3b0..ec0ac6ef50a3 100644
> >>> --- a/include/linux/efi.h
> >>> +++ b/include/linux/efi.h
> >>> @@ -1084,6 +1084,10 @@ int efivars_register(struct efivars *efivars,
> >>>                      const struct efivar_operations *ops);
> >>>  int efivars_unregister(struct efivars *efivars);
> >>>
> >>> +efi_status_t set_variable_int(efi_char16_t *name, efi_guid_t *vendor,
> >>> +                             u32 attributes, unsigned long data_size,
> >>> +                             void *data);
> >>> +
> >>>  void efivars_generic_ops_register(void);
> >>>  void efivars_generic_ops_unregister(void);
> >>>
> >>> Thanks
> >>> /Ilias
> >>
> >> As just written in my other reply: The root cause is the dependency on
> >> tee-supplicant daemon. That needs to be resolved, and then also r/w
> >> mounting will just work.
> >
> > That's partially true.  If we solve the dependency your problem will
> > go away only if everything gets compiled as built in.  But if you have
> > them as modules there's still a chance you mount the efivarfs before
> > installing all the modules.  In that case, you'll end up with the same
> > problem no?
>
> Obviously, this will need proper probing of the TA services in the
> proper order so that the STMM driver is pulled in before efivarfs gets used.
>
> >
> > That's why I think this patch (or a variation of it) is useful.  It
> > solves the kernel panic you are seeing if you remount the efivarfs as
> > RW and It unifies the way the kernel responds to userspace no matter
> > what the firmware does with its setvariableRT service.
>
> I'm not against fixes crashes, but the r/w issue is a different thing IMHO.

Fair enough, but if we want to fix the crash only I think there's a
better way to do it.
I'll cook some patches and send  them over

Thanks
/Ilias
>
> Jan
>
> --
> Siemens AG, Technology
> Competence Center Embedded Linux
>