From: Matthew Garrett Git-commit: 5d9db883761ad1bc2245fd3018715549b974203d Patch-mainline: v3.8 References: bnc#808680 Target: openSUSE 12.3 The existing EFI variables code only supports variables of up to 1024 bytes. This limitation existed in version 0.99 of the EFI specification, but was removed before any full releases. Since variables can now be larger than a single page, sysfs isn't the best interface for this. So, instead, let's add a filesystem. Variables can be read, written and created, with the first 4 bytes of each variable representing its UEFI attributes. The create() method doesn't actually commit to flash since zero-length variables can't exist per-spec. Updates from Jeremy Kerr . Signed-off-by: Matthew Garrett Signed-off-by: Jeremy Kerr Signed-off-by: Matt Fleming Acked-by: Lee, Chun-Yi --- drivers/firmware/efivars.c | 384 ++++++++++++++++++++++++++++++++++++++++++++- include/linux/efi.h | 5 2 files changed, 383 insertions(+), 6 deletions(-) --- a/drivers/firmware/efivars.c +++ b/drivers/firmware/efivars.c @@ -80,6 +80,10 @@ #include #include +#include +#include +#include + #include #define EFIVARS_VERSION "0.08" @@ -91,6 +95,7 @@ MODULE_LICENSE("GPL"); MODULE_VERSION(EFIVARS_VERSION); #define DUMP_NAME_LEN 52 +#define GUID_LEN 37 /* * The maximum size of VariableName + Data = 1024 @@ -108,7 +113,6 @@ struct efi_variable { __u32 Attributes; } __attribute__((packed)); - struct efivar_entry { struct efivars *efivars; struct efi_variable var; @@ -122,6 +126,9 @@ struct efivar_attribute { ssize_t (*store)(struct efivar_entry *entry, const char *buf, size_t count); }; +static struct efivars __efivars; +static struct efivar_operations ops; + #define PSTORE_EFI_ATTRIBUTES \ (EFI_VARIABLE_NON_VOLATILE | \ EFI_VARIABLE_BOOTSERVICE_ACCESS | \ @@ -629,14 +636,380 @@ static struct kobj_type efivar_ktype = { .default_attrs = def_attrs, }; -static struct pstore_info efi_pstore_info; - static inline void efivar_unregister(struct efivar_entry *var) { kobject_put(&var->kobj); } +static int efivarfs_file_open(struct inode *inode, struct file *file) +{ + file->private_data = inode->i_private; + return 0; +} + +static ssize_t efivarfs_file_write(struct file *file, + const char __user *userbuf, size_t count, loff_t *ppos) +{ + struct efivar_entry *var = file->private_data; + struct efivars *efivars; + efi_status_t status; + void *data; + u32 attributes; + struct inode *inode = file->f_mapping->host; + int datasize = count - sizeof(attributes); + + if (count < sizeof(attributes)) + return -EINVAL; + + data = kmalloc(datasize, GFP_KERNEL); + + if (!data) + return -ENOMEM; + + efivars = var->efivars; + + if (copy_from_user(&attributes, userbuf, sizeof(attributes))) { + count = -EFAULT; + goto out; + } + + if (attributes & ~(EFI_VARIABLE_MASK)) { + count = -EINVAL; + goto out; + } + + if (copy_from_user(data, userbuf + sizeof(attributes), datasize)) { + count = -EFAULT; + goto out; + } + + if (validate_var(&var->var, data, datasize) == false) { + count = -EINVAL; + goto out; + } + + status = efivars->ops->set_variable(var->var.VariableName, + &var->var.VendorGuid, + attributes, datasize, + data); + + switch (status) { + case EFI_SUCCESS: + mutex_lock(&inode->i_mutex); + i_size_write(inode, count); + mutex_unlock(&inode->i_mutex); + break; + case EFI_INVALID_PARAMETER: + count = -EINVAL; + break; + case EFI_OUT_OF_RESOURCES: + count = -ENOSPC; + break; + case EFI_DEVICE_ERROR: + count = -EIO; + break; + case EFI_WRITE_PROTECTED: + count = -EROFS; + break; + case EFI_SECURITY_VIOLATION: + count = -EACCES; + break; + case EFI_NOT_FOUND: + count = -ENOENT; + break; + default: + count = -EINVAL; + break; + } +out: + kfree(data); + + return count; +} + +static ssize_t efivarfs_file_read(struct file *file, char __user *userbuf, + size_t count, loff_t *ppos) +{ + struct efivar_entry *var = file->private_data; + struct efivars *efivars = var->efivars; + efi_status_t status; + unsigned long datasize = 0; + u32 attributes; + void *data; + ssize_t size; + + status = efivars->ops->get_variable(var->var.VariableName, + &var->var.VendorGuid, + &attributes, &datasize, NULL); + + if (status != EFI_BUFFER_TOO_SMALL) + return 0; + + data = kmalloc(datasize + 4, GFP_KERNEL); + + if (!data) + return 0; + + status = efivars->ops->get_variable(var->var.VariableName, + &var->var.VendorGuid, + &attributes, &datasize, + (data + 4)); + + if (status != EFI_SUCCESS) + return 0; + + memcpy(data, &attributes, 4); + size = simple_read_from_buffer(userbuf, count, ppos, + data, datasize + 4); + kfree(data); + + return size; +} + +static void efivarfs_evict_inode(struct inode *inode) +{ + clear_inode(inode); +} + +static const struct super_operations efivarfs_ops = { + .statfs = simple_statfs, + .drop_inode = generic_delete_inode, + .evict_inode = efivarfs_evict_inode, + .show_options = generic_show_options, +}; + +static struct super_block *efivarfs_sb; + +static const struct inode_operations efivarfs_dir_inode_operations; + +static const struct file_operations efivarfs_file_operations = { + .open = efivarfs_file_open, + .read = efivarfs_file_read, + .write = efivarfs_file_write, + .llseek = default_llseek, +}; + +static struct inode *efivarfs_get_inode(struct super_block *sb, + const struct inode *dir, int mode, dev_t dev) +{ + struct inode *inode = new_inode(sb); + + if (inode) { + inode->i_ino = get_next_ino(); + inode->i_uid = inode->i_gid = 0; + inode->i_mode = mode; + inode->i_atime = inode->i_mtime = inode->i_ctime = CURRENT_TIME; + switch (mode & S_IFMT) { + case S_IFREG: + inode->i_fop = &efivarfs_file_operations; + break; + case S_IFDIR: + inode->i_op = &efivarfs_dir_inode_operations; + inode->i_fop = &simple_dir_operations; + inc_nlink(inode); + break; + } + } + return inode; +} + +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]); + guid->b[1] = hex_to_bin(str[4]) << 4 | hex_to_bin(str[5]); + guid->b[2] = hex_to_bin(str[2]) << 4 | hex_to_bin(str[3]); + guid->b[3] = hex_to_bin(str[0]) << 4 | hex_to_bin(str[1]); + guid->b[4] = hex_to_bin(str[11]) << 4 | hex_to_bin(str[12]); + guid->b[5] = hex_to_bin(str[9]) << 4 | hex_to_bin(str[10]); + guid->b[6] = hex_to_bin(str[16]) << 4 | hex_to_bin(str[17]); + guid->b[7] = hex_to_bin(str[14]) << 4 | hex_to_bin(str[15]); + guid->b[8] = hex_to_bin(str[19]) << 4 | hex_to_bin(str[20]); + guid->b[9] = hex_to_bin(str[21]) << 4 | hex_to_bin(str[22]); + guid->b[10] = hex_to_bin(str[24]) << 4 | hex_to_bin(str[25]); + guid->b[11] = hex_to_bin(str[26]) << 4 | hex_to_bin(str[27]); + guid->b[12] = hex_to_bin(str[28]) << 4 | hex_to_bin(str[29]); + guid->b[13] = hex_to_bin(str[30]) << 4 | hex_to_bin(str[31]); + guid->b[14] = hex_to_bin(str[32]) << 4 | hex_to_bin(str[33]); + guid->b[15] = hex_to_bin(str[34]) << 4 | hex_to_bin(str[35]); +} + +static int efivarfs_create(struct inode *dir, struct dentry *dentry, + umode_t mode, bool excl) +{ + struct inode *inode = efivarfs_get_inode(dir->i_sb, dir, mode, 0); + struct efivars *efivars = &__efivars; + struct efivar_entry *var; + int namelen, i = 0, err = 0; + + if (dentry->d_name.len < 38) + return -EINVAL; + + if (!inode) + return -ENOSPC; + + var = kzalloc(sizeof(struct efivar_entry), GFP_KERNEL); + + if (!var) + return -ENOMEM; + + namelen = dentry->d_name.len - GUID_LEN; + + efivarfs_hex_to_guid(dentry->d_name.name + namelen + 1, + &var->var.VendorGuid); + + for (i = 0; i < namelen; i++) + var->var.VariableName[i] = dentry->d_name.name[i]; + + var->var.VariableName[i] = '\0'; + + inode->i_private = var; + var->efivars = efivars; + var->kobj.kset = efivars->kset; + + err = kobject_init_and_add(&var->kobj, &efivar_ktype, NULL, "%s", + dentry->d_name.name); + if (err) + goto out; + + kobject_uevent(&var->kobj, KOBJ_ADD); + spin_lock(&efivars->lock); + list_add(&var->list, &efivars->list); + spin_unlock(&efivars->lock); + d_instantiate(dentry, inode); + dget(dentry); +out: + if (err) + kfree(var); + return err; +} + +static int efivarfs_unlink(struct inode *dir, struct dentry *dentry) +{ + struct efivar_entry *var = dentry->d_inode->i_private; + struct efivars *efivars = var->efivars; + efi_status_t status; + + spin_lock(&efivars->lock); + + status = efivars->ops->set_variable(var->var.VariableName, + &var->var.VendorGuid, + 0, 0, NULL); + + if (status == EFI_SUCCESS || status == EFI_NOT_FOUND) { + list_del(&var->list); + spin_unlock(&efivars->lock); + efivar_unregister(var); + drop_nlink(dir); + dput(dentry); + return 0; + } + + spin_unlock(&efivars->lock); + return -EINVAL; +}; + +int efivarfs_fill_super(struct super_block *sb, void *data, int silent) +{ + struct inode *inode = NULL; + struct dentry *root; + struct efivar_entry *entry, *n; + struct efivars *efivars = &__efivars; + int err; + + efivarfs_sb = sb; + + sb->s_maxbytes = MAX_LFS_FILESIZE; + sb->s_blocksize = PAGE_CACHE_SIZE; + sb->s_blocksize_bits = PAGE_CACHE_SHIFT; + sb->s_magic = PSTOREFS_MAGIC; + sb->s_op = &efivarfs_ops; + sb->s_time_gran = 1; + + inode = efivarfs_get_inode(sb, NULL, S_IFDIR | 0755, 0); + if (!inode) { + err = -ENOMEM; + goto fail; + } + inode->i_op = &efivarfs_dir_inode_operations; + + root = d_make_root(inode); + sb->s_root = root; + if (!root) { + err = -ENOMEM; + goto fail; + } + + list_for_each_entry_safe(entry, n, &efivars->list, list) { + struct inode *inode; + struct dentry *dentry, *root = efivarfs_sb->s_root; + char *name; + unsigned long size = 0; + int len, i; + + len = utf16_strlen(entry->var.VariableName); + + /* GUID plus trailing NULL */ + name = kmalloc(len + 38, GFP_ATOMIC); + + for (i = 0; i < len; i++) + name[i] = entry->var.VariableName[i] & 0xFF; + + name[len] = '-'; + + efi_guid_unparse(&entry->var.VendorGuid, name + len + 1); + + name[len+GUID_LEN] = '\0'; + + inode = efivarfs_get_inode(efivarfs_sb, root->d_inode, + S_IFREG | 0644, 0); + dentry = d_alloc_name(root, name); + + efivars->ops->get_variable(entry->var.VariableName, + &entry->var.VendorGuid, + &entry->var.Attributes, + &size, + NULL); + + mutex_lock(&inode->i_mutex); + inode->i_private = entry; + i_size_write(inode, size+4); + mutex_unlock(&inode->i_mutex); + d_add(dentry, inode); + } + + return 0; +fail: + iput(inode); + return err; +} + +static struct dentry *efivarfs_mount(struct file_system_type *fs_type, + int flags, const char *dev_name, void *data) +{ + return mount_single(fs_type, flags, data, efivarfs_fill_super); +} + +static void efivarfs_kill_sb(struct super_block *sb) +{ + kill_litter_super(sb); + efivarfs_sb = NULL; +} + +static struct file_system_type efivarfs_type = { + .name = "efivarfs", + .mount = efivarfs_mount, + .kill_sb = efivarfs_kill_sb, +}; + +static const struct inode_operations efivarfs_dir_inode_operations = { + .lookup = simple_lookup, + .unlink = efivarfs_unlink, + .create = efivarfs_create, +}; + +static struct pstore_info efi_pstore_info; + #ifdef CONFIG_PSTORE static int efi_pstore_open(struct pstore_info *psi) @@ -1198,6 +1571,8 @@ int register_efivars(struct efivars *efi pstore_register(&efivars->efi_pstore_info); } + register_filesystem(&efivarfs_type); + out: kfree(variable_name); @@ -1205,9 +1580,6 @@ out: } EXPORT_SYMBOL_GPL(register_efivars); -static struct efivars __efivars; -static struct efivar_operations ops; - /* * For now we register the efi subsystem with the firmware subsystem * and the vars subsystem with the efi subsystem. In the future, it --- a/include/linux/efi.h +++ b/include/linux/efi.h @@ -29,7 +29,12 @@ #define EFI_UNSUPPORTED ( 3 | (1UL << (BITS_PER_LONG-1))) #define EFI_BAD_BUFFER_SIZE ( 4 | (1UL << (BITS_PER_LONG-1))) #define EFI_BUFFER_TOO_SMALL ( 5 | (1UL << (BITS_PER_LONG-1))) +#define EFI_NOT_READY ( 6 | (1UL << (BITS_PER_LONG-1))) +#define EFI_DEVICE_ERROR ( 7 | (1UL << (BITS_PER_LONG-1))) +#define EFI_WRITE_PROTECTED ( 8 | (1UL << (BITS_PER_LONG-1))) +#define EFI_OUT_OF_RESOURCES ( 9 | (1UL << (BITS_PER_LONG-1))) #define EFI_NOT_FOUND (14 | (1UL << (BITS_PER_LONG-1))) +#define EFI_SECURITY_VIOLATION (26 | (1UL << (BITS_PER_LONG-1))) typedef unsigned long efi_status_t; typedef u8 efi_bool_t; -- To unsubscribe, e-mail: opensuse-kernel+unsubscribe@opensuse.org To contact the owner, e-mail: opensuse-kernel+owner@opensuse.org

