efivar operation is updated when the tee_stmm_efi module is probed.
tee_stmm_efi module supports SetVariable runtime service,
but user needs to manually remount the efivarfs as RW to enable
the write access if the previous efivar operation does not support
SerVariable and efivarfs is mounted as read-only.
This commit notifies the update of efivar operation to
efivarfs subsystem, then drops SB_RDONLY flag if the efivar
operation supports SetVariable.
Signed-off-by: Masahisa Kojima <[email protected]>
---
drivers/firmware/efi/efi.c | 6 ++++++
drivers/firmware/efi/vars.c | 8 ++++++++
fs/efivarfs/super.c | 33 +++++++++++++++++++++++++++++++++
include/linux/efi.h | 8 ++++++++
4 files changed, 55 insertions(+)
diff --git a/drivers/firmware/efi/efi.c b/drivers/firmware/efi/efi.c
index d108cf03e19d..00494fcf16ba 100644
--- a/drivers/firmware/efi/efi.c
+++ b/drivers/firmware/efi/efi.c
@@ -32,6 +32,7 @@
#include <linux/ucs2_string.h>
#include <linux/memblock.h>
#include <linux/security.h>
+#include <linux/notifier.h>
#include <asm/early_ioremap.h>
@@ -184,6 +185,9 @@ static const struct attribute_group efi_subsys_attr_group = {
.is_visible = efi_attr_is_visible,
};
+struct blocking_notifier_head efivar_ops_nh;
+EXPORT_SYMBOL_GPL(efivar_ops_nh);
+
static struct efivars generic_efivars;
static struct efivar_operations generic_ops;
@@ -442,6 +446,8 @@ static int __init efisubsys_init(void)
platform_device_register_simple("efivars", 0, NULL, 0);
}
+ BLOCKING_INIT_NOTIFIER_HEAD(&efivar_ops_nh);
+
error = sysfs_create_group(efi_kobj, &efi_subsys_attr_group);
if (error) {
pr_err("efi: Sysfs attribute export failed with error %d.\n",
diff --git a/drivers/firmware/efi/vars.c b/drivers/firmware/efi/vars.c
index e9dc7116daf1..f654e6f6af87 100644
--- a/drivers/firmware/efi/vars.c
+++ b/drivers/firmware/efi/vars.c
@@ -63,6 +63,7 @@ int efivars_register(struct efivars *efivars,
const struct efivar_operations *ops)
{
int rv;
+ int event;
if (down_interruptible(&efivars_lock))
return -EINTR;
@@ -77,6 +78,13 @@ int efivars_register(struct efivars *efivars,
__efivars = efivars;
+ if (efivar_supports_writes())
+ event = EFIVAR_OPS_RDWR;
+ else
+ event = EFIVAR_OPS_RDONLY;
+
+ blocking_notifier_call_chain(&efivar_ops_nh, event, NULL);
+
pr_info("Registered efivars operations\n");
rv = 0;
out:
diff --git a/fs/efivarfs/super.c b/fs/efivarfs/super.c
index e028fafa04f3..0f6e4d223aea 100644
--- a/fs/efivarfs/super.c
+++ b/fs/efivarfs/super.c
@@ -14,11 +14,36 @@
#include <linux/slab.h>
#include <linux/magic.h>
#include <linux/statfs.h>
+#include <linux/notifier.h>
#include "internal.h"
LIST_HEAD(efivarfs_list);
+struct efivarfs_info {
+ struct super_block *sb;
+ struct notifier_block nb;
+};
+
+static struct efivarfs_info info;
+
+static int efivarfs_ops_notifier(struct notifier_block *nb, unsigned long event,
+ void *data)
+{
+ switch (event) {
+ case EFIVAR_OPS_RDONLY:
+ info.sb->s_flags |= SB_RDONLY;
+ break;
+ case EFIVAR_OPS_RDWR:
+ info.sb->s_flags &= ~SB_RDONLY;
+ break;
+ default:
+ return NOTIFY_DONE;
+ }
+
+ return NOTIFY_OK;
+}
+
static void efivarfs_evict_inode(struct inode *inode)
{
clear_inode(inode);
@@ -255,6 +280,12 @@ static int efivarfs_fill_super(struct super_block *sb, struct fs_context *fc)
if (!root)
return -ENOMEM;
+ info.sb = sb;
+ info.nb.notifier_call = efivarfs_ops_notifier;
+ err = blocking_notifier_chain_register(&efivar_ops_nh, &info.nb);
+ if (err)
+ return err;
+
INIT_LIST_HEAD(&efivarfs_list);
err = efivar_init(efivarfs_callback, (void *)sb, true, &efivarfs_list);
@@ -281,6 +312,8 @@ static int efivarfs_init_fs_context(struct fs_context *fc)
static void efivarfs_kill_sb(struct super_block *sb)
{
+ blocking_notifier_chain_unregister(&efivar_ops_nh, &info.nb);
+ info.sb = NULL;
kill_litter_super(sb);
if (!efivar_is_available())
diff --git a/include/linux/efi.h b/include/linux/efi.h
index 657f7e203374..2533e4f2547f 100644
--- a/include/linux/efi.h
+++ b/include/linux/efi.h
@@ -1350,6 +1350,14 @@ bool efi_config_table_is_usable(const efi_guid_t *guid, unsigned long table)
return xen_efi_config_table_is_usable(guid, table);
}
+/*
+ * efivar ops event type
+ */
+#define EFIVAR_OPS_RDONLY 0
+#define EFIVAR_OPS_RDWR 1
+
+extern struct blocking_notifier_head efivar_ops_nh;
+
void efivars_generic_ops_register(void);
void efivars_generic_ops_unregister(void);
--
2.30.2
On 22.06.23 10:51, Masahisa Kojima wrote:
> efivar operation is updated when the tee_stmm_efi module is probed.
> tee_stmm_efi module supports SetVariable runtime service,
> but user needs to manually remount the efivarfs as RW to enable
> the write access if the previous efivar operation does not support
> SerVariable and efivarfs is mounted as read-only.
>
> This commit notifies the update of efivar operation to
> efivarfs subsystem, then drops SB_RDONLY flag if the efivar
> operation supports SetVariable.
But it does not re-add it and prevents further requests to the TA (that
will only cause panics there) when the daemon terminates, does it?
Jan
--
Siemens AG, Technology
Competence Center Embedded Linux
Hi Kojima-san, Jan
On Thu, Jun 22, 2023 at 04:58:50PM +0200, Jan Kiszka wrote:
> On 22.06.23 10:51, Masahisa Kojima wrote:
> > efivar operation is updated when the tee_stmm_efi module is probed.
> > tee_stmm_efi module supports SetVariable runtime service,
> > but user needs to manually remount the efivarfs as RW to enable
> > the write access if the previous efivar operation does not support
> > SerVariable and efivarfs is mounted as read-only.
> >
> > This commit notifies the update of efivar operation to
> > efivarfs subsystem, then drops SB_RDONLY flag if the efivar
> > operation supports SetVariable.
>
> But it does not re-add it and prevents further requests to the TA (that
> will only cause panics there) when the daemon terminates, does it?
It doesn't, but I think I got a better way out. Even what you suggest won't
solve the problem entirely. For the sake of context
- The kernel decides between the RO/RW depending on the SetVariable ptr
- The stmm *module* registers and swaps the RT calls -- and the ptr is now
valid. Note here that the module probe function will run only if the
supplicant is running
- Once the module is inserted the filesystem will be remounted even without
the supplicant running, which would not trigger an oops, but an hard to
decipher error message from OP-TEE.
So even if we switch the permissions back to RO when the supplicant dies,
someone can still remount it as RW and trigger the same error.
Which got me thinking and staring the TEE subsystem a bit more. The
supplicant is backed by a /dev file, which naturally has .open() and
.release() callbacks. Why don't we leave the module perform the initial
setup -- e.g talk to StMM and make sure it's there, setup the necessary
buffers etc and defer the actual swapping of the efivar ops and the
filesystem permissions there? I might 'feel' a bit weird, but as I
mentioned the module probe function only runs if the supplicant is running
anyway
Cheers
/Ilias
>
> Jan
>
> --
> Siemens AG, Technology
> Competence Center Embedded Linux
>
Hi Ilias, Jan,
On Fri, 23 Jun 2023 at 03:56, Ilias Apalodimas
<[email protected]> wrote:
>
> Hi Kojima-san, Jan
>
> On Thu, Jun 22, 2023 at 04:58:50PM +0200, Jan Kiszka wrote:
> > On 22.06.23 10:51, Masahisa Kojima wrote:
> > > efivar operation is updated when the tee_stmm_efi module is probed.
> > > tee_stmm_efi module supports SetVariable runtime service,
> > > but user needs to manually remount the efivarfs as RW to enable
> > > the write access if the previous efivar operation does not support
> > > SerVariable and efivarfs is mounted as read-only.
> > >
> > > This commit notifies the update of efivar operation to
> > > efivarfs subsystem, then drops SB_RDONLY flag if the efivar
> > > operation supports SetVariable.
> >
> > But it does not re-add it and prevents further requests to the TA (that
> > will only cause panics there) when the daemon terminates, does it?
>
> It doesn't, but I think I got a better way out. Even what you suggest won't
> solve the problem entirely. For the sake of context
> - The kernel decides between the RO/RW depending on the SetVariable ptr
> - The stmm *module* registers and swaps the RT calls -- and the ptr is now
> valid. Note here that the module probe function will run only if the
> supplicant is running
> - Once the module is inserted the filesystem will be remounted even without
> the supplicant running, which would not trigger an oops, but an hard to
> decipher error message from OP-TEE.
>
> So even if we switch the permissions back to RO when the supplicant dies,
> someone can still remount it as RW and trigger the same error.
>
> Which got me thinking and staring the TEE subsystem a bit more. The
> supplicant is backed by a /dev file, which naturally has .open() and
> .release() callbacks. Why don't we leave the module perform the initial
> setup -- e.g talk to StMM and make sure it's there, setup the necessary
> buffers etc and defer the actual swapping of the efivar ops and the
> filesystem permissions there? I might 'feel' a bit weird, but as I
> mentioned the module probe function only runs if the supplicant is running
> anyway
I think we are discussing two issues.
1) efivar ops is not restored when the tee-supplicant daemon terminates.
The patch[1] sent by Sumit addresses this issue.
Thanks to this patch, 'remove' callback of tee_stmm_efi_driver is called
when the tee-supplicant daemon terminates, then restore the previous efivar ops
and SB_RDONLY flag if necessary.
2) cause panic when someone remounts the efivarfs as RW even if
SetVariable is not supported.
[1] https://lore.kernel.org/all/[email protected]/
Thanks,
Masahisa Kojima
>
> Cheers
> /Ilias
>
> >
> > Jan
> >
> > --
> > Siemens AG, Technology
> > Competence Center Embedded Linux
> >
Hi Kojima-san,
On Mon, 24 Jul 2023 at 05:53, Masahisa Kojima
<[email protected]> wrote:
>
> Hi Ilias, Jan,
>
> On Fri, 23 Jun 2023 at 03:56, Ilias Apalodimas
> <[email protected]> wrote:
> >
> > Hi Kojima-san, Jan
> >
> > On Thu, Jun 22, 2023 at 04:58:50PM +0200, Jan Kiszka wrote:
> > > On 22.06.23 10:51, Masahisa Kojima wrote:
> > > > efivar operation is updated when the tee_stmm_efi module is probed.
> > > > tee_stmm_efi module supports SetVariable runtime service,
> > > > but user needs to manually remount the efivarfs as RW to enable
> > > > the write access if the previous efivar operation does not support
> > > > SerVariable and efivarfs is mounted as read-only.
> > > >
> > > > This commit notifies the update of efivar operation to
> > > > efivarfs subsystem, then drops SB_RDONLY flag if the efivar
> > > > operation supports SetVariable.
> > >
> > > But it does not re-add it and prevents further requests to the TA (that
> > > will only cause panics there) when the daemon terminates, does it?
> >
> > It doesn't, but I think I got a better way out. Even what you suggest won't
> > solve the problem entirely. For the sake of context
> > - The kernel decides between the RO/RW depending on the SetVariable ptr
> > - The stmm *module* registers and swaps the RT calls -- and the ptr is now
> > valid. Note here that the module probe function will run only if the
> > supplicant is running
> > - Once the module is inserted the filesystem will be remounted even without
> > the supplicant running, which would not trigger an oops, but an hard to
> > decipher error message from OP-TEE.
> >
> > So even if we switch the permissions back to RO when the supplicant dies,
> > someone can still remount it as RW and trigger the same error.
> >
> > Which got me thinking and staring the TEE subsystem a bit more. The
> > supplicant is backed by a /dev file, which naturally has .open() and
> > .release() callbacks. Why don't we leave the module perform the initial
> > setup -- e.g talk to StMM and make sure it's there, setup the necessary
> > buffers etc and defer the actual swapping of the efivar ops and the
> > filesystem permissions there? I might 'feel' a bit weird, but as I
> > mentioned the module probe function only runs if the supplicant is running
> > anyway
>
> I think we are discussing two issues.
>
Yes
> 1) efivar ops is not restored when the tee-supplicant daemon terminates.
>
> The patch[1] sent by Sumit addresses this issue.
> Thanks to this patch, 'remove' callback of tee_stmm_efi_driver is called
> when the tee-supplicant daemon terminates, then restore the previous efivar ops
> and SB_RDONLY flag if necessary.
Ok but that didn't fix the original error Jan reported and I am not
sure about the patch status
>
> 2) cause panic when someone remounts the efivarfs as RW even if
> SetVariable is not supported.
Yes, this [0] is fixing that issue
[0] https://lore.kernel.org/linux-efi/[email protected]/
Thanks
/Ilias
>
> [1] https://lore.kernel.org/all/[email protected]/
>
> Thanks,
> Masahisa Kojima
>
> >
> > Cheers
> > /Ilias
> >
> > >
> > > Jan
> > >
> > > --
> > > Siemens AG, Technology
> > > Competence Center Embedded Linux
> > >
Hi Ilias,
On Mon, 24 Jul 2023 at 19:22, Ilias Apalodimas
<[email protected]> wrote:
>
> Hi Kojima-san,
>
> On Mon, 24 Jul 2023 at 05:53, Masahisa Kojima
> <[email protected]> wrote:
> >
> > Hi Ilias, Jan,
> >
> > On Fri, 23 Jun 2023 at 03:56, Ilias Apalodimas
> > <[email protected]> wrote:
> > >
> > > Hi Kojima-san, Jan
> > >
> > > On Thu, Jun 22, 2023 at 04:58:50PM +0200, Jan Kiszka wrote:
> > > > On 22.06.23 10:51, Masahisa Kojima wrote:
> > > > > efivar operation is updated when the tee_stmm_efi module is probed.
> > > > > tee_stmm_efi module supports SetVariable runtime service,
> > > > > but user needs to manually remount the efivarfs as RW to enable
> > > > > the write access if the previous efivar operation does not support
> > > > > SerVariable and efivarfs is mounted as read-only.
> > > > >
> > > > > This commit notifies the update of efivar operation to
> > > > > efivarfs subsystem, then drops SB_RDONLY flag if the efivar
> > > > > operation supports SetVariable.
> > > >
> > > > But it does not re-add it and prevents further requests to the TA (that
> > > > will only cause panics there) when the daemon terminates, does it?
> > >
> > > It doesn't, but I think I got a better way out. Even what you suggest won't
> > > solve the problem entirely. For the sake of context
> > > - The kernel decides between the RO/RW depending on the SetVariable ptr
> > > - The stmm *module* registers and swaps the RT calls -- and the ptr is now
> > > valid. Note here that the module probe function will run only if the
> > > supplicant is running
> > > - Once the module is inserted the filesystem will be remounted even without
> > > the supplicant running, which would not trigger an oops, but an hard to
> > > decipher error message from OP-TEE.
> > >
> > > So even if we switch the permissions back to RO when the supplicant dies,
> > > someone can still remount it as RW and trigger the same error.
> > >
> > > Which got me thinking and staring the TEE subsystem a bit more. The
> > > supplicant is backed by a /dev file, which naturally has .open() and
> > > .release() callbacks. Why don't we leave the module perform the initial
> > > setup -- e.g talk to StMM and make sure it's there, setup the necessary
> > > buffers etc and defer the actual swapping of the efivar ops and the
> > > filesystem permissions there? I might 'feel' a bit weird, but as I
> > > mentioned the module probe function only runs if the supplicant is running
> > > anyway
> >
> > I think we are discussing two issues.
> >
>
> Yes
>
> > 1) efivar ops is not restored when the tee-supplicant daemon terminates.
> >
> > The patch[1] sent by Sumit addresses this issue.
> > Thanks to this patch, 'remove' callback of tee_stmm_efi_driver is called
> > when the tee-supplicant daemon terminates, then restore the previous efivar ops
> > and SB_RDONLY flag if necessary.
>
> Ok but that didn't fix the original error Jan reported and I am not
> sure about the patch status
I think the patch is pending because the fTPM still causes panic when the system
shuts down.
https://lore.kernel.org/all/[email protected]/
This is fTPM specific issue and is unrelated to the tee-based
SetVariable runtime series itself.
>
> >
> > 2) cause panic when someone remounts the efivarfs as RW even if
> > SetVariable is not supported.
>
> Yes, this [0] is fixing that issue
Thank you, I will include this patch in the next submission.
Anyway, the GetVariable() runtime service backed by the U-Boot variable service
does not work from kernel v6.4.0, so I will investigate this issue.
Thanks,
Masahisa Kojima
>
> [0] https://lore.kernel.org/linux-efi/[email protected]/
> Thanks
> /Ilias
> >
> > [1] https://lore.kernel.org/all/[email protected]/
> >
> > Thanks,
> > Masahisa Kojima
> >
> > >
> > > Cheers
> > > /Ilias
> > >
> > > >
> > > > Jan
> > > >
> > > > --
> > > > Siemens AG, Technology
> > > > Competence Center Embedded Linux
> > > >
On Wed, 26 Jul 2023 at 13:49, Masahisa Kojima
<[email protected]> wrote:
>
> Hi Ilias,
>
> On Mon, 24 Jul 2023 at 19:22, Ilias Apalodimas
> <[email protected]> wrote:
> >
> > Hi Kojima-san,
> >
> > On Mon, 24 Jul 2023 at 05:53, Masahisa Kojima
> > <[email protected]> wrote:
> > >
> > > Hi Ilias, Jan,
> > >
> > > On Fri, 23 Jun 2023 at 03:56, Ilias Apalodimas
> > > <[email protected]> wrote:
> > > >
> > > > Hi Kojima-san, Jan
> > > >
> > > > On Thu, Jun 22, 2023 at 04:58:50PM +0200, Jan Kiszka wrote:
> > > > > On 22.06.23 10:51, Masahisa Kojima wrote:
> > > > > > efivar operation is updated when the tee_stmm_efi module is probed.
> > > > > > tee_stmm_efi module supports SetVariable runtime service,
> > > > > > but user needs to manually remount the efivarfs as RW to enable
> > > > > > the write access if the previous efivar operation does not support
> > > > > > SerVariable and efivarfs is mounted as read-only.
> > > > > >
> > > > > > This commit notifies the update of efivar operation to
> > > > > > efivarfs subsystem, then drops SB_RDONLY flag if the efivar
> > > > > > operation supports SetVariable.
> > > > >
> > > > > But it does not re-add it and prevents further requests to the TA (that
> > > > > will only cause panics there) when the daemon terminates, does it?
> > > >
> > > > It doesn't, but I think I got a better way out. Even what you suggest won't
> > > > solve the problem entirely. For the sake of context
> > > > - The kernel decides between the RO/RW depending on the SetVariable ptr
> > > > - The stmm *module* registers and swaps the RT calls -- and the ptr is now
> > > > valid. Note here that the module probe function will run only if the
> > > > supplicant is running
> > > > - Once the module is inserted the filesystem will be remounted even without
> > > > the supplicant running, which would not trigger an oops, but an hard to
> > > > decipher error message from OP-TEE.
> > > >
> > > > So even if we switch the permissions back to RO when the supplicant dies,
> > > > someone can still remount it as RW and trigger the same error.
> > > >
> > > > Which got me thinking and staring the TEE subsystem a bit more. The
> > > > supplicant is backed by a /dev file, which naturally has .open() and
> > > > .release() callbacks. Why don't we leave the module perform the initial
> > > > setup -- e.g talk to StMM and make sure it's there, setup the necessary
> > > > buffers etc and defer the actual swapping of the efivar ops and the
> > > > filesystem permissions there? I might 'feel' a bit weird, but as I
> > > > mentioned the module probe function only runs if the supplicant is running
> > > > anyway
> > >
> > > I think we are discussing two issues.
> > >
> >
> > Yes
> >
> > > 1) efivar ops is not restored when the tee-supplicant daemon terminates.
> > >
> > > The patch[1] sent by Sumit addresses this issue.
> > > Thanks to this patch, 'remove' callback of tee_stmm_efi_driver is called
> > > when the tee-supplicant daemon terminates, then restore the previous efivar ops
> > > and SB_RDONLY flag if necessary.
> >
> > Ok but that didn't fix the original error Jan reported and I am not
> > sure about the patch status
>
> I think the patch is pending because the fTPM still causes panic when the system
> shuts down.
> https://lore.kernel.org/all/[email protected]/
>
> This is fTPM specific issue and is unrelated to the tee-based
> SetVariable runtime series itself.
>
> >
> > >
> > > 2) cause panic when someone remounts the efivarfs as RW even if
> > > SetVariable is not supported.
> >
> > Yes, this [0] is fixing that issue
>
> Thank you, I will include this patch in the next submission.
>
> Anyway, the GetVariable() runtime service backed by the U-Boot variable service
> does not work from kernel v6.4.0, so I will investigate this issue.
I found that the QueryVariableInfo EFI API is required since this patch[2].
Current U-Boot does not support QueryVariableInfo runtime service.
Anyway this is not directly related to this series.
After efivar ops is replaced by the tee-based one, variable access works fine.
[2] https://lore.kernel.org/all/[email protected]/
Thanks,
Masahisa Kojima
>
> Thanks,
> Masahisa Kojima
>
> >
> > [0] https://lore.kernel.org/linux-efi/[email protected]/
> > Thanks
> > /Ilias
> > >
> > > [1] https://lore.kernel.org/all/[email protected]/
> > >
> > > Thanks,
> > > Masahisa Kojima
> > >
> > > >
> > > > Cheers
> > > > /Ilias
> > > >
> > > > >
> > > > > Jan
> > > > >
> > > > > --
> > > > > Siemens AG, Technology
> > > > > Competence Center Embedded Linux
> > > > >