( Good Lord, I hate doing string manipulation in C )
On Wed, 20 Apr, at 03:25:32PM, Laszlo Ersek wrote:
>
> So, "len" does not include the room for the terminating NUL-byte here.
> When "len" is passed to ucs2_as_utf8(), with the proposed patch applied,
> a NUL byte will be produced in "name", but it will be at the price of a
> genuine character from the input variable name.
Right, and this is a problem because we're trying to keep the names
consistent between efivarfs and the EFI variable data. Force
NUL-terminating the string is wrong, because if you have no room for
the NUL the caller should check for that. Sadly none do.
On the flip-side, passing around non-NUL terminated strings is just
begging for these kinds of issues to come up.
The fact is that the callers of ucs2_as_utf8() are passing it the
wrong 'len' argument. We want a NUL-terminated utf8 string and we're
passing a NUL-terminated ucs2 string. We should tell ucs2_as_utf8() it
has enough room to copy the NUL.
Wouldn't this work (minus the return value checking)?
---
diff --git a/drivers/firmware/efi/vars.c b/drivers/firmware/efi/vars.c
index 0ac594c0a234..13a837c70e90 100644
--- a/drivers/firmware/efi/vars.c
+++ b/drivers/firmware/efi/vars.c
@@ -235,13 +235,12 @@ efivar_validate(efi_guid_t vendor, efi_char16_t *var_name, u8 *data,
unsigned long utf8_size;
u8 *utf8_name;
- utf8_size = ucs2_utf8size(var_name);
- utf8_name = kmalloc(utf8_size + 1, GFP_KERNEL);
+ utf8_size = ucs2_utf8size(var_name) + 1;
+ utf8_name = kmalloc(utf8_size, GFP_KERNEL);
if (!utf8_name)
return false;
ucs2_as_utf8(utf8_name, var_name, utf8_size);
- utf8_name[utf8_size] = '\0';
for (i = 0; variable_validate[i].name[0] != '\0'; i++) {
const char *name = variable_validate[i].name;
@@ -250,7 +249,7 @@ efivar_validate(efi_guid_t vendor, efi_char16_t *var_name, u8 *data,
if (efi_guidcmp(vendor, variable_validate[i].vendor))
continue;
- if (variable_matches(utf8_name, utf8_size+1, name, &match)) {
+ if (variable_matches(utf8_name, utf8_size, name, &match)) {
if (variable_validate[i].validate == NULL)
break;
kfree(utf8_name);
diff --git a/fs/efivarfs/super.c b/fs/efivarfs/super.c
index dd029d13ea61..be5a02721b41 100644
--- a/fs/efivarfs/super.c
+++ b/fs/efivarfs/super.c
@@ -136,7 +136,7 @@ static int efivarfs_callback(efi_char16_t *name16, efi_guid_t vendor,
if (!name)
goto fail;
- ucs2_as_utf8(name, entry->var.VariableName, len);
+ ucs2_as_utf8(name, entry->var.VariableName, len + 1);
if (efivar_variable_is_removable(entry->var.VendorGuid, name, len))
is_removable = true;
--
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