From: Lee, Chun-Yi Git-commit: 605e70c7aa1b7b0d554baf945630c1d606bbfbc3 Patch-mainline: v3.8 References: bnc#808680 Target: openSUSE 12.3 UEFI variable filesystem need a new mount point, so this patch add efivars kobject to efi_kobj for create a /sys/firmware/efi/efivars folder. Cc: Matthew Garrett Cc: H. Peter Anvin Signed-off-by: Lee, Chun-Yi Signed-off-by: Jeremy Kerr Signed-off-by: Matt Fleming --- drivers/firmware/efivars.c | 9 +++++++++ include/linux/efi.h | 1 + 2 files changed, 10 insertions(+) --- a/drivers/firmware/efivars.c +++ b/drivers/firmware/efivars.c @@ -1527,6 +1527,7 @@ void unregister_efivars(struct efivars * sysfs_remove_bin_file(&efivars->kset->kobj, efivars->del_var); kfree(efivars->new_var); kfree(efivars->del_var); + kobject_put(efivars->kobject); kset_unregister(efivars->kset); } EXPORT_SYMBOL_GPL(unregister_efivars); @@ -1558,6 +1559,14 @@ int register_efivars(struct efivars *efi goto out; } + efivars->kobject = kobject_create_and_add("efivars", parent_kobj); + if (!efivars->kobject) { + pr_err("efivars: Subsystem registration failed.\n"); + error = -ENOMEM; + kset_unregister(efivars->kset); + goto out; + } + /* * Per EFI spec, the maximum storage allocated for both * the variable name and variable data is 1024 bytes. --- a/include/linux/efi.h +++ b/include/linux/efi.h @@ -678,6 +678,7 @@ struct efivars { spinlock_t lock; struct list_head list; struct kset *kset; + struct kobject *kobject; struct bin_attribute *new_var, *del_var; const struct efivar_operations *ops; struct efivar_entry *walk_entry; -- To unsubscribe, e-mail: opensuse-kernel+unsubscribe@opensuse.org To contact the owner, e-mail: opensuse-kernel+owner@opensuse.org

