(Cc'ing some other folks that might be interested)
On Fri, 2012-10-19 at 15:16 +0800, Jeremy Kerr wrote:
> At present, the handling of GUIDs in efivar file names isn't consistent.
> We use GUID_LEN in some places, and 38 in others (GUID_LEN plus
> separator), and implicitly use the presence of the trailing NUL.
>
> This change removes the trailing NUL from GUID_LEN, so that we're
> explicitly adding it when required. We also replace magic numbers
> with GUID_LEN, and clarify the comments where appropriate.
>
> We also fix the allocation size in efivar_create_sysfs_entry, where
> we're allocating one byte too much, due to counting the trailing NUL
> twice - once when calculating short_name_size, and once in the kzalloc.
>
> Signed-off-by: Jeremy Kerr <jeremy.kerr@xxxxxxxxxxxxx>
>
> ---
> drivers/firmware/efivars.c | 29 +++++++++++++++++++++--------
> 1 file changed, 21 insertions(+), 8 deletions(-)
>
> diff --git a/drivers/firmware/efivars.c b/drivers/firmware/efivars.c
> index a86eb55..ce0d6fa 100644
> --- a/drivers/firmware/efivars.c
> +++ b/drivers/firmware/efivars.c
> @@ -95,7 +95,12 @@ MODULE_LICENSE("GPL");
> MODULE_VERSION(EFIVARS_VERSION);
>
> #define DUMP_NAME_LEN 52
> -#define GUID_LEN 37
> +
> +/*
> + * Length of a GUID string (strlen("aaaaaaaa-bbbb-cccc-dddd-eeeeeeeeeeee"))
> + * not including trailing NUL
> + */
> +#define GUID_LEN 36
>
> /*
> * The maximum size of VariableName + Data = 1024
> @@ -876,7 +881,9 @@ static int efivarfs_create(struct inode *dir, struct dentry *dentry,
> struct efivar_entry *var;
> int namelen, i = 0, err = 0;
>
> - if (dentry->d_name.len < 38)
> + /* We need a GUID, plus at least one letter for the variable name,
> + * plus the '-' separator */
> + if (dentry->d_name.len < GUID_LEN + 2)
> return -EINVAL;
>
> inode = efivarfs_get_inode(dir->i_sb, dir, mode, 0);
> @@ -889,7 +896,8 @@ static int efivarfs_create(struct inode *dir, struct dentry *dentry,
> goto out;
> }
>
> - namelen = dentry->d_name.len - GUID_LEN;
> + /* length of the variable name itself: remove GUID and separator */
> + namelen = dentry->d_name.len - GUID_LEN - 1;
>
> efivarfs_hex_to_guid(dentry->d_name.name + namelen + 1,
> &var->var.VendorGuid);
> @@ -983,8 +991,8 @@ int efivarfs_fill_super(struct super_block *sb, void *data, int silent)
>
> len = utf16_strlen(entry->var.VariableName);
>
> - /* GUID plus trailing NULL */
> - name = kmalloc(len + 38, GFP_ATOMIC);
> + /* name, plus '-', plus GUID, plus NUL*/
> + name = kmalloc(len + 1 + GUID_LEN + 1, GFP_ATOMIC);
> if (!name)
> goto fail;
>
> @@ -995,7 +1003,7 @@ int efivarfs_fill_super(struct super_block *sb, void *data, int silent)
>
> efi_guid_unparse(&entry->var.VendorGuid, name + len + 1);
>
> - name[len+GUID_LEN] = '\0';
> + name[len+GUID_LEN+1] = '\0';
>
> inode = efivarfs_get_inode(efivarfs_sb, root->d_inode,
> S_IFREG | 0644, 0);
> @@ -1424,11 +1432,16 @@ efivar_create_sysfs_entry(struct efivars *efivars,
> efi_char16_t *variable_name,
> efi_guid_t *vendor_guid)
> {
> - int i, short_name_size = variable_name_size / sizeof(efi_char16_t) + 38;
> + int i, short_name_size;
> char *short_name;
> struct efivar_entry *new_efivar;
>
> - short_name = kzalloc(short_name_size + 1, GFP_KERNEL);
> + /* Length of the variable bytes in ASCII, plus the '-' separator,
> + * plus the GUID, plus trailing NUL */
> + short_name_size = variable_name_size / sizeof(efi_char_16_t)
> + + 1 + GUID_LEN + 1;
> +
> + short_name = kzalloc(short_name_size, GFP_KERNEL);
> new_efivar = kzalloc(sizeof(struct efivar_entry), GFP_KERNEL);
>
> if (!short_name || !new_efivar) {
--
To unsubscribe from this list: send the line "unsubscribe linux-efi" in
the body of a message to majordomo@xxxxxxxxxxxxxxx
More majordomo info at http://vger.kernel.org/majordomo-info.html
