Re: [PATCH 2/5] efi: Use ucs2_as_utf8 in efivarfs instead of open coding a bad version

From: H. Peter Anvin
Date: Thu Feb 18 2016 - 00:35:00 EST


On February 12, 2016 3:27:09 AM PST, Matt Fleming <matt@xxxxxxxxxxxxxxxxxxx> wrote:
>From: Peter Jones <pjones@xxxxxxxxxx>
>
>Translate EFI's UCS-2 variable names to UTF-8 instead of just assuming
>all variable names fit in ASCII.
>
>Signed-off-by: Peter Jones <pjones@xxxxxxxxxx>
>Acked-by: Matthew Garrett <mjg59@xxxxxxxxxx>
>Tested-by: "Lee, Chun-Yi" <jlee@xxxxxxxx>
>Signed-off-by: Matt Fleming <matt@xxxxxxxxxxxxxxxxxxx>
>---
> drivers/firmware/efi/efivars.c | 30 +++++++++++-------------------
> fs/efivarfs/super.c | 7 +++----
> 2 files changed, 14 insertions(+), 23 deletions(-)
>
>diff --git a/drivers/firmware/efi/efivars.c
>b/drivers/firmware/efi/efivars.c
>index 756eca8c4cf8..f4ff8abc5f3e 100644
>--- a/drivers/firmware/efi/efivars.c
>+++ b/drivers/firmware/efi/efivars.c
>@@ -540,38 +540,30 @@ static ssize_t efivar_delete(struct file *filp,
>struct kobject *kobj,
> static int
> efivar_create_sysfs_entry(struct efivar_entry *new_var)
> {
>- int i, short_name_size;
>+ int short_name_size;
> char *short_name;
>- unsigned long variable_name_size;
>- efi_char16_t *variable_name;
>+ unsigned long utf8_name_size;
>+ efi_char16_t *variable_name = new_var->var.VariableName;
> int ret;
>
>- variable_name = new_var->var.VariableName;
>- variable_name_size = ucs2_strlen(variable_name) *
>sizeof(efi_char16_t);
>-
> /*
>- * Length of the variable bytes in ASCII, plus the '-' separator,
>+ * Length of the variable bytes in UTF8, plus the '-' separator,
> * plus the GUID, plus trailing NUL
> */
>- short_name_size = variable_name_size / sizeof(efi_char16_t)
>- + 1 + EFI_VARIABLE_GUID_LEN + 1;
>-
>- short_name = kzalloc(short_name_size, GFP_KERNEL);
>+ utf8_name_size = ucs2_utf8size(variable_name);
>+ short_name_size = utf8_name_size + 1 + EFI_VARIABLE_GUID_LEN + 1;
>
>+ short_name = kmalloc(short_name_size, GFP_KERNEL);
> if (!short_name)
> return -ENOMEM;
>
>- /* Convert Unicode to normal chars (assume top bits are 0),
>- ala UTF-8 */
>- for (i=0; i < (int)(variable_name_size / sizeof(efi_char16_t)); i++)
>{
>- short_name[i] = variable_name[i] & 0xFF;
>- }
>+ ucs2_as_utf8(short_name, variable_name, short_name_size);
>+
> /* This is ugly, but necessary to separate one vendor's
> private variables from another's. */
>-
>- *(short_name + strlen(short_name)) = '-';
>+ short_name[utf8_name_size] = '-';
> efi_guid_to_str(&new_var->var.VendorGuid,
>- short_name + strlen(short_name));
>+ short_name + utf8_name_size + 1);
>
> new_var->kobj.kset = efivars_kset;
>
>diff --git a/fs/efivarfs/super.c b/fs/efivarfs/super.c
>index b8a564f29107..8651ac28ec0d 100644
>--- a/fs/efivarfs/super.c
>+++ b/fs/efivarfs/super.c
>@@ -118,7 +118,7 @@ static int efivarfs_callback(efi_char16_t *name16,
>efi_guid_t vendor,
> struct dentry *dentry, *root = sb->s_root;
> unsigned long size = 0;
> char *name;
>- int len, i;
>+ int len;
> int err = -ENOMEM;
>
> entry = kzalloc(sizeof(*entry), GFP_KERNEL);
>@@ -128,15 +128,14 @@ static int efivarfs_callback(efi_char16_t
>*name16, efi_guid_t vendor,
> memcpy(entry->var.VariableName, name16, name_size);
> memcpy(&(entry->var.VendorGuid), &vendor, sizeof(efi_guid_t));
>
>- len = ucs2_strlen(entry->var.VariableName);
>+ len = ucs2_utf8size(entry->var.VariableName);
>
> /* name, plus '-', plus GUID, plus NUL*/
> name = kmalloc(len + 1 + EFI_VARIABLE_GUID_LEN + 1, GFP_KERNEL);
> if (!name)
> goto fail;
>
>- for (i = 0; i < len; i++)
>- name[i] = entry->var.VariableName[i] & 0xFF;
>+ ucs2_as_utf8(name, entry->var.VariableName, len);
>
> name[len] = '-';
>

However, I think we should treat this "ucs2" as utf16, because sooner or later someone will enter utf16 characters.
--
Sent from my Android device with K-9 Mail. Please excuse brevity and formatting.