From: Andy Whitcroft Git-commit: d142df03a798ee7d2db10a1f20945110ea6067ff Patch-mainline: v3.8 References: bnc#808680 Target: openSUSE 12.3 Signed-off-by: Andy Whitcroft Acked-by: Matthew Garrett Acked-by: Jeremy Kerr Signed-off-by: Matt Fleming Acked-by: Lee, Chun-Yi --- drivers/firmware/efivars.c | 6 +++--- 1 file changed, 3 insertions(+), 3 deletions(-) --- a/drivers/firmware/efivars.c +++ b/drivers/firmware/efivars.c @@ -766,7 +766,7 @@ static ssize_t efivarfs_file_read(struct unsigned long datasize = 0; u32 attributes; void *data; - ssize_t size; + ssize_t size = 0; status = efivars->ops->get_variable(var->var.VariableName, &var->var.VendorGuid, @@ -784,13 +784,13 @@ static ssize_t efivarfs_file_read(struct &var->var.VendorGuid, &attributes, &datasize, (data + 4)); - if (status != EFI_SUCCESS) - return 0; + goto out_free; memcpy(data, &attributes, 4); size = simple_read_from_buffer(userbuf, count, ppos, data, datasize + 4); +out_free: kfree(data); return size; -- To unsubscribe, e-mail: opensuse-kernel+unsubscribe@opensuse.org To contact the owner, e-mail: opensuse-kernel+owner@opensuse.org

From: Andy Whitcroft Git-commit: 5c9b50ab8ca962bc085cfb49bae947910d73f794 Patch-mainline: v3.8 References: bnc#808680 Target: openSUSE 12.3 When d_make_root() fails it will automatically drop the reference on the root inode. We should not be doing so as well. Signed-off-by: Andy Whitcroft Acked-by: Matthew Garrett Acked-by: Jeremy Kerr Signed-off-by: Matt Fleming Acked-by: Lee, Chun-Yi --- drivers/firmware/efivars.c | 16 ++++------------ 1 file changed, 4 insertions(+), 12 deletions(-) --- a/drivers/firmware/efivars.c +++ b/drivers/firmware/efivars.c @@ -948,7 +948,6 @@ int efivarfs_fill_super(struct super_blo struct dentry *root; struct efivar_entry *entry, *n; struct efivars *efivars = &__efivars; - int err; efivarfs_sb = sb; @@ -960,18 +959,14 @@ int efivarfs_fill_super(struct super_blo sb->s_time_gran = 1; inode = efivarfs_get_inode(sb, NULL, S_IFDIR | 0755, 0); - if (!inode) { - err = -ENOMEM; - goto fail; - } + if (!inode) + return -ENOMEM; inode->i_op = &efivarfs_dir_inode_operations; root = d_make_root(inode); sb->s_root = root; - if (!root) { - err = -ENOMEM; - goto fail; - } + if (!root) + return -ENOMEM; list_for_each_entry_safe(entry, n, &efivars->list, list) { struct inode *inode; @@ -1012,9 +1007,6 @@ int efivarfs_fill_super(struct super_blo } return 0; -fail: - iput(inode); - return err; } static struct dentry *efivarfs_mount(struct file_system_type *fs_type, -- To unsubscribe, e-mail: opensuse-kernel+unsubscribe@opensuse.org To contact the owner, e-mail: opensuse-kernel+owner@opensuse.org

