On Tue, Dec 10, 2024 at 9:03 AM James Bottomley
<James.Bottomley@xxxxxxxxxxxxxxxxxxxxx> wrote:
> 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);
>
> +
> +
Unnecessary
> #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;
Why is this true? I understand the previous implementation would have
hit a null dereference trying to calculate strsize1 on null, so this
isn't worse, but if we considered its length to be 0, it would not be
found.
> +
> + 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;
> --
> 2.35.3
>
>
--
-Dionna Glaze, PhD, CISSP, CCSP (she/her)
