| From 47f531e8ba3bc3901a0c493f4252826c41dea1a1 Mon Sep 17 00:00:00 2001 |
| From: Matt Fleming <matt.fleming@intel.com> |
| Date: Thu, 31 Jan 2013 19:02:03 +0000 |
| Subject: efivarfs: Validate filenames much more aggressively |
| |
| From: Matt Fleming <matt.fleming@intel.com> |
| |
| commit 47f531e8ba3bc3901a0c493f4252826c41dea1a1 upstream. |
| |
| The only thing that efivarfs does to enforce a valid filename is |
| ensure that the name isn't too short. We need to strongly sanitise any |
| filenames, not least because variable creation is delayed until |
| efivarfs_file_write(), which means we can't rely on the firmware to |
| inform us of an invalid name, because if the file is never written to |
| we'll never know it's invalid. |
| |
| Perform a couple of steps before agreeing to create a new file, |
| |
| * hex_to_bin() returns a value indicating whether or not it was able |
| to convert its arguments to a binary representation - we should |
| check it. |
| |
| * Ensure that the GUID portion of the filename is the correct length |
| and format. |
| |
| * The variable name portion of the filename needs to be at least one |
| character in size. |
| |
| Reported-by: Lingzhu Xiang <lxiang@redhat.com> |
| Cc: Matthew Garrett <mjg59@srcf.ucam.org> |
| Cc: Jeremy Kerr <jeremy.kerr@canonical.com> |
| Cc: Al Viro <viro@zeniv.linux.org.uk> |
| Signed-off-by: Matt Fleming <matt.fleming@intel.com> |
| Signed-off-by: Greg Kroah-Hartman <gregkh@linuxfoundation.org> |
| |
| --- |
| drivers/firmware/efivars.c | 49 ++++++++++++++++++++++++++++++++++++++++----- |
| 1 file changed, 44 insertions(+), 5 deletions(-) |
| |
| --- a/drivers/firmware/efivars.c |
| +++ b/drivers/firmware/efivars.c |
| @@ -79,6 +79,7 @@ |
| #include <linux/device.h> |
| #include <linux/slab.h> |
| #include <linux/pstore.h> |
| +#include <linux/ctype.h> |
| |
| #include <linux/fs.h> |
| #include <linux/ramfs.h> |
| @@ -900,6 +901,48 @@ static struct inode *efivarfs_get_inode( |
| return inode; |
| } |
| |
| +/* |
| + * Return true if 'str' is a valid efivarfs filename of the form, |
| + * |
| + * VariableName-12345678-1234-1234-1234-1234567891bc |
| + */ |
| +static bool efivarfs_valid_name(const char *str, int len) |
| +{ |
| + static const char dashes[GUID_LEN] = { |
| + [8] = 1, [13] = 1, [18] = 1, [23] = 1 |
| + }; |
| + const char *s = str + len - GUID_LEN; |
| + int i; |
| + |
| + /* |
| + * We need a GUID, plus at least one letter for the variable name, |
| + * plus the '-' separator |
| + */ |
| + if (len < GUID_LEN + 2) |
| + return false; |
| + |
| + /* GUID should be right after the first '-' */ |
| + if (s - 1 != strchr(str, '-')) |
| + return false; |
| + |
| + /* |
| + * Validate that 's' is of the correct format, e.g. |
| + * |
| + * 12345678-1234-1234-1234-123456789abc |
| + */ |
| + for (i = 0; i < GUID_LEN; i++) { |
| + if (dashes[i]) { |
| + if (*s++ != '-') |
| + return false; |
| + } else { |
| + if (!isxdigit(*s++)) |
| + return false; |
| + } |
| + } |
| + |
| + return true; |
| +} |
| + |
| static void efivarfs_hex_to_guid(const char *str, efi_guid_t *guid) |
| { |
| guid->b[0] = hex_to_bin(str[6]) << 4 | hex_to_bin(str[7]); |
| @@ -928,11 +971,7 @@ static int efivarfs_create(struct inode |
| struct efivar_entry *var; |
| int namelen, i = 0, err = 0; |
| |
| - /* |
| - * We need a GUID, plus at least one letter for the variable name, |
| - * plus the '-' separator |
| - */ |
| - if (dentry->d_name.len < GUID_LEN + 2) |
| + if (!efivarfs_valid_name(dentry->d_name.name, dentry->d_name.len)) |
| return -EINVAL; |
| |
| inode = efivarfs_get_inode(dir->i_sb, dir, mode, 0); |