From: Andy Whitcroft Git-commit: 5ba6e2919b9e18a051e5bdd6c21f52ee4976513f Patch-mainline: v3.8 References: bnc#808680 Target: openSUSE 12.3 Ensure we free both the name and inode on error when building the individual variables. Signed-off-by: Andy Whitcroft Acked-by: Matthew Garrett Acked-by: Jeremy Kerr Signed-off-by: Matt Fleming Acked-by: Lee, Chun-Yi --- drivers/firmware/efivars.c | 20 ++++++++++++++++++-- 1 file changed, 18 insertions(+), 2 deletions(-) --- a/drivers/firmware/efivars.c +++ b/drivers/firmware/efivars.c @@ -948,6 +948,7 @@ int efivarfs_fill_super(struct super_blo struct dentry *root; struct efivar_entry *entry, *n; struct efivars *efivars = &__efivars; + char *name; efivarfs_sb = sb; @@ -969,16 +970,18 @@ int efivarfs_fill_super(struct super_blo return -ENOMEM; list_for_each_entry_safe(entry, n, &efivars->list, list) { - struct inode *inode; struct dentry *dentry, *root = efivarfs_sb->s_root; - char *name; unsigned long size = 0; int len, i; + inode = NULL; + len = utf16_strlen(entry->var.VariableName); /* GUID plus trailing NULL */ name = kmalloc(len + 38, GFP_ATOMIC); + if (!name) + goto fail; for (i = 0; i < len; i++) name[i] = entry->var.VariableName[i] & 0xFF; @@ -991,7 +994,13 @@ int efivarfs_fill_super(struct super_blo inode = efivarfs_get_inode(efivarfs_sb, root->d_inode, S_IFREG | 0644, 0); + if (!inode) + goto fail_name; + dentry = d_alloc_name(root, name); + if (!dentry) + goto fail_inode; + /* copied by the above to local storage in the dentry. */ kfree(name); @@ -1009,6 +1018,13 @@ int efivarfs_fill_super(struct super_blo } return 0; + +fail_inode: + iput(inode); +fail_name: + kfree(name); +fail: + return -ENOMEM; } static struct dentry *efivarfs_mount(struct file_system_type *fs_type, -- To unsubscribe, e-mail: opensuse-kernel+unsubscribe@opensuse.org To contact the owner, e-mail: opensuse-kernel+owner@opensuse.org

From: Matt Fleming Git-commit: 7253eaba7b179db2e07e67c5b78d5f10b332291d Patch-mainline: v3.8 References: bnc#808680 Target: openSUSE 12.3 Instead of always returning 0 in efivarfs_file_read(), even when we fail to successfully read the variable, convert the EFI status to something meaningful and return that to the caller. This way the user will have some hint as to why the read failed. Acked-by: Jeremy Kerr Signed-off-by: Matt Fleming Acked-by: Lee, Chun-Yi --- drivers/firmware/efivars.c | 62 ++++++++++++++++++++++++++------------------- 1 file changed, 36 insertions(+), 26 deletions(-) --- a/drivers/firmware/efivars.c +++ b/drivers/firmware/efivars.c @@ -648,6 +648,36 @@ static int efivarfs_file_open(struct ino return 0; } +static int efi_status_to_err(efi_status_t status) +{ + int err; + + switch (status) { + case EFI_INVALID_PARAMETER: + err = -EINVAL; + break; + case EFI_OUT_OF_RESOURCES: + err = -ENOSPC; + break; + case EFI_DEVICE_ERROR: + err = -EIO; + break; + case EFI_WRITE_PROTECTED: + err = -EROFS; + break; + case EFI_SECURITY_VIOLATION: + err = -EACCES; + break; + case EFI_NOT_FOUND: + err = -ENOENT; + break; + default: + err = -EINVAL; + } + + return err; +} + static ssize_t efivarfs_file_write(struct file *file, const char __user *userbuf, size_t count, loff_t *ppos) { @@ -706,29 +736,7 @@ static ssize_t efivarfs_file_write(struc spin_unlock(&efivars->lock); kfree(data); - switch (status) { - case EFI_INVALID_PARAMETER: - count = -EINVAL; - break; - case EFI_OUT_OF_RESOURCES: - count = -ENOSPC; - break; - case EFI_DEVICE_ERROR: - count = -EIO; - break; - case EFI_WRITE_PROTECTED: - count = -EROFS; - break; - case EFI_SECURITY_VIOLATION: - count = -EACCES; - break; - case EFI_NOT_FOUND: - count = -ENOENT; - break; - default: - count = -EINVAL; - } - return count; + return efi_status_to_err(status); } /* @@ -786,12 +794,12 @@ static ssize_t efivarfs_file_read(struct spin_unlock(&efivars->lock); if (status != EFI_BUFFER_TOO_SMALL) - return 0; + return efi_status_to_err(status); data = kmalloc(datasize + 4, GFP_KERNEL); if (!data) - return 0; + return -ENOMEM; spin_lock(&efivars->lock); status = efivars->ops->get_variable(var->var.VariableName, @@ -800,8 +808,10 @@ static ssize_t efivarfs_file_read(struct (data + 4)); spin_unlock(&efivars->lock); - if (status != EFI_SUCCESS) + if (status != EFI_SUCCESS) { + size = efi_status_to_err(status); goto out_free; + } memcpy(data, &attributes, 4); size = simple_read_from_buffer(userbuf, count, ppos, -- To unsubscribe, e-mail: opensuse-kernel+unsubscribe@opensuse.org To contact the owner, e-mail: opensuse-kernel+owner@opensuse.org

From: Matt Fleming Git-commit: d292384152fdd465dba792d555ce5db83e94efa0 Patch-mainline: v3.8 References: bnc#808680 Target: openSUSE 12.3 Seeing "+ 4" littered throughout the functions gets a bit confusing. Use "sizeof(attributes)" which clearly explains what quantity we're adding. Acked-by: Jeremy Kerr Signed-off-by: Matt Fleming Acked-by: Lee, Chun-Yi --- drivers/firmware/efivars.c | 8 ++++---- 1 file changed, 4 insertions(+), 4 deletions(-) --- a/drivers/firmware/efivars.c +++ b/drivers/firmware/efivars.c @@ -801,7 +801,7 @@ static ssize_t efivarfs_file_read(struct if (status != EFI_BUFFER_TOO_SMALL) return efi_status_to_err(status); - data = kmalloc(datasize + 4, GFP_KERNEL); + data = kmalloc(datasize + sizeof(attributes), GFP_KERNEL); if (!data) return -ENOMEM; @@ -810,7 +810,7 @@ static ssize_t efivarfs_file_read(struct status = efivars->ops->get_variable(var->var.VariableName, &var->var.VendorGuid, &attributes, &datasize, - (data + 4)); + (data + sizeof(attributes))); spin_unlock(&efivars->lock); if (status != EFI_SUCCESS) { @@ -818,9 +818,9 @@ static ssize_t efivarfs_file_read(struct goto out_free; } - memcpy(data, &attributes, 4); + memcpy(data, &attributes, sizeof(attributes)); size = simple_read_from_buffer(userbuf, count, ppos, - data, datasize + 4); + data, datasize + sizeof(attributes)); out_free: kfree(data); -- To unsubscribe, e-mail: opensuse-kernel+unsubscribe@opensuse.org To contact the owner, e-mail: opensuse-kernel+owner@opensuse.org

