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 53ae25bbb6ac..d2eec5ed8e5e 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>
@@ -187,6 +188,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;
@@ -427,6 +431,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 603bba2d6437..17cd628b5c42 100644
--- a/include/linux/efi.h
+++ b/include/linux/efi.h
@@ -1365,6 +1365,14 @@ bool efi_config_table_is_usable(const efi_guid_t *guid, unsigned long table)
umode_t efi_attr_is_visible(struct kobject *kobj, struct attribute *attr, int n);
+/*
+ * 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
Kojima-san
Apologies for the late reply, I just found some to test this.
On Sun, 6 Aug 2023 at 19:55, Masahisa Kojima <[email protected]> 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.
The RO->RW transition works fine and I did manage to test basic stuff
like setting up efibootmgr options. IIUC the RW->RO should be covered
by this patchset [0] ?
Wouldn't it be better to detect that the tee device of the supplicant
closes and use that to switch the permissions? I get why we need that
for the TPM, the entire subsystem needs to send TPM commands *before*
the supplicant dies. But this is not needed for the EFI variables
case, we could just remount the FS as RO the moment the supplicant
dies.
[0] https://lore.kernel.org/all/[email protected]/
Regards
/Ilias
>
> 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 53ae25bbb6ac..d2eec5ed8e5e 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>
>
> @@ -187,6 +188,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;
>
> @@ -427,6 +431,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 603bba2d6437..17cd628b5c42 100644
> --- a/include/linux/efi.h
> +++ b/include/linux/efi.h
> @@ -1365,6 +1365,14 @@ bool efi_config_table_is_usable(const efi_guid_t *guid, unsigned long table)
>
> umode_t efi_attr_is_visible(struct kobject *kobj, struct attribute *attr, int n);
>
> +/*
> + * 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
>
Hi Ilias,
On Thu, 12 Oct 2023 at 02:00, Ilias Apalodimas
<[email protected]> wrote:
>
> Kojima-san
> Apologies for the late reply, I just found some to test this.
>
> On Sun, 6 Aug 2023 at 19:55, Masahisa Kojima <[email protected]> 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.
>
> The RO->RW transition works fine and I did manage to test basic stuff
> like setting up efibootmgr options. IIUC the RW->RO should be covered
> by this patchset [0] ?
Yes.
> Wouldn't it be better to detect that the tee device of the supplicant
> closes and use that to switch the permissions? I get why we need that
> for the TPM, the entire subsystem needs to send TPM commands *before*
> the supplicant dies. But this is not needed for the EFI variables
> case, we could just remount the FS as RO the moment the supplicant
> dies.
OK, I will add the patch to restore the efivars generic ops when
tee-supplicant stops.
Thanks,
Masahisa Kojima
>
> [0] https://lore.kernel.org/all/[email protected]/
>
> Regards
> /Ilias
> >
> > 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 53ae25bbb6ac..d2eec5ed8e5e 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>
> >
> > @@ -187,6 +188,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;
> >
> > @@ -427,6 +431,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 603bba2d6437..17cd628b5c42 100644
> > --- a/include/linux/efi.h
> > +++ b/include/linux/efi.h
> > @@ -1365,6 +1365,14 @@ bool efi_config_table_is_usable(const efi_guid_t *guid, unsigned long table)
> >
> > umode_t efi_attr_is_visible(struct kobject *kobj, struct attribute *attr, int n);
> >
> > +/*
> > + * 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
> >
Hi Ilias,
On Wed, 11 Oct 2023 at 22:30, Ilias Apalodimas
<[email protected]> wrote:
>
> Kojima-san
> Apologies for the late reply, I just found some to test this.
>
> On Sun, 6 Aug 2023 at 19:55, Masahisa Kojima <[email protected]> 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.
>
> The RO->RW transition works fine and I did manage to test basic stuff
> like setting up efibootmgr options. IIUC the RW->RO should be covered
> by this patchset [0] ?
Yeah.
> Wouldn't it be better to detect that the tee device of the supplicant
> closes and use that to switch the permissions? I get why we need that
> for the TPM, the entire subsystem needs to send TPM commands *before*
> the supplicant dies. But this is not needed for the EFI variables
> case, we could just remount the FS as RO the moment the supplicant
> dies.
As we discussed offline, we should have a unified approach to notify
kernel TEE client drivers. So the approach implemented as part of [0]
should address the needs for fTPM as well as EFI.
-Sumit
>
> [0] https://lore.kernel.org/all/[email protected]/
>
> Regards
> /Ilias
> >
> > 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 53ae25bbb6ac..d2eec5ed8e5e 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>
> >
> > @@ -187,6 +188,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;
> >
> > @@ -427,6 +431,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 603bba2d6437..17cd628b5c42 100644
> > --- a/include/linux/efi.h
> > +++ b/include/linux/efi.h
> > @@ -1365,6 +1365,14 @@ bool efi_config_table_is_usable(const efi_guid_t *guid, unsigned long table)
> >
> > umode_t efi_attr_is_visible(struct kobject *kobj, struct attribute *attr, int n);
> >
> > +/*
> > + * 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
> >
Hi Sumit
On Fri, 13 Oct 2023 at 09:53, Sumit Garg <[email protected]> wrote:
>
> Hi Ilias,
>
> On Wed, 11 Oct 2023 at 22:30, Ilias Apalodimas
> <[email protected]> wrote:
> >
> > Kojima-san
> > Apologies for the late reply, I just found some to test this.
> >
> > On Sun, 6 Aug 2023 at 19:55, Masahisa Kojima <[email protected]> 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.
> >
> > The RO->RW transition works fine and I did manage to test basic stuff
> > like setting up efibootmgr options. IIUC the RW->RO should be covered
> > by this patchset [0] ?
>
> Yeah.
>
> > Wouldn't it be better to detect that the tee device of the supplicant
> > closes and use that to switch the permissions? I get why we need that
> > for the TPM, the entire subsystem needs to send TPM commands *before*
> > the supplicant dies. But this is not needed for the EFI variables
> > case, we could just remount the FS as RO the moment the supplicant
> > dies.
>
> As we discussed offline, we should have a unified approach to notify
> kernel TEE client drivers. So the approach implemented as part of [0]
> should address the needs for fTPM as well as EFI.
Ideally yes, we should have a unified approach. But this is a bit
different IMHO. In the majority of the cases, the supplicant goes
away, we lose access to storage and that's the only thing we care
about. Only the TPM subsystem is 'special' because it has to perform
a shutdown of the device as well. On top of that, I think we should
try to avoid the kernel depending on userspace apps as much as
possible. I think it's best if we support both of these and add
documentation on why this is happening. Would it be hard to have a
combination of both of your patches?
Thanks
/Ilias
>
> -Sumit
>
> >
> > [0] https://lore.kernel.org/all/[email protected]/
> >
> > Regards
> > /Ilias
> > >
> > > 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 53ae25bbb6ac..d2eec5ed8e5e 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>
> > >
> > > @@ -187,6 +188,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;
> > >
> > > @@ -427,6 +431,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 603bba2d6437..17cd628b5c42 100644
> > > --- a/include/linux/efi.h
> > > +++ b/include/linux/efi.h
> > > @@ -1365,6 +1365,14 @@ bool efi_config_table_is_usable(const efi_guid_t *guid, unsigned long table)
> > >
> > > umode_t efi_attr_is_visible(struct kobject *kobj, struct attribute *attr, int n);
> > >
> > > +/*
> > > + * 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 Fri, 13 Oct 2023 at 20:56, Ilias Apalodimas
<[email protected]> wrote:
>
> Hi Sumit
>
> On Fri, 13 Oct 2023 at 09:53, Sumit Garg <[email protected]> wrote:
> >
> > Hi Ilias,
> >
> > On Wed, 11 Oct 2023 at 22:30, Ilias Apalodimas
> > <[email protected]> wrote:
> > >
> > > Kojima-san
> > > Apologies for the late reply, I just found some to test this.
> > >
> > > On Sun, 6 Aug 2023 at 19:55, Masahisa Kojima <[email protected]> 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.
> > >
> > > The RO->RW transition works fine and I did manage to test basic stuff
> > > like setting up efibootmgr options. IIUC the RW->RO should be covered
> > > by this patchset [0] ?
> >
> > Yeah.
> >
> > > Wouldn't it be better to detect that the tee device of the supplicant
> > > closes and use that to switch the permissions? I get why we need that
> > > for the TPM, the entire subsystem needs to send TPM commands *before*
> > > the supplicant dies. But this is not needed for the EFI variables
> > > case, we could just remount the FS as RO the moment the supplicant
> > > dies.
> >
> > As we discussed offline, we should have a unified approach to notify
> > kernel TEE client drivers. So the approach implemented as part of [0]
> > should address the needs for fTPM as well as EFI.
>
> Ideally yes, we should have a unified approach. But this is a bit
> different IMHO. In the majority of the cases, the supplicant goes
> away, we lose access to storage and that's the only thing we care
> about.
What do you think about in-flight secure storage operations while the
tee-supplicant goes away? They can fail abruptly if the tee-supplicant
goes away without waiting for them to complete. IMO, the graceful
handling should be that the tee-supplicant should be alive when the
dependent devices are being closed.
For an EFI variable store case as well isn't that a possible scenario?
> Only the TPM subsystem is 'special' because it has to perform
> a shutdown of the device as well. On top of that, I think we should
> try to avoid the kernel depending on userspace apps as much as
> possible.
There is only tee-supplicant user-space dependency which has to be
handled carefully. I am very much up for RPMB fastpath via kernel and
get rid of this dependency as well.
> I think it's best if we support both of these and add
> documentation on why this is happening. Would it be hard to have a
> combination of both of your patches?
>
It won't be hard but there can be corner cases as mentioned above.
-Sumit
> Thanks
> /Ilias
>
> >
> > -Sumit
> >
> > >
> > > [0] https://lore.kernel.org/all/[email protected]/
> > >
> > > Regards
> > > /Ilias
> > > >
> > > > 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 53ae25bbb6ac..d2eec5ed8e5e 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>
> > > >
> > > > @@ -187,6 +188,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;
> > > >
> > > > @@ -427,6 +431,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 603bba2d6437..17cd628b5c42 100644
> > > > --- a/include/linux/efi.h
> > > > +++ b/include/linux/efi.h
> > > > @@ -1365,6 +1365,14 @@ bool efi_config_table_is_usable(const efi_guid_t *guid, unsigned long table)
> > > >
> > > > umode_t efi_attr_is_visible(struct kobject *kobj, struct attribute *attr, int n);
> > > >
> > > > +/*
> > > > + * 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
> > > >
Hi Sumit,
[...]
> > >
> > > > Wouldn't it be better to detect that the tee device of the supplicant
> > > > closes and use that to switch the permissions? I get why we need that
> > > > for the TPM, the entire subsystem needs to send TPM commands *before*
> > > > the supplicant dies. But this is not needed for the EFI variables
> > > > case, we could just remount the FS as RO the moment the supplicant
> > > > dies.
> > >
> > > As we discussed offline, we should have a unified approach to notify
> > > kernel TEE client drivers. So the approach implemented as part of [0]
> > > should address the needs for fTPM as well as EFI.
> >
> > Ideally yes, we should have a unified approach. But this is a bit
> > different IMHO. In the majority of the cases, the supplicant goes
> > away, we lose access to storage and that's the only thing we care
> > about.
>
> What do you think about in-flight secure storage operations while the
> tee-supplicant goes away? They can fail abruptly if the tee-supplicant
> goes away without waiting for them to complete. IMO, the graceful
> handling should be that the tee-supplicant should be alive when the
> dependent devices are being closed.
>
> For an EFI variable store case as well isn't that a possible scenario?
I think it is, but that would involve issuing a write and then killing
the supplicant immediately.
But that would lead to a weird state as well with the graceful exit.
IOW we don't block until the operation is finished, if you want to
shoot yourself in the foot you can start a write and then unbind. That
would lead to a partial EFI variable being written as well no?
>
> > Only the TPM subsystem is 'special' because it has to perform
> > a shutdown of the device as well. On top of that, I think we should
> > try to avoid the kernel depending on userspace apps as much as
> > possible.
>
> There is only tee-supplicant user-space dependency which has to be
> handled carefully. I am very much up for RPMB fastpath via kernel and
> get rid of this dependency as well.
Yes, that is without a doubt our best solution. Jens is already
working on that, but the supplicant has an entire ecosystem built
around it. There might be cases where using the in-kernel replacement
might not be possible.
>
> > I think it's best if we support both of these and add
> > documentation on why this is happening. Would it be hard to have a
> > combination of both of your patches?
> >
>
> It won't be hard but there can be corner cases as mentioned above.
TBH I am fine with merging it as-is. Both cases aren't ideal so it's
mostly a matter of documenting them properly. I can update the docs
once we decide what to pull and mention the caveats in detail.
I am mostly interested to see what Jan is thinking since he pointed
out the behavior initially
Thanks
/Ilias
>
> -Sumit
>
> > Thanks
> > /Ilias
> >
> > >
> > > -Sumit
> > >
> > > >
> > > > [0] https://lore.kernel.org/all/[email protected]/
> > > >
> > > > Regards
> > > > /Ilias
> > > > >
> > > > > 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 53ae25bbb6ac..d2eec5ed8e5e 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>
> > > > >
> > > > > @@ -187,6 +188,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;
> > > > >
> > > > > @@ -427,6 +431,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 603bba2d6437..17cd628b5c42 100644
> > > > > --- a/include/linux/efi.h
> > > > > +++ b/include/linux/efi.h
> > > > > @@ -1365,6 +1365,14 @@ bool efi_config_table_is_usable(const efi_guid_t *guid, unsigned long table)
> > > > >
> > > > > umode_t efi_attr_is_visible(struct kobject *kobj, struct attribute *attr, int n);
> > > > >
> > > > > +/*
> > > > > + * 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
> > > > >