[PATCH] efivarfs: allow creation of zero length files

[PATCH] efivarfs: allow creation of zero length files

[James Bottomley]

Temporarily allow the creation of zero length files in efivarfs so the
Linux Vendor Firmware Service can continue to operate. This hack should
be reverted as soon as the LVFS mechanisms for updating firmware have
been fixed.

LVFS has been coded to open a firmware file, close it, remove the
immutable bit and write to it.  Since commit 908af31f4896 ("efivarfs:
fix error on write to new variable leaving remnants") this behaviour
results in the first close removing the file which causes the second
write to fail.  To allow LVFS to keep working code up an indicator of
size 1 if a write fails and only remove the file on that condition (so
create at zero size is allowed).

Tested-by: Richard Hughes <richard@hughsie.com>
Signed-off-by: James Bottomley <James.Bottomley@HansenPartnership.com>
---
This should be reverted in the 6.15 release

 fs/efivarfs/file.c | 10 ++++++----
 1 file changed, 6 insertions(+), 4 deletions(-)

diff --git a/fs/efivarfs/file.c b/fs/efivarfs/file.c
index cb1b6d0c3454..c294a8fc566d 100644
--- a/fs/efivarfs/file.c
+++ b/fs/efivarfs/file.c
@@ -57,10 +57,11 @@ static ssize_t efivarfs_file_write(struct file
*file,
 
 	if (bytes == -ENOENT) {
 		/*
-		 * zero size signals to release that the write deleted
-		 * the variable
+		 * FIXME: temporary workaround for fwupdate, signal
+		 * failed write with a 1 to keep created but not
+		 * written files
 		 */
-		i_size_write(inode, 0);
+		i_size_write(inode, 1);
 	} else {
 		i_size_write(inode, datasize + sizeof(attributes));
 		inode_set_mtime_to_ts(inode,
inode_set_ctime_current(inode));
@@ -124,7 +125,8 @@ static int efivarfs_file_release(struct inode
*inode, struct file *file)
 	struct efivar_entry *var = inode->i_private;
 
 	inode_lock(inode);
-	var->removed = (--var->open_count == 0 && i_size_read(inode)
== 0);
+	/* FIXME: temporary work around for fwupdate */
+	var->removed = (--var->open_count == 0 && i_size_read(inode)
== 1);
 	inode_unlock(inode);
 
 	if (var->removed)
-- 
2.43.0

Re: [PATCH] efivarfs: allow creation of zero length files

[Ard Biesheuvel]

On Wed, 26 Feb 2025 at 22:48, James Bottomley
<James.Bottomley@hansenpartnership.com> wrote:
>
> Temporarily allow the creation of zero length files in efivarfs so the
> Linux Vendor Firmware Service can continue to operate. This hack should
> be reverted as soon as the LVFS mechanisms for updating firmware have
> been fixed.
>
> LVFS has been coded to open a firmware file, close it, remove the
> immutable bit and write to it.  Since commit 908af31f4896 ("efivarfs:
> fix error on write to new variable leaving remnants") this behaviour
> results in the first close removing the file which causes the second
> write to fail.  To allow LVFS to keep working code up an indicator of
> size 1 if a write fails and only remove the file on that condition (so
> create at zero size is allowed).
>
> Tested-by: Richard Hughes <richard@hughsie.com>
> Signed-off-by: James Bottomley <James.Bottomley@HansenPartnership.com>
> ---
> This should be reverted in the 6.15 release
>

Thanks James. Mind resending this with a MUA that doesn't mangle patches?


>  fs/efivarfs/file.c | 10 ++++++----
>  1 file changed, 6 insertions(+), 4 deletions(-)
>
> diff --git a/fs/efivarfs/file.c b/fs/efivarfs/file.c
> index cb1b6d0c3454..c294a8fc566d 100644
> --- a/fs/efivarfs/file.c
> +++ b/fs/efivarfs/file.c
> @@ -57,10 +57,11 @@ static ssize_t efivarfs_file_write(struct file
> *file,
>
>         if (bytes == -ENOENT) {
>                 /*
> -                * zero size signals to release that the write deleted
> -                * the variable
> +                * FIXME: temporary workaround for fwupdate, signal
> +                * failed write with a 1 to keep created but not
> +                * written files
>                  */
> -               i_size_write(inode, 0);
> +               i_size_write(inode, 1);
>         } else {
>                 i_size_write(inode, datasize + sizeof(attributes));
>                 inode_set_mtime_to_ts(inode,
> inode_set_ctime_current(inode));
> @@ -124,7 +125,8 @@ static int efivarfs_file_release(struct inode
> *inode, struct file *file)
>         struct efivar_entry *var = inode->i_private;
>
>         inode_lock(inode);
> -       var->removed = (--var->open_count == 0 && i_size_read(inode)
> == 0);
> +       /* FIXME: temporary work around for fwupdate */
> +       var->removed = (--var->open_count == 0 && i_size_read(inode)
> == 1);
>         inode_unlock(inode);
>
>         if (var->removed)
> --
> 2.43.0
>
>

Re: [PATCH] efivarfs: allow creation of zero length files

[Richard Hughes]

Also, a tiny nit -- "fwupd" is the program that actually deploys the capsule updates -- the "LVFS" is just the webservice that vendors upload firmware to. Thanks.

Richard.

On Thursday, 27 February 2025 at 07:54, Ard Biesheuvel <ardb@kernel.org> wrote:

> 
> 
> On Wed, 26 Feb 2025 at 22:48, James Bottomley
> James.Bottomley@hansenpartnership.com wrote:
> 
> > Temporarily allow the creation of zero length files in efivarfs so the
> > Linux Vendor Firmware Service can continue to operate. This hack should
> > be reverted as soon as the LVFS mechanisms for updating firmware have
> > been fixed.
> > 
> > LVFS has been coded to open a firmware file, close it, remove the
> > immutable bit and write to it. Since commit 908af31f4896 ("efivarfs:
> > fix error on write to new variable leaving remnants") this behaviour
> > results in the first close removing the file which causes the second
> > write to fail. To allow LVFS to keep working code up an indicator of
> > size 1 if a write fails and only remove the file on that condition (so
> > create at zero size is allowed).
> > 
> > Tested-by: Richard Hughes richard@hughsie.com
> > Signed-off-by: James Bottomley James.Bottomley@HansenPartnership.com
> > ---
> > This should be reverted in the 6.15 release
> 
> 
> Thanks James. Mind resending this with a MUA that doesn't mangle patches?
> 
> > fs/efivarfs/file.c | 10 ++++++----
> > 1 file changed, 6 insertions(+), 4 deletions(-)
> > 
> > diff --git a/fs/efivarfs/file.c b/fs/efivarfs/file.c
> > index cb1b6d0c3454..c294a8fc566d 100644
> > --- a/fs/efivarfs/file.c
> > +++ b/fs/efivarfs/file.c
> > @@ -57,10 +57,11 @@ static ssize_t efivarfs_file_write(struct file
> > *file,
> > 
> > if (bytes == -ENOENT) {
> > /*
> > - * zero size signals to release that the write deleted
> > - * the variable
> > + * FIXME: temporary workaround for fwupdate, signal
> > + * failed write with a 1 to keep created but not
> > + * written files
> > */
> > - i_size_write(inode, 0);
> > + i_size_write(inode, 1);
> > } else {
> > i_size_write(inode, datasize + sizeof(attributes));
> > inode_set_mtime_to_ts(inode,
> > inode_set_ctime_current(inode));
> > @@ -124,7 +125,8 @@ static int efivarfs_file_release(struct inode
> > *inode, struct file *file)
> > struct efivar_entry *var = inode->i_private;
> > 
> > inode_lock(inode);
> > - var->removed = (--var->open_count == 0 && i_size_read(inode)
> > == 0);
> > + /* FIXME: temporary work around for fwupdate */
> > + var->removed = (--var->open_count == 0 && i_size_read(inode)
> > == 1);
> > inode_unlock(inode);
> > 
> > if (var->removed)
> > --
> > 2.43.0

Re: [PATCH] efivarfs: allow creation of zero length files

[Ard Biesheuvel]

> On Thursday, 27 February 2025 at 07:54, Ard Biesheuvel <ardb@kernel.org> wrote:
>

> > Thanks James. Mind resending this with a MUA that doesn't mangle patches?
> >

On Thu, 27 Feb 2025 at 09:40, Richard Hughes <richard@hughsie.com> wrote:
>
> Also, a tiny nit -- "fwupd" is the program that actually deploys the capsule updates -- the "LVFS" is just the webservice that vendors upload firmware to. Thanks.
>

Never mind - I fixed up the patch by hand, and replaced LVFS with
fwupd in the commit message.

Unless more time is needed, I will queue the revert of this patch for v6.15

And for the record, I checked the libefivar sources, and there the
problem does not appear to exist.