From: Matt Fleming Git-commit: 07b1c5bc64cff9c880261a1fef562ef7ea7f6575 Patch-mainline: v3.8 References: bnc#808680 Target: openSUSE 12.3 There's no reason to declare 'datasize' as an int, since the majority of the functions it's passed to expect an unsigned long anyway. Plus, this way we avoid any sign problems during arithmetic. Acked-by: Jeremy Kerr Signed-off-by: Matt Fleming Acked-by: Lee, Chun-Yi --- drivers/firmware/efivars.c | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) --- a/drivers/firmware/efivars.c +++ b/drivers/firmware/efivars.c @@ -692,7 +692,7 @@ static ssize_t efivarfs_file_write(struc void *data; u32 attributes; struct inode *inode = file->f_mapping->host; - int datasize = count - sizeof(attributes); + unsigned long datasize = count - sizeof(attributes); unsigned long newdatasize; if (count < sizeof(attributes)) -- To unsubscribe, e-mail: opensuse-kernel+unsubscribe@opensuse.org To contact the owner, e-mail: opensuse-kernel+owner@opensuse.org

From: Matt Fleming Git-commit: cfcf2f11708f934d2bd294f973c2fcb0cc54f293 Patch-mainline: v3.8 References: bnc#808680 Target: openSUSE 12.3 We're stuffing a variable of type size_t (unsigned) into a ssize_t (signed) which, even though both types should be the same number of bits, it's just asking for sign issues to be introduced. Cc: Jeremy Kerr Reported-by: Alan Cox Signed-off-by: Matt Fleming Acked-by: Lee, Chun-Yi --- drivers/firmware/efivars.c | 13 ++++++++----- 1 file changed, 8 insertions(+), 5 deletions(-) --- a/drivers/firmware/efivars.c +++ b/drivers/firmware/efivars.c @@ -694,6 +694,7 @@ static ssize_t efivarfs_file_write(struc struct inode *inode = file->f_mapping->host; unsigned long datasize = count - sizeof(attributes); unsigned long newdatasize; + ssize_t bytes = 0; if (count < sizeof(attributes)) return -EINVAL; @@ -706,22 +707,22 @@ static ssize_t efivarfs_file_write(struc efivars = var->efivars; if (copy_from_user(&attributes, userbuf, sizeof(attributes))) { - count = -EFAULT; + bytes = -EFAULT; goto out; } if (attributes & ~(EFI_VARIABLE_MASK)) { - count = -EINVAL; + bytes = -EINVAL; goto out; } if (copy_from_user(data, userbuf + sizeof(attributes), datasize)) { - count = -EFAULT; + bytes = -EFAULT; goto out; } if (validate_var(&var->var, data, datasize) == false) { - count = -EINVAL; + bytes = -EINVAL; goto out; } @@ -744,6 +745,8 @@ static ssize_t efivarfs_file_write(struc return efi_status_to_err(status); } + bytes = count; + /* * Writing to the variable may have caused a change in size (which * could either be an append or an overwrite), or the variable to be @@ -778,7 +781,7 @@ static ssize_t efivarfs_file_write(struc out: kfree(data); - return count; + return bytes; } static ssize_t efivarfs_file_read(struct file *file, char __user *userbuf, -- To unsubscribe, e-mail: opensuse-kernel+unsubscribe@opensuse.org To contact the owner, e-mail: opensuse-kernel+owner@opensuse.org

From: Matt Fleming Git-commit: e83af1f18c78c7b6aa720beecc927ecc8afd3647 Patch-mainline: v3.8 References: bnc#808680 Target: openSUSE 12.3 sparse is complaining that efivarfs_fill_super() doesn't have a prototype. Make it static to avoid the warning. Cc: Xie ChanglongX Cc: Matthew Garrett Cc: Jeremy Kerr Signed-off-by: Matt Fleming Acked-by: Lee, Chun-Yi --- drivers/firmware/efivars.c | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) --- a/drivers/firmware/efivars.c +++ b/drivers/firmware/efivars.c @@ -1004,7 +1004,7 @@ static int efivarfs_unlink(struct inode return -EINVAL; }; -int efivarfs_fill_super(struct super_block *sb, void *data, int silent) +static int efivarfs_fill_super(struct super_block *sb, void *data, int silent) { struct inode *inode = NULL; struct dentry *root; -- To unsubscribe, e-mail: opensuse-kernel+unsubscribe@opensuse.org To contact the owner, e-mail: opensuse-kernel+owner@opensuse.org

From: Matt Fleming Git-commit: 1fa7e6958c5f82cc75c594e3ffaf73cc26fc94c1 Patch-mainline: v3.8 References: bnc#808680 Target: openSUSE 12.3 Files are created in efivarfs_create() before a corresponding variable is created in the firmware. This leads to users being able to read/write to the file without the variable existing in the firmware. Reading a non-existent variable currently returns -ENOENT, which is confusing because the file obviously *does* exist. Convert EFI_NOT_FOUND into -EIO which is the closest thing to "error while interacting with firmware", and should hopefully indicate to the caller that the variable is in some uninitialised state. Cc: Josh Boyer Acked-by: Jeremy Kerr Cc: Lee, Chun-Yi Cc: Andy Whitcroft Reported-by: Lingzhu Xiang Signed-off-by: Matt Fleming Acked-by: Lee, Chun-Yi --- drivers/firmware/efivars.c | 2 +- 1 files changed, 1 insertions(+), 1 deletions(-) diff --git a/drivers/firmware/efivars.c b/drivers/firmware/efivars.c index fa9fa03..807dad4 100644 --- a/drivers/firmware/efivars.c +++ b/drivers/firmware/efivars.c @@ -674,7 +674,7 @@ static int efi_status_to_err(efi_status_t status) err = -EACCES; break; case EFI_NOT_FOUND: - err = -ENOENT; + err = -EIO; break; default: err = -EINVAL; -- 1.6.4.2 -- To unsubscribe, e-mail: opensuse-kernel+unsubscribe@opensuse.org To contact the owner, e-mail: opensuse-kernel+owner@opensuse.org

From: Seiji Aguchi Git-commit: d80a361d779a9f19498943d1ca84243209cd5647 Patch-mainline: v3.8 References: bnc#808680 Target: openSUSE 12.3 [Issue] As discussed in a thread below, Running out of space in EFI isn't a well-tested scenario. And we wouldn't expect all firmware to handle it gracefully. http://marc.info/?l=linux-kernel&m=134305325801789&w=2 On the other hand, current efi_pstore doesn't check a remaining space of storage at writing time. Therefore, efi_pstore may not work if it tries to write a large amount of data. [Patch Description] To avoid handling the situation above, this patch checks if there is a space enough to log with QueryVariableInfo() before writing data. Signed-off-by: Seiji Aguchi Acked-by: Mike Waychison Signed-off-by: Tony Luck Acked-by: Lee, Chun-Yi --- drivers/firmware/efivars.c | 17 +++++++++++++++++ 1 file changed, 17 insertions(+) --- a/drivers/firmware/efivars.c +++ b/drivers/firmware/efivars.c @@ -1188,12 +1188,29 @@ static int efi_pstore_write(enum pstore_ struct efivars *efivars = psi->data; struct efivar_entry *entry, *found = NULL; int i, ret = 0; + u64 storage_space, remaining_space, max_variable_size; + efi_status_t status = EFI_NOT_FOUND; sprintf(stub_name, "dump-type%u-%u-", type, part); sprintf(name, "%s%lu", stub_name, get_seconds()); spin_lock(&efivars->lock); + /* + * Check if there is a space enough to log. + * size: a size of logging data + * DUMP_NAME_LEN * 2: a maximum size of variable name + */ + status = efivars->ops->query_variable_info(PSTORE_EFI_ATTRIBUTES, + &storage_space, + &remaining_space, + &max_variable_size); + if (status || remaining_space < size + DUMP_NAME_LEN * 2) { + spin_unlock(&efivars->lock); + *id = part; + return -ENOSPC; + } + for (i = 0; i < DUMP_NAME_LEN; i++) efi_name[i] = stub_name[i]; -- To unsubscribe, e-mail: opensuse-kernel+unsubscribe@opensuse.org To contact the owner, e-mail: opensuse-kernel+owner@opensuse.org

