diff mbox series

[3/6] efivarfs: make variable_is_present use dcache lookup

Message ID 20241210170224.19159-4-James.Bottomley@HansenPartnership.com
State New
Headers show
Series convert efivarfs to manage object data correctly | expand

Commit Message

James Bottomley Dec. 10, 2024, 5:02 p.m. UTC 
Instead of searching the variable entry list for a variable, use the
dcache lookup functions to find it instead.  Also add an efivarfs_
prefix to the function now it is no longer static.

Signed-off-by: James Bottomley <James.Bottomley@HansenPartnership.com>
---
 fs/efivarfs/internal.h |  4 ++++
 fs/efivarfs/super.c    | 20 ++++++++++++++++++++
 fs/efivarfs/vars.c     | 26 ++------------------------
 3 files changed, 26 insertions(+), 24 deletions(-)

Comments

James Bottomley Dec. 23, 2024, 9:44 p.m. UTC | #1 
On Mon, 2024-12-23 at 20:20 +0000, Al Viro wrote:
> On Tue, Dec 10, 2024 at 12:02:21PM -0500, James Bottomley wrote:
> > Instead of searching the variable entry list for a variable, use
> > the
> > dcache lookup functions to find it instead.  Also add an efivarfs_
> > prefix to the function now it is no longer static.
> 
> > +bool efivarfs_variable_is_present(efi_char16_t *variable_name,
> > +                                 efi_guid_t *vendor, void *data)
> > +{
> > +       char *name = efivar_get_utf8name(variable_name, vendor);
> > +       struct super_block *sb = data;
> > +       struct dentry *dentry;
> > +       struct qstr qstr;
> > +
> > +       if (!name)
> > +               return true;
> > +
> > +       qstr.name = name;
> > +       qstr.len = strlen(name);
> > +       dentry = d_hash_and_lookup(sb->s_root, &qstr);
> > +       kfree(name);
> > +       if (dentry)
> > +               dput(dentry);
> 
> If that ever gets called with efivarfs_valid_name(name, strlen(name))
> being false, that's going to oops...

Well for the current use case that can't happen because a) that check
is gone from efivarfs_d_hash and b) the name is constructed to pass
this check anyway.  But I'm guessing you mean I should be doing

if (dentry && !IS_ERR(dentry))
   dput(dentry);

?

Regards,

James
diff mbox series

Patch

diff --git a/fs/efivarfs/internal.h b/fs/efivarfs/internal.h
index 4b7330b90958..84a36e6fb653 100644
--- a/fs/efivarfs/internal.h
+++ b/fs/efivarfs/internal.h
@@ -56,6 +56,8 @@  bool efivar_validate(efi_guid_t vendor, efi_char16_t *var_name, u8 *data,
 bool efivar_variable_is_removable(efi_guid_t vendor, const char *name,
 				  size_t len);
 char *efivar_get_utf8name(const efi_char16_t *name16, efi_guid_t *vendor);
+bool efivarfs_variable_is_present(efi_char16_t *variable_name,
+				  efi_guid_t *vendor, void *data);
 
 extern const struct file_operations efivarfs_file_operations;
 extern const struct inode_operations efivarfs_dir_inode_operations;
@@ -64,4 +66,6 @@  extern struct inode *efivarfs_get_inode(struct super_block *sb,
 			const struct inode *dir, int mode, dev_t dev,
 			bool is_removable);
 
+
+
 #endif /* EFIVAR_FS_INTERNAL_H */
diff --git a/fs/efivarfs/super.c b/fs/efivarfs/super.c
index b22441f7f7c6..dc3870ae784b 100644
--- a/fs/efivarfs/super.c
+++ b/fs/efivarfs/super.c
@@ -181,6 +181,26 @@  static struct dentry *efivarfs_alloc_dentry(struct dentry *parent, char *name)
 	return ERR_PTR(-ENOMEM);
 }
 
+bool efivarfs_variable_is_present(efi_char16_t *variable_name,
+				  efi_guid_t *vendor, void *data)
+{
+	char *name = efivar_get_utf8name(variable_name, vendor);
+	struct super_block *sb = data;
+	struct dentry *dentry;
+	struct qstr qstr;
+
+	if (!name)
+		return true;
+
+	qstr.name = name;
+	qstr.len = strlen(name);
+	dentry = d_hash_and_lookup(sb->s_root, &qstr);
+	kfree(name);
+	if (dentry)
+		dput(dentry);
+	return dentry != NULL;
+}
+
 static int efivarfs_callback(efi_char16_t *name16, efi_guid_t vendor,
 			     unsigned long name_size, void *data,
 			     struct list_head *list)
diff --git a/fs/efivarfs/vars.c b/fs/efivarfs/vars.c
index 7a07b767e2cc..f6380fdbe173 100644
--- a/fs/efivarfs/vars.c
+++ b/fs/efivarfs/vars.c
@@ -313,28 +313,6 @@  efivar_variable_is_removable(efi_guid_t vendor, const char *var_name,
 	return found;
 }
 
-static bool variable_is_present(efi_char16_t *variable_name, efi_guid_t *vendor,
-				struct list_head *head)
-{
-	struct efivar_entry *entry, *n;
-	unsigned long strsize1, strsize2;
-	bool found = false;
-
-	strsize1 = ucs2_strsize(variable_name, EFI_VAR_NAME_LEN);
-	list_for_each_entry_safe(entry, n, head, list) {
-		strsize2 = ucs2_strsize(entry->var.VariableName, EFI_VAR_NAME_LEN);
-		if (strsize1 == strsize2 &&
-			!memcmp(variable_name, &(entry->var.VariableName),
-				strsize2) &&
-			!efi_guidcmp(entry->var.VendorGuid,
-				*vendor)) {
-			found = true;
-			break;
-		}
-	}
-	return found;
-}
-
 /*
  * Returns the size of variable_name, in bytes, including the
  * terminating NULL character, or variable_name_size if no NULL
@@ -439,8 +417,8 @@  int efivar_init(int (*func)(efi_char16_t *, efi_guid_t, unsigned long, void *,
 			 * we'll ever see a different variable name,
 			 * and may end up looping here forever.
 			 */
-			if (variable_is_present(variable_name, &vendor_guid,
-						head)) {
+			if (efivarfs_variable_is_present(variable_name,
+							 &vendor_guid, data)) {
 				dup_variable_bug(variable_name, &vendor_guid,
 						 variable_name_size);
 				status = EFI_NOT_FOUND;