From 5d9db883761ad1bc2245fd3018715549b974203d Mon Sep 17 00:00:00 2001 From: Matthew Garrett Date: Fri, 5 Oct 2012 13:54:56 +0800 Subject: [PATCH 01/24] efi: Add support for a UEFI variable filesystem 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 --- drivers/firmware/efivars.c | 384 ++++++++++++++++++++++++++++++++++++- include/linux/efi.h | 5 + 2 files changed, 383 insertions(+), 6 deletions(-) diff --git a/drivers/firmware/efivars.c b/drivers/firmware/efivars.c index d10c9873dd9a..b605c4849772 100644 --- 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 = no_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 *efivars, 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 diff --git a/include/linux/efi.h b/include/linux/efi.h index 8670eb1eb8cd..b2af1571592b 100644 --- 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; From 0c542edde3cecc99b180a440ae33dcb7f28642ce Mon Sep 17 00:00:00 2001 From: Jeremy Kerr Date: Fri, 5 Oct 2012 13:54:56 +0800 Subject: [PATCH 02/24] efi: Handle deletions and size changes in efivarfs_write_file A write to an efivarfs file will not always result in a variable of 'count' size after the EFI SetVariable() call. We may have appended to the existing data (ie, with the EFI_VARIABLE_APPEND_WRITE attribute), or even have deleted the variable (with an authenticated variable update, with a zero datasize). This change re-reads the updated variable from firmware, to check for size changes and deletions. In the latter case, we need to drop the dentry. Signed-off-by: Jeremy Kerr Signed-off-by: Matt Fleming --- drivers/firmware/efivars.c | 49 ++++++++++++++++++++++++++++++-------- 1 file changed, 39 insertions(+), 10 deletions(-) diff --git a/drivers/firmware/efivars.c b/drivers/firmware/efivars.c index b605c4849772..d7658b4a5010 100644 --- a/drivers/firmware/efivars.c +++ b/drivers/firmware/efivars.c @@ -658,6 +658,7 @@ static ssize_t efivarfs_file_write(struct file *file, u32 attributes; struct inode *inode = file->f_mapping->host; int datasize = count - sizeof(attributes); + unsigned long newdatasize; if (count < sizeof(attributes)) return -EINVAL; @@ -696,32 +697,60 @@ static ssize_t efivarfs_file_write(struct file *file, 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; + goto out; case EFI_OUT_OF_RESOURCES: count = -ENOSPC; - break; + goto out; case EFI_DEVICE_ERROR: count = -EIO; - break; + goto out; case EFI_WRITE_PROTECTED: count = -EROFS; - break; + goto out; case EFI_SECURITY_VIOLATION: count = -EACCES; - break; + goto out; case EFI_NOT_FOUND: count = -ENOENT; - break; + goto out; default: count = -EINVAL; - break; + goto out; } + + /* + * 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 + * deleted. Perform a GetVariable() so we can tell what actually + * happened. + */ + newdatasize = 0; + status = efivars->ops->get_variable(var->var.VariableName, + &var->var.VendorGuid, + NULL, &newdatasize, + NULL); + + if (status == EFI_BUFFER_TOO_SMALL) { + mutex_lock(&inode->i_mutex); + i_size_write(inode, newdatasize + sizeof(attributes)); + mutex_unlock(&inode->i_mutex); + + } else if (status == EFI_NOT_FOUND) { + spin_lock(&efivars->lock); + list_del(&var->list); + spin_unlock(&efivars->lock); + efivar_unregister(var); + drop_nlink(inode); + dput(file->f_dentry); + + } else { + pr_warn("efivarfs: inconsistent EFI variable implementation? " + "status = %lx\n", status); + } + out: kfree(data); From 605e70c7aa1b7b0d554baf945630c1d606bbfbc3 Mon Sep 17 00:00:00 2001 From: "Lee, Chun-Yi" Date: Fri, 5 Oct 2012 13:54:56 +0800 Subject: [PATCH 03/24] efi: add efivars kobject to efi sysfs folder 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(+) diff --git a/drivers/firmware/efivars.c b/drivers/firmware/efivars.c index d7658b4a5010..6793965b7c8b 100644 --- a/drivers/firmware/efivars.c +++ b/drivers/firmware/efivars.c @@ -1527,6 +1527,7 @@ void unregister_efivars(struct efivars *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 *efivars, 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. diff --git a/include/linux/efi.h b/include/linux/efi.h index b2af1571592b..337aefbfb003 100644 --- a/include/linux/efi.h +++ b/include/linux/efi.h @@ -662,6 +662,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; From e913ca7d16d70b75367ff56a3b201980501d542c Mon Sep 17 00:00:00 2001 From: Matt Fleming Date: Thu, 4 Oct 2012 09:57:31 +0100 Subject: [PATCH 04/24] efivarfs: Add documentation for the EFI variable filesystem Signed-off-by: Matt Fleming --- Documentation/filesystems/00-INDEX | 2 ++ Documentation/filesystems/efivarfs.txt | 16 ++++++++++++++++ 2 files changed, 18 insertions(+) create mode 100644 Documentation/filesystems/efivarfs.txt diff --git a/Documentation/filesystems/00-INDEX b/Documentation/filesystems/00-INDEX index 8c624a18f67d..7b52ba7bf32a 100644 --- a/Documentation/filesystems/00-INDEX +++ b/Documentation/filesystems/00-INDEX @@ -38,6 +38,8 @@ dnotify_test.c - example program for dnotify ecryptfs.txt - docs on eCryptfs: stacked cryptographic filesystem for Linux. +efivarfs.txt + - info for the efivarfs filesystem. exofs.txt - info, usage, mount options, design about EXOFS. ext2.txt diff --git a/Documentation/filesystems/efivarfs.txt b/Documentation/filesystems/efivarfs.txt new file mode 100644 index 000000000000..c477af086e65 --- /dev/null +++ b/Documentation/filesystems/efivarfs.txt @@ -0,0 +1,16 @@ + +efivarfs - a (U)EFI variable filesystem + +The efivarfs filesystem was created to address the shortcomings of +using entries in sysfs to maintain EFI variables. The old sysfs EFI +variables code only supported 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. + +Variables can be created, deleted and modified with the efivarfs +filesystem. + +efivarfs is typically mounted like this, + + mount -t efivarfs none /sys/firmware/efi/efivars From 53b87cf088e2ea68d7c59619d0214cc15bb76133 Mon Sep 17 00:00:00 2001 From: Matt Fleming Date: Fri, 7 Sep 2012 18:23:51 +0100 Subject: [PATCH 05/24] x86, mm: Include the entire kernel memory map in trampoline_pgd There are various pieces of code in arch/x86 that require a page table with an identity mapping. Make trampoline_pgd a proper kernel page table, it currently only includes the kernel text and module space mapping. One new feature of trampoline_pgd is that it now has mappings for the physical I/O device addresses, which are inserted at ioremap() time. Some broken implementations of EFI firmware require these mappings to always be around. Acked-by: Jan Beulich Signed-off-by: Matt Fleming --- arch/x86/mm/init_64.c | 9 +++- arch/x86/mm/ioremap.c | 105 +++++++++++++++++++++++++++++++++++++++ arch/x86/realmode/init.c | 17 ++++++- 3 files changed, 128 insertions(+), 3 deletions(-) diff --git a/arch/x86/mm/init_64.c b/arch/x86/mm/init_64.c index 2b6b4a3c8beb..fd4404f19d39 100644 --- a/arch/x86/mm/init_64.c +++ b/arch/x86/mm/init_64.c @@ -108,13 +108,13 @@ void sync_global_pgds(unsigned long start, unsigned long end) for (address = start; address <= end; address += PGDIR_SIZE) { const pgd_t *pgd_ref = pgd_offset_k(address); struct page *page; + pgd_t *pgd; if (pgd_none(*pgd_ref)) continue; spin_lock(&pgd_lock); list_for_each_entry(page, &pgd_list, lru) { - pgd_t *pgd; spinlock_t *pgt_lock; pgd = (pgd_t *)page_address(page) + pgd_index(address); @@ -130,6 +130,13 @@ void sync_global_pgds(unsigned long start, unsigned long end) spin_unlock(pgt_lock); } + + pgd = __va(real_mode_header->trampoline_pgd); + pgd += pgd_index(address); + + if (pgd_none(*pgd)) + set_pgd(pgd, *pgd_ref); + spin_unlock(&pgd_lock); } } diff --git a/arch/x86/mm/ioremap.c b/arch/x86/mm/ioremap.c index 78fe3f1ac49f..e190f7b56653 100644 --- a/arch/x86/mm/ioremap.c +++ b/arch/x86/mm/ioremap.c @@ -50,6 +50,107 @@ int ioremap_change_attr(unsigned long vaddr, unsigned long size, return err; } +#ifdef CONFIG_X86_64 +static void ident_pte_range(unsigned long paddr, unsigned long vaddr, + pmd_t *ppmd, pmd_t *vpmd, unsigned long end) +{ + pte_t *ppte = pte_offset_kernel(ppmd, paddr); + pte_t *vpte = pte_offset_kernel(vpmd, vaddr); + + do { + set_pte(ppte, *vpte); + } while (ppte++, vpte++, vaddr += PAGE_SIZE, vaddr != end); +} + +static int ident_pmd_range(unsigned long paddr, unsigned long vaddr, + pud_t *ppud, pud_t *vpud, unsigned long end) +{ + pmd_t *ppmd = pmd_offset(ppud, paddr); + pmd_t *vpmd = pmd_offset(vpud, vaddr); + unsigned long next; + + do { + next = pmd_addr_end(vaddr, end); + + if (!pmd_present(*ppmd)) { + pte_t *ppte = (pte_t *)get_zeroed_page(GFP_KERNEL); + if (!ppte) + return 1; + + set_pmd(ppmd, __pmd(_KERNPG_TABLE | __pa(ppte))); + } + + ident_pte_range(paddr, vaddr, ppmd, vpmd, next); + } while (ppmd++, vpmd++, vaddr = next, vaddr != end); + + return 0; +} + +static int ident_pud_range(unsigned long paddr, unsigned long vaddr, + pgd_t *ppgd, pgd_t *vpgd, unsigned long end) +{ + pud_t *ppud = pud_offset(ppgd, paddr); + pud_t *vpud = pud_offset(vpgd, vaddr); + unsigned long next; + + do { + next = pud_addr_end(vaddr, end); + + if (!pud_present(*ppud)) { + pmd_t *ppmd = (pmd_t *)get_zeroed_page(GFP_KERNEL); + if (!ppmd) + return 1; + + set_pud(ppud, __pud(_KERNPG_TABLE | __pa(ppmd))); + } + + if (ident_pmd_range(paddr, vaddr, ppud, vpud, next)) + return 1; + } while (ppud++, vpud++, vaddr = next, vaddr != end); + + return 0; +} + +static int insert_identity_mapping(resource_size_t paddr, unsigned long vaddr, + unsigned long size) +{ + unsigned long end = vaddr + size; + unsigned long next; + pgd_t *vpgd, *ppgd; + + /* Don't map over the guard hole. */ + if (paddr >= 0x800000000000 || paddr + size > 0x800000000000) + return 1; + + ppgd = __va(real_mode_header->trampoline_pgd) + pgd_index(paddr); + + vpgd = pgd_offset_k(vaddr); + do { + next = pgd_addr_end(vaddr, end); + + if (!pgd_present(*ppgd)) { + pud_t *ppud = (pud_t *)get_zeroed_page(GFP_KERNEL); + if (!ppud) + return 1; + + set_pgd(ppgd, __pgd(_KERNPG_TABLE | __pa(ppud))); + } + + if (ident_pud_range(paddr, vaddr, ppgd, vpgd, next)) + return 1; + } while (ppgd++, vpgd++, vaddr = next, vaddr != end); + + return 0; +} +#else +static inline int insert_identity_mapping(resource_size_t paddr, + unsigned long vaddr, + unsigned long size) +{ + return 0; +} +#endif /* CONFIG_X86_64 */ + /* * Remap an arbitrary physical address space into the kernel virtual * address space. Needed when the kernel wants to access high addresses @@ -163,6 +264,10 @@ static void __iomem *__ioremap_caller(resource_size_t phys_addr, ret_addr = (void __iomem *) (vaddr + offset); mmiotrace_ioremap(unaligned_phys_addr, unaligned_size, ret_addr); + if (insert_identity_mapping(phys_addr, vaddr, size)) + printk(KERN_WARNING "ioremap: unable to map 0x%llx in identity pagetable\n", + (unsigned long long)phys_addr); + /* * Check if the request spans more than any BAR in the iomem resource * tree. diff --git a/arch/x86/realmode/init.c b/arch/x86/realmode/init.c index cbca565af5bd..8e6ab6137852 100644 --- a/arch/x86/realmode/init.c +++ b/arch/x86/realmode/init.c @@ -78,8 +78,21 @@ void __init setup_real_mode(void) *trampoline_cr4_features = read_cr4(); trampoline_pgd = (u64 *) __va(real_mode_header->trampoline_pgd); - trampoline_pgd[0] = __pa(level3_ident_pgt) + _KERNPG_TABLE; - trampoline_pgd[511] = __pa(level3_kernel_pgt) + _KERNPG_TABLE; + + /* + * Create an identity mapping for all of physical memory. + */ + for (i = 0; i <= pgd_index(max_pfn << PAGE_SHIFT); i++) { + int index = pgd_index(PAGE_OFFSET) + i; + + trampoline_pgd[i] = (u64)pgd_val(swapper_pg_dir[index]); + } + + /* + * Copy the upper-half of the kernel pages tables. + */ + for (i = pgd_index(PAGE_OFFSET); i < PTRS_PER_PGD; i++) + trampoline_pgd[i] = (u64)pgd_val(swapper_pg_dir[i]); #endif } From 185034e72d591f9465e5e18f937ed642e7ea0070 Mon Sep 17 00:00:00 2001 From: Matt Fleming Date: Fri, 7 Sep 2012 18:28:04 +0100 Subject: [PATCH 06/24] x86, efi: 1:1 pagetable mapping for virtual EFI calls MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Some firmware still needs a 1:1 (virt->phys) mapping even after we've called SetVirtualAddressMap(). So install the mapping alongside our existing kernel mapping whenever we make EFI calls in virtual mode. This bug was discovered on ASUS machines where the firmware implementation of GetTime() accesses the RTC device via physical addresses, even though that's bogus per the UEFI spec since we've informed the firmware via SetVirtualAddressMap() that the boottime memory map is no longer valid. This bug seems to be present in a lot of consumer devices, so there's not a lot we can do about this spec violation apart from workaround it. Cc: JérômeCarretero Cc: Vasco Dias Acked-by: Jan Beulich Signed-off-by: Matt Fleming --- arch/x86/include/asm/efi.h | 28 +++++++++++++++++++++------- arch/x86/platform/efi/efi_64.c | 15 +++++++++++++++ 2 files changed, 36 insertions(+), 7 deletions(-) diff --git a/arch/x86/include/asm/efi.h b/arch/x86/include/asm/efi.h index c9dcc181d4d1..ae3bf3b8da71 100644 --- a/arch/x86/include/asm/efi.h +++ b/arch/x86/include/asm/efi.h @@ -69,23 +69,37 @@ extern u64 efi_call6(void *fp, u64 arg1, u64 arg2, u64 arg3, efi_call6((void *)(f), (u64)(a1), (u64)(a2), (u64)(a3), \ (u64)(a4), (u64)(a5), (u64)(a6)) +extern unsigned long efi_call_virt_prelog(void); +extern void efi_call_virt_epilog(unsigned long); + +#define efi_callx(x, func, ...) \ + ({ \ + efi_status_t __status; \ + unsigned long __pgd; \ + \ + __pgd = efi_call_virt_prelog(); \ + __status = efi_call##x(func, __VA_ARGS__); \ + efi_call_virt_epilog(__pgd); \ + __status; \ + }) + #define efi_call_virt0(f) \ - efi_call0((void *)(efi.systab->runtime->f)) + efi_callx(0, (void *)(efi.systab->runtime->f)) #define efi_call_virt1(f, a1) \ - efi_call1((void *)(efi.systab->runtime->f), (u64)(a1)) + efi_callx(1, (void *)(efi.systab->runtime->f), (u64)(a1)) #define efi_call_virt2(f, a1, a2) \ - efi_call2((void *)(efi.systab->runtime->f), (u64)(a1), (u64)(a2)) + efi_callx(2, (void *)(efi.systab->runtime->f), (u64)(a1), (u64)(a2)) #define efi_call_virt3(f, a1, a2, a3) \ - efi_call3((void *)(efi.systab->runtime->f), (u64)(a1), (u64)(a2), \ + efi_callx(3, (void *)(efi.systab->runtime->f), (u64)(a1), (u64)(a2), \ (u64)(a3)) #define efi_call_virt4(f, a1, a2, a3, a4) \ - efi_call4((void *)(efi.systab->runtime->f), (u64)(a1), (u64)(a2), \ + efi_callx(4, (void *)(efi.systab->runtime->f), (u64)(a1), (u64)(a2), \ (u64)(a3), (u64)(a4)) #define efi_call_virt5(f, a1, a2, a3, a4, a5) \ - efi_call5((void *)(efi.systab->runtime->f), (u64)(a1), (u64)(a2), \ + efi_callx(5, (void *)(efi.systab->runtime->f), (u64)(a1), (u64)(a2), \ (u64)(a3), (u64)(a4), (u64)(a5)) #define efi_call_virt6(f, a1, a2, a3, a4, a5, a6) \ - efi_call6((void *)(efi.systab->runtime->f), (u64)(a1), (u64)(a2), \ + efi_callx(6, (void *)(efi.systab->runtime->f), (u64)(a1), (u64)(a2), \ (u64)(a3), (u64)(a4), (u64)(a5), (u64)(a6)) extern void __iomem *efi_ioremap(unsigned long addr, unsigned long size, diff --git a/arch/x86/platform/efi/efi_64.c b/arch/x86/platform/efi/efi_64.c index ac3aa54e2654..ddb0174cf093 100644 --- a/arch/x86/platform/efi/efi_64.c +++ b/arch/x86/platform/efi/efi_64.c @@ -58,6 +58,21 @@ static void __init early_code_mapping_set_exec(int executable) } } +unsigned long efi_call_virt_prelog(void) +{ + unsigned long saved; + + saved = read_cr3(); + write_cr3(real_mode_header->trampoline_pgd); + + return saved; +} + +void efi_call_virt_epilog(unsigned long saved) +{ + write_cr3(saved); +} + void __init efi_call_phys_prelog(void) { unsigned long vaddress; From da5a108d05b4f350be33e62d2db125673823e7ff Mon Sep 17 00:00:00 2001 From: Xiaoyan Zhang Date: Fri, 7 Sep 2012 19:29:04 +0100 Subject: [PATCH 07/24] x86/kernel: remove tboot 1:1 page table creation code For TXT boot, while Linux kernel trys to shutdown/S3/S4/reboot, it need to jump back to tboot code and do TXT teardown work. Previously kernel zapped all mem page identity mapping (va=pa) after booting, so tboot code mem address was mapped again with identity mapping. Now kernel didn't zap the identity mapping page table, so tboot related code can remove the remapping code before trapping back now. Signed-off-by: Xiaoyan Zhang Acked-by: Gang Wei Signed-off-by: Matt Fleming --- arch/x86/kernel/tboot.c | 78 +++-------------------------------------- 1 file changed, 5 insertions(+), 73 deletions(-) diff --git a/arch/x86/kernel/tboot.c b/arch/x86/kernel/tboot.c index f84fe00fad48..d4f460f962ee 100644 --- a/arch/x86/kernel/tboot.c +++ b/arch/x86/kernel/tboot.c @@ -103,71 +103,13 @@ void __init tboot_probe(void) pr_debug("tboot_size: 0x%x\n", tboot->tboot_size); } -static pgd_t *tboot_pg_dir; -static struct mm_struct tboot_mm = { - .mm_rb = RB_ROOT, - .pgd = swapper_pg_dir, - .mm_users = ATOMIC_INIT(2), - .mm_count = ATOMIC_INIT(1), - .mmap_sem = __RWSEM_INITIALIZER(init_mm.mmap_sem), - .page_table_lock = __SPIN_LOCK_UNLOCKED(init_mm.page_table_lock), - .mmlist = LIST_HEAD_INIT(init_mm.mmlist), -}; - static inline void switch_to_tboot_pt(void) { - write_cr3(virt_to_phys(tboot_pg_dir)); -} - -static int map_tboot_page(unsigned long vaddr, unsigned long pfn, - pgprot_t prot) -{ - pgd_t *pgd; - pud_t *pud; - pmd_t *pmd; - pte_t *pte; - - pgd = pgd_offset(&tboot_mm, vaddr); - pud = pud_alloc(&tboot_mm, pgd, vaddr); - if (!pud) - return -1; - pmd = pmd_alloc(&tboot_mm, pud, vaddr); - if (!pmd) - return -1; - pte = pte_alloc_map(&tboot_mm, NULL, pmd, vaddr); - if (!pte) - return -1; - set_pte_at(&tboot_mm, vaddr, pte, pfn_pte(pfn, prot)); - pte_unmap(pte); - return 0; -} - -static int map_tboot_pages(unsigned long vaddr, unsigned long start_pfn, - unsigned long nr) -{ - /* Reuse the original kernel mapping */ - tboot_pg_dir = pgd_alloc(&tboot_mm); - if (!tboot_pg_dir) - return -1; - - for (; nr > 0; nr--, vaddr += PAGE_SIZE, start_pfn++) { - if (map_tboot_page(vaddr, start_pfn, PAGE_KERNEL_EXEC)) - return -1; - } - - return 0; -} - -static void tboot_create_trampoline(void) -{ - u32 map_base, map_size; - - /* Create identity map for tboot shutdown code. */ - map_base = PFN_DOWN(tboot->tboot_base); - map_size = PFN_UP(tboot->tboot_size); - if (map_tboot_pages(map_base << PAGE_SHIFT, map_base, map_size)) - panic("tboot: Error mapping tboot pages (mfns) @ 0x%x, 0x%x\n", - map_base, map_size); +#ifdef CONFIG_X86_32 + load_cr3(initial_page_table); +#else + write_cr3(real_mode_header->trampoline_pgd); +#endif } #ifdef CONFIG_ACPI_SLEEP @@ -225,14 +167,6 @@ void tboot_shutdown(u32 shutdown_type) if (!tboot_enabled()) return; - /* - * if we're being called before the 1:1 mapping is set up then just - * return and let the normal shutdown happen; this should only be - * due to very early panic() - */ - if (!tboot_pg_dir) - return; - /* if this is S3 then set regions to MAC */ if (shutdown_type == TB_SHUTDOWN_S3) if (tboot_setup_sleep()) @@ -343,8 +277,6 @@ static __init int tboot_late_init(void) if (!tboot_enabled()) return 0; - tboot_create_trampoline(); - atomic_set(&ap_wfs_count, 0); register_hotcpu_notifier(&tboot_cpu_notifier); From bd52276fa1d420c3a504b76ffaaa1642cc79d4c4 Mon Sep 17 00:00:00 2001 From: Jan Beulich Date: Fri, 25 May 2012 16:20:31 +0100 Subject: [PATCH 08/24] x86-64/efi: Use EFI to deal with platform wall clock (again) Other than ix86, x86-64 on EFI so far didn't set the {g,s}et_wallclock accessors to the EFI routines, thus incorrectly using raw RTC accesses instead. Simply removing the #ifdef around the respective code isn't enough, however: While so far early get-time calls were done in physical mode, this doesn't work properly for x86-64, as virtual addresses would still need to be set up for all runtime regions (which wasn't the case on the system I have access to), so instead the patch moves the call to efi_enter_virtual_mode() ahead (which in turn allows to drop all code related to calling efi-get-time in physical mode). Additionally the earlier calling of efi_set_executable() requires the CPA code to cope, i.e. during early boot it must be avoided to call cpa_flush_array(), as the first thing this function does is a BUG_ON(irqs_disabled()). Also make the two EFI functions in question here static - they're not being referenced elsewhere. History: This commit was originally merged as bacef661acdb ("x86-64/efi: Use EFI to deal with platform wall clock") but it resulted in some ASUS machines no longer booting due to a firmware bug, and so was reverted in f026cfa82f62. A pre-emptive fix for the buggy ASUS firmware was merged in 03a1c254975e ("x86, efi: 1:1 pagetable mapping for virtual EFI calls") so now this patch can be reapplied. Signed-off-by: Jan Beulich Tested-by: Matt Fleming Acked-by: Matthew Garrett Cc: Ingo Molnar Cc: Peter Zijlstra Cc: H. Peter Anvin Signed-off-by: Matt Fleming [added commit history] --- arch/x86/mm/pageattr.c | 10 ++++++---- arch/x86/platform/efi/efi.c | 30 ++++-------------------------- include/linux/efi.h | 2 -- init/main.c | 8 ++++---- 4 files changed, 14 insertions(+), 36 deletions(-) diff --git a/arch/x86/mm/pageattr.c b/arch/x86/mm/pageattr.c index a718e0d23503..931930a96160 100644 --- a/arch/x86/mm/pageattr.c +++ b/arch/x86/mm/pageattr.c @@ -919,11 +919,13 @@ static int change_page_attr_set_clr(unsigned long *addr, int numpages, /* * On success we use clflush, when the CPU supports it to - * avoid the wbindv. If the CPU does not support it and in the - * error case we fall back to cpa_flush_all (which uses - * wbindv): + * avoid the wbindv. If the CPU does not support it, in the + * error case, and during early boot (for EFI) we fall back + * to cpa_flush_all (which uses wbinvd): */ - if (!ret && cpu_has_clflush) { + if (early_boot_irqs_disabled) + __cpa_flush_all((void *)(long)cache); + else if (!ret && cpu_has_clflush) { if (cpa.flags & (CPA_PAGES_ARRAY | CPA_ARRAY)) { cpa_flush_array(addr, numpages, cache, cpa.flags, pages); diff --git a/arch/x86/platform/efi/efi.c b/arch/x86/platform/efi/efi.c index aded2a91162a..757834434e59 100644 --- a/arch/x86/platform/efi/efi.c +++ b/arch/x86/platform/efi/efi.c @@ -235,22 +235,7 @@ static efi_status_t __init phys_efi_set_virtual_address_map( return status; } -static efi_status_t __init phys_efi_get_time(efi_time_t *tm, - efi_time_cap_t *tc) -{ - unsigned long flags; - efi_status_t status; - - spin_lock_irqsave(&rtc_lock, flags); - efi_call_phys_prelog(); - status = efi_call_phys2(efi_phys.get_time, virt_to_phys(tm), - virt_to_phys(tc)); - efi_call_phys_epilog(); - spin_unlock_irqrestore(&rtc_lock, flags); - return status; -} - -int efi_set_rtc_mmss(unsigned long nowtime) +static int efi_set_rtc_mmss(unsigned long nowtime) { int real_seconds, real_minutes; efi_status_t status; @@ -279,7 +264,7 @@ int efi_set_rtc_mmss(unsigned long nowtime) return 0; } -unsigned long efi_get_time(void) +static unsigned long efi_get_time(void) { efi_status_t status; efi_time_t eft; @@ -635,18 +620,13 @@ static int __init efi_runtime_init(void) } /* * We will only need *early* access to the following - * two EFI runtime services before set_virtual_address_map + * EFI runtime service before set_virtual_address_map * is invoked. */ - efi_phys.get_time = (efi_get_time_t *)runtime->get_time; efi_phys.set_virtual_address_map = (efi_set_virtual_address_map_t *) runtime->set_virtual_address_map; - /* - * Make efi_get_time can be called before entering - * virtual mode. - */ - efi.get_time = phys_efi_get_time; + early_iounmap(runtime, sizeof(efi_runtime_services_t)); return 0; @@ -734,12 +714,10 @@ void __init efi_init(void) efi_enabled = 0; return; } -#ifdef CONFIG_X86_32 if (efi_native) { x86_platform.get_wallclock = efi_get_time; x86_platform.set_wallclock = efi_set_rtc_mmss; } -#endif #if EFI_DEBUG print_efi_memmap(); diff --git a/include/linux/efi.h b/include/linux/efi.h index 337aefbfb003..5e2308d9c6be 100644 --- a/include/linux/efi.h +++ b/include/linux/efi.h @@ -516,8 +516,6 @@ extern u64 efi_mem_attribute (unsigned long phys_addr, unsigned long size); extern int __init efi_uart_console_only (void); extern void efi_initialize_iomem_resources(struct resource *code_resource, struct resource *data_resource, struct resource *bss_resource); -extern unsigned long efi_get_time(void); -extern int efi_set_rtc_mmss(unsigned long nowtime); extern void efi_reserve_boot_services(void); extern struct efi_memory_map memmap; diff --git a/init/main.c b/init/main.c index 9cf77ab138a6..ae70b647b4d9 100644 --- a/init/main.c +++ b/init/main.c @@ -461,6 +461,10 @@ static void __init mm_init(void) percpu_init_late(); pgtable_cache_init(); vmalloc_init(); +#ifdef CONFIG_X86 + if (efi_enabled) + efi_enter_virtual_mode(); +#endif } asmlinkage void __init start_kernel(void) @@ -601,10 +605,6 @@ asmlinkage void __init start_kernel(void) calibrate_delay(); pidmap_init(); anon_vma_init(); -#ifdef CONFIG_X86 - if (efi_enabled) - efi_enter_virtual_mode(); -#endif thread_info_cache_init(); cred_init(); fork_init(totalram_pages); From d142df03a798ee7d2db10a1f20945110ea6067ff Mon Sep 17 00:00:00 2001 From: Andy Whitcroft Date: Thu, 11 Oct 2012 11:32:17 +0100 Subject: [PATCH 09/24] efivarfs: efivarfs_file_read ensure we free data in error paths Signed-off-by: Andy Whitcroft Acked-by: Matthew Garrett Acked-by: Jeremy Kerr Signed-off-by: Matt Fleming --- drivers/firmware/efivars.c | 6 +++--- 1 file changed, 3 insertions(+), 3 deletions(-) diff --git a/drivers/firmware/efivars.c b/drivers/firmware/efivars.c index 6793965b7c8b..b7c9a3261dc4 100644 --- a/drivers/firmware/efivars.c +++ b/drivers/firmware/efivars.c @@ -766,7 +766,7 @@ static ssize_t efivarfs_file_read(struct file *file, char __user *userbuf, 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 file *file, char __user *userbuf, &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; From 45a937a883c4411648b80e87341b237cc48009c1 Mon Sep 17 00:00:00 2001 From: Andy Whitcroft Date: Thu, 11 Oct 2012 11:32:18 +0100 Subject: [PATCH 10/24] efivarfs: efivarfs_create() ensure we drop our reference on inode on error Signed-off-by: Andy Whitcroft Acked-by: Matthew Garrett Acked-by: Jeremy Kerr Signed-off-by: Matt Fleming --- drivers/firmware/efivars.c | 14 +++++++++----- 1 file changed, 9 insertions(+), 5 deletions(-) diff --git a/drivers/firmware/efivars.c b/drivers/firmware/efivars.c index b7c9a3261dc4..6e5f367145f5 100644 --- a/drivers/firmware/efivars.c +++ b/drivers/firmware/efivars.c @@ -866,7 +866,7 @@ static void efivarfs_hex_to_guid(const char *str, efi_guid_t *guid) 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 inode *inode; struct efivars *efivars = &__efivars; struct efivar_entry *var; int namelen, i = 0, err = 0; @@ -874,13 +874,15 @@ static int efivarfs_create(struct inode *dir, struct dentry *dentry, if (dentry->d_name.len < 38) return -EINVAL; + inode = efivarfs_get_inode(dir->i_sb, dir, mode, 0); if (!inode) return -ENOSPC; var = kzalloc(sizeof(struct efivar_entry), GFP_KERNEL); - - if (!var) - return -ENOMEM; + if (!var) { + err = -ENOMEM; + goto out; + } namelen = dentry->d_name.len - GUID_LEN; @@ -908,8 +910,10 @@ static int efivarfs_create(struct inode *dir, struct dentry *dentry, d_instantiate(dentry, inode); dget(dentry); out: - if (err) + if (err) { kfree(var); + iput(inode); + } return err; } From 5c9b50ab8ca962bc085cfb49bae947910d73f794 Mon Sep 17 00:00:00 2001 From: Andy Whitcroft Date: Thu, 11 Oct 2012 11:32:19 +0100 Subject: [PATCH 11/24] efivarfs: efivarfs_fill_super() fix inode reference counts 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 --- drivers/firmware/efivars.c | 16 ++++------------ 1 file changed, 4 insertions(+), 12 deletions(-) diff --git a/drivers/firmware/efivars.c b/drivers/firmware/efivars.c index 6e5f367145f5..adfc4863202b 100644 --- a/drivers/firmware/efivars.c +++ b/drivers/firmware/efivars.c @@ -948,7 +948,6 @@ int efivarfs_fill_super(struct super_block *sb, void *data, int silent) 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_block *sb, void *data, int silent) 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_block *sb, void *data, int silent) } return 0; -fail: - iput(inode); - return err; } static struct dentry *efivarfs_mount(struct file_system_type *fs_type, From c0359db1a1c484ea810f4d5a877d3b00d107908b Mon Sep 17 00:00:00 2001 From: Andy Whitcroft Date: Thu, 11 Oct 2012 11:32:20 +0100 Subject: [PATCH 12/24] efivarfs: efivarfs_fill_super() ensure we free our temporary name d_alloc_name() copies the passed name to new storage, once complete we no longer need our name. Signed-off-by: Andy Whitcroft Acked-by: Matthew Garrett Acked-by: Jeremy Kerr Signed-off-by: Matt Fleming --- drivers/firmware/efivars.c | 2 ++ 1 file changed, 2 insertions(+) diff --git a/drivers/firmware/efivars.c b/drivers/firmware/efivars.c index adfc4863202b..36b3dd6e1e5e 100644 --- a/drivers/firmware/efivars.c +++ b/drivers/firmware/efivars.c @@ -992,6 +992,8 @@ int efivarfs_fill_super(struct super_block *sb, void *data, int silent) inode = efivarfs_get_inode(efivarfs_sb, root->d_inode, S_IFREG | 0644, 0); dentry = d_alloc_name(root, name); + /* copied by the above to local storage in the dentry. */ + kfree(name); efivars->ops->get_variable(entry->var.VariableName, &entry->var.VendorGuid, From 5ba6e2919b9e18a051e5bdd6c21f52ee4976513f Mon Sep 17 00:00:00 2001 From: Andy Whitcroft Date: Thu, 11 Oct 2012 11:32:21 +0100 Subject: [PATCH 13/24] efivarfs: efivarfs_fill_super() ensure we clean up correctly on error 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 --- drivers/firmware/efivars.c | 20 ++++++++++++++++++-- 1 file changed, 18 insertions(+), 2 deletions(-) diff --git a/drivers/firmware/efivars.c b/drivers/firmware/efivars.c index 36b3dd6e1e5e..216086d88380 100644 --- a/drivers/firmware/efivars.c +++ b/drivers/firmware/efivars.c @@ -948,6 +948,7 @@ int efivarfs_fill_super(struct super_block *sb, void *data, int silent) 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_block *sb, void *data, int silent) 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_block *sb, void *data, int silent) 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_block *sb, void *data, int silent) } return 0; + +fail_inode: + iput(inode); +fail_name: + kfree(name); +fail: + return -ENOMEM; } static struct dentry *efivarfs_mount(struct file_system_type *fs_type, From f5f6a60ad52fc786c517303590f1efaea614c69b Mon Sep 17 00:00:00 2001 From: Jeremy Kerr Date: Thu, 11 Oct 2012 21:19:11 +0800 Subject: [PATCH 14/24] efivarfs: Implement exclusive access for {get,set}_variable Currently, efivarfs does not enforce exclusion over the get_variable and set_variable operations. Section 7.1 of UEFI requires us to only allow a single processor to enter {get,set}_variable services at once. This change acquires the efivars->lock over calls to these operations from the efivarfs paths. Signed-off-by: Jeremy Kerr Signed-off-by: Matt Fleming --- drivers/firmware/efivars.c | 68 ++++++++++++++++++++++++-------------- 1 file changed, 43 insertions(+), 25 deletions(-) diff --git a/drivers/firmware/efivars.c b/drivers/firmware/efivars.c index 216086d88380..d478c568ce4f 100644 --- a/drivers/firmware/efivars.c +++ b/drivers/firmware/efivars.c @@ -690,35 +690,45 @@ static ssize_t efivarfs_file_write(struct file *file, goto out; } + /* + * The lock here protects the get_variable call, the conditional + * set_variable call, and removal of the variable from the efivars + * list (in the case of an authenticated delete). + */ + spin_lock(&efivars->lock); + status = efivars->ops->set_variable(var->var.VariableName, &var->var.VendorGuid, attributes, datasize, data); - switch (status) { - case EFI_SUCCESS: - break; - case EFI_INVALID_PARAMETER: - count = -EINVAL; - goto out; - case EFI_OUT_OF_RESOURCES: - count = -ENOSPC; - goto out; - case EFI_DEVICE_ERROR: - count = -EIO; - goto out; - case EFI_WRITE_PROTECTED: - count = -EROFS; - goto out; - case EFI_SECURITY_VIOLATION: - count = -EACCES; - goto out; - case EFI_NOT_FOUND: - count = -ENOENT; - goto out; - default: - count = -EINVAL; - goto out; + if (status != EFI_SUCCESS) { + 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; } /* @@ -734,12 +744,12 @@ static ssize_t efivarfs_file_write(struct file *file, NULL); if (status == EFI_BUFFER_TOO_SMALL) { + spin_unlock(&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) { - spin_lock(&efivars->lock); list_del(&var->list); spin_unlock(&efivars->lock); efivar_unregister(var); @@ -747,6 +757,7 @@ static ssize_t efivarfs_file_write(struct file *file, dput(file->f_dentry); } else { + spin_unlock(&efivars->lock); pr_warn("efivarfs: inconsistent EFI variable implementation? " "status = %lx\n", status); } @@ -768,9 +779,11 @@ static ssize_t efivarfs_file_read(struct file *file, char __user *userbuf, void *data; ssize_t size = 0; + spin_lock(&efivars->lock); status = efivars->ops->get_variable(var->var.VariableName, &var->var.VendorGuid, &attributes, &datasize, NULL); + spin_unlock(&efivars->lock); if (status != EFI_BUFFER_TOO_SMALL) return 0; @@ -780,10 +793,13 @@ static ssize_t efivarfs_file_read(struct file *file, char __user *userbuf, if (!data) return 0; + spin_lock(&efivars->lock); status = efivars->ops->get_variable(var->var.VariableName, &var->var.VendorGuid, &attributes, &datasize, (data + 4)); + spin_unlock(&efivars->lock); + if (status != EFI_SUCCESS) goto out_free; @@ -1004,11 +1020,13 @@ int efivarfs_fill_super(struct super_block *sb, void *data, int silent) /* copied by the above to local storage in the dentry. */ kfree(name); + spin_lock(&efivars->lock); efivars->ops->get_variable(entry->var.VariableName, &entry->var.VendorGuid, &entry->var.Attributes, &size, NULL); + spin_unlock(&efivars->lock); mutex_lock(&inode->i_mutex); inode->i_private = entry; From 310ad75448329f167af3c4a10e4a9de4d80058ff Mon Sep 17 00:00:00 2001 From: Jeremy Kerr Date: Fri, 19 Oct 2012 15:16:45 +0800 Subject: [PATCH 15/24] efi: Clarify GUID length calculations 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 Signed-off-by: Matt Fleming --- drivers/firmware/efivars.c | 33 +++++++++++++++++++++++++-------- 1 file changed, 25 insertions(+), 8 deletions(-) diff --git a/drivers/firmware/efivars.c b/drivers/firmware/efivars.c index d478c568ce4f..a93e401c20a7 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 @@ -887,7 +892,11 @@ 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); @@ -900,7 +909,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); @@ -994,8 +1004,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; @@ -1006,7 +1016,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); @@ -1435,11 +1445,18 @@ 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_char16_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) { From 7253eaba7b179db2e07e67c5b78d5f10b332291d Mon Sep 17 00:00:00 2001 From: Matt Fleming Date: Tue, 16 Oct 2012 15:58:07 +0100 Subject: [PATCH 16/24] efivarfs: Return an error if we fail to read a variable 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 --- drivers/firmware/efivars.c | 62 ++++++++++++++++++++++---------------- 1 file changed, 36 insertions(+), 26 deletions(-) diff --git a/drivers/firmware/efivars.c b/drivers/firmware/efivars.c index a93e401c20a7..277e42616f0d 100644 --- a/drivers/firmware/efivars.c +++ b/drivers/firmware/efivars.c @@ -653,6 +653,36 @@ static int efivarfs_file_open(struct inode *inode, struct file *file) 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) { @@ -711,29 +741,7 @@ static ssize_t efivarfs_file_write(struct file *file, 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); } /* @@ -791,12 +799,12 @@ static ssize_t efivarfs_file_read(struct file *file, char __user *userbuf, 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, @@ -805,8 +813,10 @@ static ssize_t efivarfs_file_read(struct file *file, char __user *userbuf, (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, From d292384152fdd465dba792d555ce5db83e94efa0 Mon Sep 17 00:00:00 2001 From: Matt Fleming Date: Mon, 22 Oct 2012 15:23:29 +0100 Subject: [PATCH 17/24] efivarfs: Replace magic number with sizeof(attributes) 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 --- drivers/firmware/efivars.c | 8 ++++---- 1 file changed, 4 insertions(+), 4 deletions(-) diff --git a/drivers/firmware/efivars.c b/drivers/firmware/efivars.c index 277e42616f0d..2c044343c99b 100644 --- a/drivers/firmware/efivars.c +++ b/drivers/firmware/efivars.c @@ -801,7 +801,7 @@ static ssize_t efivarfs_file_read(struct file *file, char __user *userbuf, 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 file *file, char __user *userbuf, 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 file *file, char __user *userbuf, 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); From 91716322d834cba34f4a7ed5e4a39673eb90862b Mon Sep 17 00:00:00 2001 From: Matt Fleming Date: Mon, 22 Oct 2012 15:51:45 +0100 Subject: [PATCH 18/24] efivarfs: Add unique magic number Using pstore's superblock magic number is no doubt going to cause problems in the future. Give efivarfs its own magic number. Acked-by: Jeremy Kerr Signed-off-by: Matt Fleming --- drivers/firmware/efivars.c | 2 +- include/uapi/linux/magic.h | 1 + 2 files changed, 2 insertions(+), 1 deletion(-) diff --git a/drivers/firmware/efivars.c b/drivers/firmware/efivars.c index 2c044343c99b..3b0cf9acb504 100644 --- a/drivers/firmware/efivars.c +++ b/drivers/firmware/efivars.c @@ -991,7 +991,7 @@ int efivarfs_fill_super(struct super_block *sb, void *data, int silent) 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_magic = EFIVARFS_MAGIC; sb->s_op = &efivarfs_ops; sb->s_time_gran = 1; diff --git a/include/uapi/linux/magic.h b/include/uapi/linux/magic.h index e15192cb9cf4..12f68c7ceba6 100644 --- a/include/uapi/linux/magic.h +++ b/include/uapi/linux/magic.h @@ -27,6 +27,7 @@ #define ISOFS_SUPER_MAGIC 0x9660 #define JFFS2_SUPER_MAGIC 0x72b6 #define PSTOREFS_MAGIC 0x6165676C +#define EFIVARFS_MAGIC 0xde5e81e4 #define MINIX_SUPER_MAGIC 0x137F /* minix v1 fs, 14 char names */ #define MINIX_SUPER_MAGIC2 0x138F /* minix v1 fs, 30 char names */ From 07b1c5bc64cff9c880261a1fef562ef7ea7f6575 Mon Sep 17 00:00:00 2001 From: Matt Fleming Date: Tue, 23 Oct 2012 12:35:43 +0100 Subject: [PATCH 19/24] efivarfs: Make 'datasize' unsigned long 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 --- drivers/firmware/efivars.c | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/drivers/firmware/efivars.c b/drivers/firmware/efivars.c index 3b0cf9acb504..6a858d1a5bb3 100644 --- a/drivers/firmware/efivars.c +++ b/drivers/firmware/efivars.c @@ -692,7 +692,7 @@ static ssize_t efivarfs_file_write(struct file *file, 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)) From aeeaa8d46aa38c9cc5fac23feb9b1f91bdbf5dd3 Mon Sep 17 00:00:00 2001 From: Matt Fleming Date: Tue, 23 Oct 2012 12:41:03 +0100 Subject: [PATCH 20/24] efivarfs: Return a consistent error when efivarfs_get_inode() fails Instead of returning -ENOSPC if efivarfs_get_inode() fails we should be returning -ENOMEM, since running out of memory is the only reason it can fail. Furthermore, that's the error value used everywhere else in this file. It's also less likely to confuse users that hit this error case. Acked-by: Jeremy Kerr Signed-off-by: Matt Fleming --- drivers/firmware/efivars.c | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/drivers/firmware/efivars.c b/drivers/firmware/efivars.c index 6a858d1a5bb3..58cec627a821 100644 --- a/drivers/firmware/efivars.c +++ b/drivers/firmware/efivars.c @@ -911,7 +911,7 @@ static int efivarfs_create(struct inode *dir, struct dentry *dentry, inode = efivarfs_get_inode(dir->i_sb, dir, mode, 0); if (!inode) - return -ENOSPC; + return -ENOMEM; var = kzalloc(sizeof(struct efivar_entry), GFP_KERNEL); if (!var) { From cfcf2f11708f934d2bd294f973c2fcb0cc54f293 Mon Sep 17 00:00:00 2001 From: Matt Fleming Date: Fri, 26 Oct 2012 12:18:53 +0100 Subject: [PATCH 21/24] efivarfs: Fix return value of efivarfs_file_write() 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 --- drivers/firmware/efivars.c | 13 ++++++++----- 1 file changed, 8 insertions(+), 5 deletions(-) diff --git a/drivers/firmware/efivars.c b/drivers/firmware/efivars.c index 58cec627a821..9ac934018bba 100644 --- a/drivers/firmware/efivars.c +++ b/drivers/firmware/efivars.c @@ -694,6 +694,7 @@ static ssize_t efivarfs_file_write(struct file *file, 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(struct file *file, 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(struct file *file, 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(struct file *file, out: kfree(data); - return count; + return bytes; } static ssize_t efivarfs_file_read(struct file *file, char __user *userbuf, From 89d16665d388837b30972081d97b814be26d68a2 Mon Sep 17 00:00:00 2001 From: Matt Fleming Date: Fri, 9 Nov 2012 21:02:56 +0000 Subject: [PATCH 22/24] efivarfs: Use query_variable_info() to limit kmalloc() We don't want someone who can write EFI variables to be able to allocate arbitrarily large amounts of memory, so cap it to something sensible like the amount of free space for EFI variables. Acked-by: Jeremy Kerr Cc: Matthew Garrett Cc: Alan Cox Signed-off-by: Matt Fleming --- drivers/firmware/efivars.c | 43 ++++++++++++++++++++++++++++++-------- include/linux/efi.h | 1 + 2 files changed, 35 insertions(+), 9 deletions(-) diff --git a/drivers/firmware/efivars.c b/drivers/firmware/efivars.c index 9ac934018bba..d6b8d2f628fa 100644 --- a/drivers/firmware/efivars.c +++ b/drivers/firmware/efivars.c @@ -694,28 +694,51 @@ static ssize_t efivarfs_file_write(struct file *file, struct inode *inode = file->f_mapping->host; unsigned long datasize = count - sizeof(attributes); unsigned long newdatasize; + u64 storage_size, remaining_size, max_size; ssize_t bytes = 0; if (count < sizeof(attributes)) return -EINVAL; - data = kmalloc(datasize, GFP_KERNEL); + if (copy_from_user(&attributes, userbuf, sizeof(attributes))) + return -EFAULT; - if (!data) - return -ENOMEM; + if (attributes & ~(EFI_VARIABLE_MASK)) + return -EINVAL; efivars = var->efivars; - if (copy_from_user(&attributes, userbuf, sizeof(attributes))) { - bytes = -EFAULT; - goto out; + /* + * Ensure that the user can't allocate arbitrarily large + * amounts of memory. Pick a default size of 64K if + * QueryVariableInfo() isn't supported by the firmware. + */ + spin_lock(&efivars->lock); + + if (!efivars->ops->query_variable_info) + status = EFI_UNSUPPORTED; + else { + const struct efivar_operations *fops = efivars->ops; + status = fops->query_variable_info(attributes, &storage_size, + &remaining_size, &max_size); } - if (attributes & ~(EFI_VARIABLE_MASK)) { - bytes = -EINVAL; - goto out; + spin_unlock(&efivars->lock); + + if (status != EFI_SUCCESS) { + if (status != EFI_UNSUPPORTED) + return efi_status_to_err(status); + + remaining_size = 65536; } + if (datasize > remaining_size) + return -ENOSPC; + + data = kmalloc(datasize, GFP_KERNEL); + if (!data) + return -ENOMEM; + if (copy_from_user(data, userbuf + sizeof(attributes), datasize)) { bytes = -EFAULT; goto out; @@ -1709,6 +1732,8 @@ efivars_init(void) ops.get_variable = efi.get_variable; ops.set_variable = efi.set_variable; ops.get_next_variable = efi.get_next_variable; + ops.query_variable_info = efi.query_variable_info; + error = register_efivars(&__efivars, &ops, efi_kobj); if (error) goto err_put; diff --git a/include/linux/efi.h b/include/linux/efi.h index 5e2308d9c6be..f80079cd84f4 100644 --- a/include/linux/efi.h +++ b/include/linux/efi.h @@ -646,6 +646,7 @@ struct efivar_operations { efi_get_variable_t *get_variable; efi_get_next_variable_t *get_next_variable; efi_set_variable_t *set_variable; + efi_query_variable_info_t *query_variable_info; }; struct efivars { From 5d6d578c170bb280db5d4779f666e456f4f82ec5 Mon Sep 17 00:00:00 2001 From: Jan Beulich Date: Wed, 7 Nov 2012 16:46:08 +0000 Subject: [PATCH 23/24] x86, efi: Check table header length in efi_bgrt_init() Header length should be validated for all ACPI tables before accessing any non-header field. Signed-off-by: Jan Beulich Link: http://lkml.kernel.org/r/509A9E6002000078000A7079@nat28.tlf.novell.com Acked-by: Matt Fleming Signed-off-by: H. Peter Anvin --- arch/x86/platform/efi/efi-bgrt.c | 2 ++ 1 file changed, 2 insertions(+) diff --git a/arch/x86/platform/efi/efi-bgrt.c b/arch/x86/platform/efi/efi-bgrt.c index f6a0c1b8e518..d9c1b95af17c 100644 --- a/arch/x86/platform/efi/efi-bgrt.c +++ b/arch/x86/platform/efi/efi-bgrt.c @@ -39,6 +39,8 @@ void efi_bgrt_init(void) if (ACPI_FAILURE(status)) return; + if (bgrt_tab->header.length < sizeof(*bgrt_tab)) + return; if (bgrt_tab->version != 1) return; if (bgrt_tab->image_type != 0 || !bgrt_tab->image_address) From e83af1f18c78c7b6aa720beecc927ecc8afd3647 Mon Sep 17 00:00:00 2001 From: Matt Fleming Date: Thu, 15 Nov 2012 20:03:16 +0000 Subject: [PATCH 24/24] efivarfs: Make efivarfs_fill_super() static 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 --- drivers/firmware/efivars.c | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/drivers/firmware/efivars.c b/drivers/firmware/efivars.c index d6b8d2f628fa..47aae91ff72c 100644 --- a/drivers/firmware/efivars.c +++ b/drivers/firmware/efivars.c @@ -1004,7 +1004,7 @@ static int efivarfs_unlink(struct inode *dir, struct dentry *dentry) 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;