From: Seiji Aguchi Git-commit: e59310adf5eebce108f78b6c47bb330aae2e1666 Patch-mainline: v3.9-rc1 References: bnc#808680 Target: openSUSE 12.3 [Issue] There is a scenario which efi_pstore may hang up: - cpuA grabs efivars->lock - cpuB panics and calls smp_send_stop - smp_send_stop sends IRQ to cpuA - after 1 second, cpuB gives up on cpuA and sends an NMI instead - cpuA is now in an NMI handler while still holding efivars->lock - cpuB is deadlocked This case may happen if a firmware has a bug and cpuA is stuck talking with it. [Solution] This patch changes a spin_lock to a spin_trylock in non-blocking paths. and if the spin_lock has already taken by another cpu, it returns without accessing to a firmware to avoid the deadlock. Signed-off-by: Seiji Aguchi Acked-by: Don Zickus Signed-off-by: Tony Luck Acked-by: Lee, Chun-Yi --- drivers/firmware/efivars.c | 11 ++++++++++- 1 file changed, 10 insertions(+), 1 deletion(-) --- a/drivers/firmware/efivars.c +++ b/drivers/firmware/efivars.c @@ -1194,7 +1194,16 @@ static int efi_pstore_write(enum pstore_ sprintf(stub_name, "dump-type%u-%u-", type, part); sprintf(name, "%s%lu", stub_name, get_seconds()); - spin_lock(&efivars->lock); + if (pstore_cannot_block_path(reason)) { + /* + * If the lock is taken by another cpu in non-blocking path, + * this driver returns without entering firmware to avoid + * hanging up. + */ + if (!spin_trylock(&efivars->lock)) + return -EBUSY; + } else + spin_lock(&efivars->lock); /* * Check if there is a space enough to log. -- To unsubscribe, e-mail: opensuse-kernel+unsubscribe@opensuse.org To contact the owner, e-mail: opensuse-kernel+owner@opensuse.org

