linux-kernel - Re: [PATCH v5 3/3] efi: Add tee-based EFI variable driver

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: <CAC_iWjLdmmQ4+BVt_mOP5Ht9jDHQqoVV+-ATR-nDrmG37raGjQ@mail.gmail.com>
Date:   Fri, 9 Jun 2023 09:34:45 +0300
From:   Ilias Apalodimas <ilias.apalodimas@...aro.org>
To:     Jan Kiszka <jan.kiszka@...mens.com>
Cc:     Ard Biesheuvel <ardb@...nel.org>,
        Sumit Garg <sumit.garg@...aro.org>,
        Masahisa Kojima <masahisa.kojima@...aro.org>,
        Jens Wiklander <jens.wiklander@...aro.org>,
        linux-kernel@...r.kernel.org, op-tee@...ts.trustedfirmware.org,
        Johan Hovold <johan+linaro@...nel.org>,
        Maxime Coquelin <mcoquelin.stm32@...il.com>,
        Alexandre Torgue <alexandre.torgue@...s.st.com>,
        linux-efi@...r.kernel.org,
        linux-stm32@...md-mailman.stormreply.com,
        linux-arm-kernel@...ts.infradead.org,
        "Su, Bao Cheng (RC-CN DF FA R&D)" <baocheng.su@...mens.com>
Subject: Re: [PATCH v5 3/3] efi: Add tee-based EFI variable driver

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?

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.

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