If an EFI variable is created by an open and write but returns an
error in set_variable, it isn't removed but hangs around in efivarfs
with invalid inode attributes. This happens because the entry is
created in efivarfs_create but the EFI set_variable problem isn't
discovered until efivarfs_file_write(). Fix by having set_variable
failure in efivarfs_file_write() check if the variable existed before
or is newly created and remove it again on the latter. The signal for
a newly created variable is that var.Attributes is empty. This cannot
happen for a real variable because one of the flags in
EFI_VARIABLE_MASK must be set.
Signed-off-by: James Bottomley <James.Bottomley@xxxxxxxxxxxxxxxxxxxxx>
---
fs/efivarfs/file.c | 31 ++++++++++++++++++++++---------
1 file changed, 22 insertions(+), 9 deletions(-)
diff --git a/fs/efivarfs/file.c b/fs/efivarfs/file.c
index 41b2f5a7239c..5ef88c577e03 100644
--- a/fs/efivarfs/file.c
+++ b/fs/efivarfs/file.c
@@ -23,18 +23,23 @@ static ssize_t efivarfs_file_write(struct file *file,
ssize_t bytes;
bool set = false;
+ bytes = -EINVAL;
if (count < sizeof(attributes))
- return -EINVAL;
+ goto err;
+ bytes = -EFAULT;
if (copy_from_user(&attributes, userbuf, sizeof(attributes)))
- return -EFAULT;
+ goto err;
+ bytes = -EINVAL;
if (attributes & ~(EFI_VARIABLE_MASK))
- return -EINVAL;
+ goto err;
data = memdup_user(userbuf + sizeof(attributes), datasize);
- if (IS_ERR(data))
- return PTR_ERR(data);
+ if (IS_ERR(data)) {
+ bytes = PTR_ERR(data);
+ goto err;
+ }
bytes = efivar_entry_set_get_size(var, attributes, &datasize,
data, &set);
@@ -45,10 +50,7 @@ static ssize_t efivarfs_file_write(struct file *file,
}
if (bytes == -ENOENT) {
- drop_nlink(inode);
- d_delete(file->f_path.dentry);
- dput(file->f_path.dentry);
- kfree(var);
+ var->var.Attributes = 0;
} else {
inode_lock(inode);
i_size_write(inode, datasize + sizeof(attributes));
@@ -60,6 +62,17 @@ static ssize_t efivarfs_file_write(struct file *file,
out:
kfree(data);
+err:
+ if (var->var.Attributes == 0) {
+ /*
+ * variable got deleted or didn't exist before we
+ * tried to set it
+ */
+ drop_nlink(inode);
+ d_delete(file->f_path.dentry);
+ dput(file->f_path.dentry);
+ kfree(var);
+ }
return bytes;
}
--
2.35.3