From: Seiji Aguchi Git-commit: 81fa4e581d9283f7992a0d8c534bb141eb840a14 Patch-mainline: v3.9-rc1 References: bnc#808680 Target: openSUSE 12.3 [Problem] There is a scenario which efi_pstore fails to log messages in a panic case. - CPUA holds an efi_var->lock in either efivarfs parts or efi_pstore with interrupt enabled. - CPUB panics and sends IPI to CPUA in smp_send_stop(). - CPUA stops with holding the lock. - CPUB kicks efi_pstore_write() via kmsg_dump(KSMG_DUMP_PANIC) but it returns without logging messages. [Patch Description] This patch disables an external interruption while holding efivars->lock as follows. In efi_pstore_write() and get_var_data(), spin_lock/spin_unlock is replaced by spin_lock_irqsave/spin_unlock_irqrestore because they may be called in an interrupt context. In other functions, they are replaced by spin_lock_irq/spin_unlock_irq. because they are all called from a process context. By applying this patch, we can avoid the problem above with a following senario. - CPUA holds an efi_var->lock with interrupt disabled. - CPUB panics and sends IPI to CPUA in smp_send_stop(). - CPUA receives the IPI after releasing the lock because it is disabling interrupt while holding the lock. - CPUB waits for one sec until CPUA releases the lock. - CPUB kicks efi_pstore_write() via kmsg_dump(KSMG_DUMP_PANIC) And it can hold the lock successfully. Signed-off-by: Seiji Aguchi Acked-by: Mike Waychison Acked-by: Matt Fleming Signed-off-by: Tony Luck Acked-by: Lee, Chun-Yi --- drivers/firmware/efivars.c | 82 +++++++++++++++++++++++---------------------- 1 file changed, 42 insertions(+), 40 deletions(-) --- a/drivers/firmware/efivars.c +++ b/drivers/firmware/efivars.c @@ -405,10 +405,11 @@ static efi_status_t get_var_data(struct efivars *efivars, struct efi_variable *var) { efi_status_t status; + unsigned long flags; - spin_lock(&efivars->lock); + spin_lock_irqsave(&efivars->lock, flags); status = get_var_data_locked(efivars, var); - spin_unlock(&efivars->lock); + spin_unlock_irqrestore(&efivars->lock, flags); if (status != EFI_SUCCESS) { printk(KERN_WARNING "efivars: get_variable() failed 0x%lx!\n", @@ -537,14 +538,14 @@ efivar_store_raw(struct efivar_entry *en return -EINVAL; } - spin_lock(&efivars->lock); + spin_lock_irq(&efivars->lock); status = efivars->ops->set_variable(new_var->VariableName, &new_var->VendorGuid, new_var->Attributes, new_var->DataSize, new_var->Data); - spin_unlock(&efivars->lock); + spin_unlock_irq(&efivars->lock); if (status != EFI_SUCCESS) { printk(KERN_WARNING "efivars: set_variable() failed: status=%lx\n", @@ -713,7 +714,7 @@ static ssize_t efivarfs_file_write(struc * amounts of memory. Pick a default size of 64K if * QueryVariableInfo() isn't supported by the firmware. */ - spin_lock(&efivars->lock); + spin_lock_irq(&efivars->lock); if (!efivars->ops->query_variable_info) status = EFI_UNSUPPORTED; @@ -723,7 +724,7 @@ static ssize_t efivarfs_file_write(struc &remaining_size, &max_size); } - spin_unlock(&efivars->lock); + spin_unlock_irq(&efivars->lock); if (status != EFI_SUCCESS) { if (status != EFI_UNSUPPORTED) @@ -754,7 +755,7 @@ static ssize_t efivarfs_file_write(struc * set_variable call, and removal of the variable from the efivars * list (in the case of an authenticated delete). */ - spin_lock(&efivars->lock); + spin_lock_irq(&efivars->lock); status = efivars->ops->set_variable(var->var.VariableName, &var->var.VendorGuid, @@ -762,7 +763,7 @@ static ssize_t efivarfs_file_write(struc data); if (status != EFI_SUCCESS) { - spin_unlock(&efivars->lock); + spin_unlock_irq(&efivars->lock); kfree(data); return efi_status_to_err(status); @@ -783,21 +784,21 @@ static ssize_t efivarfs_file_write(struc NULL); if (status == EFI_BUFFER_TOO_SMALL) { - spin_unlock(&efivars->lock); + spin_unlock_irq(&efivars->lock); mutex_lock(&inode->i_mutex); i_size_write(inode, newdatasize + sizeof(attributes)); mutex_unlock(&inode->i_mutex); } else if (status == EFI_NOT_FOUND) { list_del(&var->list); - spin_unlock(&efivars->lock); + spin_unlock_irq(&efivars->lock); efivar_unregister(var); drop_nlink(inode); d_delete(file->f_dentry); dput(file->f_dentry); } else { - spin_unlock(&efivars->lock); + spin_unlock_irq(&efivars->lock); pr_warn("efivarfs: inconsistent EFI variable implementation? " "status = %lx\n", status); } @@ -819,11 +820,11 @@ static ssize_t efivarfs_file_read(struct void *data; ssize_t size = 0; - spin_lock(&efivars->lock); + spin_lock_irq(&efivars->lock); status = efivars->ops->get_variable(var->var.VariableName, &var->var.VendorGuid, &attributes, &datasize, NULL); - spin_unlock(&efivars->lock); + spin_unlock_irq(&efivars->lock); if (status != EFI_BUFFER_TOO_SMALL) return efi_status_to_err(status); @@ -833,12 +834,12 @@ static ssize_t efivarfs_file_read(struct if (!data) return -ENOMEM; - spin_lock(&efivars->lock); + spin_lock_irq(&efivars->lock); status = efivars->ops->get_variable(var->var.VariableName, &var->var.VendorGuid, &attributes, &datasize, (data + sizeof(attributes))); - spin_unlock(&efivars->lock); + spin_unlock_irq(&efivars->lock); if (status != EFI_SUCCESS) { size = efi_status_to_err(status); @@ -967,9 +968,9 @@ static int efivarfs_create(struct inode goto out; kobject_uevent(&var->kobj, KOBJ_ADD); - spin_lock(&efivars->lock); + spin_lock_irq(&efivars->lock); list_add(&var->list, &efivars->list); - spin_unlock(&efivars->lock); + spin_unlock_irq(&efivars->lock); d_instantiate(dentry, inode); dget(dentry); out: @@ -986,7 +987,7 @@ static int efivarfs_unlink(struct inode struct efivars *efivars = var->efivars; efi_status_t status; - spin_lock(&efivars->lock); + spin_lock_irq(&efivars->lock); status = efivars->ops->set_variable(var->var.VariableName, &var->var.VendorGuid, @@ -994,14 +995,14 @@ static int efivarfs_unlink(struct inode if (status == EFI_SUCCESS || status == EFI_NOT_FOUND) { list_del(&var->list); - spin_unlock(&efivars->lock); + spin_unlock_irq(&efivars->lock); efivar_unregister(var); drop_nlink(dentry->d_inode); dput(dentry); return 0; } - spin_unlock(&efivars->lock); + spin_unlock_irq(&efivars->lock); return -EINVAL; }; @@ -1146,13 +1147,13 @@ static int efivarfs_fill_super(struct su /* copied by the above to local storage in the dentry. */ kfree(name); - spin_lock(&efivars->lock); + spin_lock_irq(&efivars->lock); efivars->ops->get_variable(entry->var.VariableName, &entry->var.VendorGuid, &entry->var.Attributes, &size, NULL); - spin_unlock(&efivars->lock); + spin_unlock_irq(&efivars->lock); mutex_lock(&inode->i_mutex); inode->i_private = entry; @@ -1215,7 +1216,7 @@ static int efi_pstore_open(struct pstore { struct efivars *efivars = psi->data; - spin_lock(&efivars->lock); + spin_lock_irq(&efivars->lock); efivars->walk_entry = list_first_entry(&efivars->list, struct efivar_entry, list); return 0; @@ -1225,7 +1226,7 @@ static int efi_pstore_close(struct pstor { struct efivars *efivars = psi->data; - spin_unlock(&efivars->lock); + spin_unlock_irq(&efivars->lock); return 0; } @@ -1281,6 +1282,7 @@ static int efi_pstore_write(enum pstore_ int i, ret = 0; u64 storage_space, remaining_space, max_variable_size; efi_status_t status = EFI_NOT_FOUND; + unsigned long flags; sprintf(stub_name, "dump-type%u-%u-", type, part); sprintf(name, "%s%lu", stub_name, get_seconds()); @@ -1291,10 +1293,10 @@ static int efi_pstore_write(enum pstore_ * this driver returns without entering firmware to avoid * hanging up. */ - if (!spin_trylock(&efivars->lock)) + if (!spin_trylock_irqsave(&efivars->lock, flags)) return -EBUSY; } else - spin_lock(&efivars->lock); + spin_lock_irqsave(&efivars->lock, flags); /* * Check if there is a space enough to log. @@ -1306,7 +1308,7 @@ static int efi_pstore_write(enum pstore_ &remaining_space, &max_variable_size); if (status || remaining_space < size + DUMP_NAME_LEN * 2) { - spin_unlock(&efivars->lock); + spin_unlock_irqrestore(&efivars->lock, flags); *id = part; return -ENOSPC; } @@ -1347,7 +1349,7 @@ static int efi_pstore_write(enum pstore_ efivars->ops->set_variable(efi_name, &vendor, PSTORE_EFI_ATTRIBUTES, size, psi->buf); - spin_unlock(&efivars->lock); + spin_unlock_irqrestore(&efivars->lock, flags); if (found) efivar_unregister(found); @@ -1431,7 +1433,7 @@ static ssize_t efivar_create(struct file return -EINVAL; } - spin_lock(&efivars->lock); + spin_lock_irq(&efivars->lock); /* * Does this variable already exist? @@ -1449,7 +1451,7 @@ static ssize_t efivar_create(struct file } } if (found) { - spin_unlock(&efivars->lock); + spin_unlock_irq(&efivars->lock); return -EINVAL; } @@ -1463,10 +1465,10 @@ static ssize_t efivar_create(struct file if (status != EFI_SUCCESS) { printk(KERN_WARNING "efivars: set_variable() failed: status=%lx\n", status); - spin_unlock(&efivars->lock); + spin_unlock_irq(&efivars->lock); return -EIO; } - spin_unlock(&efivars->lock); + spin_unlock_irq(&efivars->lock); /* Create the entry in sysfs. Locking is not required here */ status = efivar_create_sysfs_entry(efivars, @@ -1494,7 +1496,7 @@ static ssize_t efivar_delete(struct file if (!capable(CAP_SYS_ADMIN)) return -EACCES; - spin_lock(&efivars->lock); + spin_lock_irq(&efivars->lock); /* * Does this variable already exist? @@ -1512,7 +1514,7 @@ static ssize_t efivar_delete(struct file } } if (!found) { - spin_unlock(&efivars->lock); + spin_unlock_irq(&efivars->lock); return -EINVAL; } /* force the Attributes/DataSize to 0 to ensure deletion */ @@ -1528,12 +1530,12 @@ static ssize_t efivar_delete(struct file if (status != EFI_SUCCESS) { printk(KERN_WARNING "efivars: set_variable() failed: status=%lx\n", status); - spin_unlock(&efivars->lock); + spin_unlock_irq(&efivars->lock); return -EIO; } list_del(&search_efivar->list); /* We need to release this lock before unregistering. */ - spin_unlock(&efivars->lock); + spin_unlock_irq(&efivars->lock); efivar_unregister(search_efivar); /* It's dead Jim.... */ @@ -1648,9 +1650,9 @@ efivar_create_sysfs_entry(struct efivars kfree(short_name); short_name = NULL; - spin_lock(&efivars->lock); + spin_lock_irq(&efivars->lock); list_add(&new_efivar->list, &efivars->list); - spin_unlock(&efivars->lock); + spin_unlock_irq(&efivars->lock); return 0; } @@ -1719,9 +1721,9 @@ void unregister_efivars(struct efivars * struct efivar_entry *entry, *n; list_for_each_entry_safe(entry, n, &efivars->list, list) { - spin_lock(&efivars->lock); + spin_lock_irq(&efivars->lock); list_del(&entry->list); - spin_unlock(&efivars->lock); + spin_unlock_irq(&efivars->lock); efivar_unregister(entry); } if (efivars->new_var) -- To unsubscribe, e-mail: opensuse-kernel+unsubscribe@opensuse.org To contact the owner, e-mail: opensuse-kernel+owner@opensuse.org

