diff mbox series

[3/3] efivarfs: fix incorrect variable creation

Message ID 20241208183415.21181-4-James.Bottomley@HansenPartnership.com
State New
Headers show
Series efivarfs: bug fixes | expand

Commit Message

James Bottomley Dec. 8, 2024, 6:34 p.m. UTC
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@HansenPartnership.com>
---
 fs/efivarfs/file.c | 31 ++++++++++++++++++++++---------
 1 file changed, 22 insertions(+), 9 deletions(-)
diff mbox series

Patch

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;
 }