From: Matt Fleming Git-commit: 47f531e8ba3bc3901a0c493f4252826c41dea1a1 Patch-mainline: v3.9-rc1 References: bnc#808680 Target: openSUSE 12.3 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 Cc: Matthew Garrett Cc: Jeremy Kerr Cc: Al Viro Signed-off-by: Matt Fleming Acked-by: Lee, Chun-Yi --- 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 #include #include +#include #include #include @@ -909,6 +910,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]); @@ -937,11 +980,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); -- To unsubscribe, e-mail: opensuse-kernel+unsubscribe@opensuse.org To contact the owner, e-mail: opensuse-kernel+owner@opensuse.org

From: Matt Fleming Git-commit: 123abd76edf56c02a76b46d3d673897177ef067b Patch-mainline: v3.9-rc2 References: bnc#808680 Target: openSUSE 12.3 Stricter validation was introduced with commit da27a24383b2b ("efivarfs: guid part of filenames are case-insensitive") and commit 47f531e8ba3b ("efivarfs: Validate filenames much more aggressively"), which is necessary for the guid portion of efivarfs filenames, but we don't need to be so strict with the first part, the variable name. The UEFI specification doesn't impose any constraints on variable names other than they be a NULL-terminated string. The above commits caused a regression that resulted in users seeing the following message, $ sudo mount -v /sys/firmware/efi/efivars mount: Cannot allocate memory whenever pstore EFI variables were present in the variable store, since their variable names failed to pass the following check, /* GUID should be right after the first '-' */ if (s - 1 != strchr(str, '-')) as a typical pstore filename is of the form, dump-type0-10-1-<guid>. The fix is trivial since the guid portion of the filename is GUID_LEN bytes, we can use (len - GUID_LEN) to ensure the '-' character is where we expect it to be. (The bogus ENOMEM error value will be fixed in a separate patch.) Reported-by: Joseph Yasi Tested-by: Joseph Yasi Reported-by: Lingzhu Xiang Cc: Josh Boyer Cc: Jeremy Kerr Cc: Matthew Garrett Cc: # v3.8 Signed-off-by: Matt Fleming Acked-by: Lee, Chun-Yi --- drivers/firmware/efivars.c | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) --- a/drivers/firmware/efivars.c +++ b/drivers/firmware/efivars.c @@ -975,8 +975,8 @@ static bool efivarfs_valid_name(const ch if (len < GUID_LEN + 2) return false; - /* GUID should be right after the first '-' */ - if (s - 1 != strchr(str, '-')) + /* GUID must be preceded by a '-' */ + if (*(s - 1) != '-') return false; /* -- To unsubscribe, e-mail: opensuse-kernel+unsubscribe@opensuse.org To contact the owner, e-mail: opensuse-kernel+owner@opensuse.org

From: Matt Fleming Git-commit: ec50bd32f1672d38ddce10fb1841cbfda89cfe9a Patch-mainline: v3.9-rc4 References: bnc#808680 Target: openSUSE 12.3 Some buggy firmware implementations return a string length from GetNextVariableName() that is actually larger than the string in 'variable_name', as Michael Schroeder writes,

On HP z220 system (firmware version 1.54), some EFI variables are incorrectly named :

ls -d /sys/firmware/efi/vars/*8be4d* | grep -v -- -8be returns /sys/firmware/efi/vars/dbxDefault-pport8be4df61-93ca-11d2-aa0d-00e098032b8c /sys/firmware/efi/vars/KEKDefault-pport8be4df61-93ca-11d2-aa0d-00e098032b8c /sys/firmware/efi/vars/SecureBoot-pport8be4df61-93ca-11d2-aa0d-00e098032b8c /sys/firmware/efi/vars/SetupMode-Information8be4df61-93ca-11d2-aa0d-00e098032b8c

Since 'variable_name' is a string, we can validate its size by searching for the terminating NULL character. Reported-by: Frederic Crozat Cc: Matthew Garrett Cc: Josh Boyer Cc: Michael Schroeder Cc: Lee, Chun-Yi Cc: Lingzhu Xiang Signed-off-by: Matt Fleming Acked-by: Lee, Chun-Yi --- drivers/firmware/efivars.c | 30 ++++++++++++++++++++++++++++++ 1 file changed, 30 insertions(+) --- a/drivers/firmware/efivars.c +++ b/drivers/firmware/efivars.c @@ -1911,6 +1911,33 @@ void unregister_efivars(struct efivars * } EXPORT_SYMBOL_GPL(unregister_efivars); +/* + * Sanity check size of a variable name. + */ +static unsigned long sanity_check_size(efi_char16_t *variable_name, + unsigned long variable_name_size) +{ + unsigned long len; + efi_char16_t c; + + /* + * The variable name is, by definition, a NULL-terminated + * string, so make absolutely sure that variable_name_size is + * the value we expect it to be. If not, return the real size. + */ + for (len = 2; len <= variable_name_size; len += sizeof(c)) { + c = variable_name[(len / sizeof(c)) - 1]; + if (!c) + break; + } + + + if (len != variable_name_size) + printk(KERN_WARNING "efivars: bogus variable_name_size: %lu %lu\n", len, variable_name_size); + + return min(len, variable_name_size); +} + int register_efivars(struct efivars *efivars, const struct efivar_operations *ops, struct kobject *parent_kobj) @@ -1957,8 +1984,11 @@ int register_efivars(struct efivars *efi status = ops->get_next_variable(&variable_name_size, variable_name, &vendor_guid); + switch (status) { case EFI_SUCCESS: + variable_name_size = sanity_check_size(variable_name, + variable_name_size); efivar_create_sysfs_entry(efivars, variable_name_size, variable_name, -- To unsubscribe, e-mail: opensuse-kernel+unsubscribe@opensuse.org To contact the owner, e-mail: opensuse-kernel+owner@opensuse.org