Re: [PATCH] efivarfs: Implement exclusive access for {get,set}_variable
Hi Greg, Should this be backported to the stable kernels? No, the efivarfs code that this touches was only recently committed; it won't be in any of the stable series. Cheers, Jeremy -- To unsubscribe from this list: send the line "unsubscribe linux-efi" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH] efivarfs: Implement exclusive access for {get,set}_variable
On Thu, Oct 11, 2012 at 02:42:36PM +0100, Matt Fleming wrote: > On Thu, 2012-10-11 at 21:19 +0800, Jeremy Kerr wrote: > > 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 > > > > --- > > drivers/firmware/efivars.c | 68 +++-- > > 1 file changed, 43 insertions(+), 25 deletions(-) > > Thanks, applied to 'next'. Should this be backported to the stable kernels? thanks, greg k-h -- To unsubscribe from this list: send the line "unsubscribe linux-kernel" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html Please read the FAQ at http://www.tux.org/lkml/
Re: [PATCH] efivarfs: Implement exclusive access for {get,set}_variable
On Thu, 2012-10-11 at 21:19 +0800, Jeremy Kerr wrote: > 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 > > --- > drivers/firmware/efivars.c | 68 +++-- > 1 file changed, 43 insertions(+), 25 deletions(-) Thanks, applied to 'next'. -- To unsubscribe from this list: send the line "unsubscribe linux-kernel" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html Please read the FAQ at http://www.tux.org/lkml/
[PATCH] 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 --- 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 5765664..a86eb55 100644 --- a/drivers/firmware/efivars.c +++ b/drivers/firmware/efivars.c @@ -679,35 +679,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(>lock); + status = efivars->ops->set_variable(var->var.VariableName, >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(>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; } /* @@ -723,12 +733,12 @@ static ssize_t efivarfs_file_write(struct file *file, NULL); if (status == EFI_BUFFER_TOO_SMALL) { + spin_unlock(>lock); mutex_lock(>i_mutex); i_size_write(inode, newdatasize + sizeof(attributes)); mutex_unlock(>i_mutex); } else if (status == EFI_NOT_FOUND) { - spin_lock(>lock); list_del(>list); spin_unlock(>lock); efivar_unregister(var); @@ -736,6 +746,7 @@ static ssize_t efivarfs_file_write(struct file *file, dput(file->f_dentry); } else { + spin_unlock(>lock); pr_warn("efivarfs: inconsistent EFI variable implementation? " "status = %lx\n", status); } @@ -757,9 +768,11 @@ static ssize_t efivarfs_file_read(struct file *file, char __user *userbuf, void *data; ssize_t size = 0; + spin_lock(>lock); status = efivars->ops->get_variable(var->var.VariableName, >var.VendorGuid, , , NULL); + spin_unlock(>lock); if (status != EFI_BUFFER_TOO_SMALL) return 0; @@ -769,10 +782,13 @@ static ssize_t efivarfs_file_read(struct file *file, char __user *userbuf, if (!data) return 0; + spin_lock(>lock); status = efivars->ops->get_variable(var->var.VariableName, >var.VendorGuid, , , (data + 4)); + spin_unlock(>lock); + if (status != EFI_SUCCESS) goto out_free; @@ -993,11 +1009,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(>lock);
[PATCH] 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 jeremy.k...@canonical.com --- 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 5765664..a86eb55 100644 --- a/drivers/firmware/efivars.c +++ b/drivers/firmware/efivars.c @@ -679,35 +679,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; } /* @@ -723,12 +733,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); @@ -736,6 +746,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); } @@ -757,9 +768,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; @@ -769,10 +782,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; @@ -993,11 +1009,13 @@ int efivarfs_fill_super(struct super_block *sb, void *data, int silent) /* copied by the above to
Re: [PATCH] efivarfs: Implement exclusive access for {get,set}_variable
On Thu, 2012-10-11 at 21:19 +0800, Jeremy Kerr wrote: 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 jeremy.k...@canonical.com --- drivers/firmware/efivars.c | 68 +++-- 1 file changed, 43 insertions(+), 25 deletions(-) Thanks, applied to 'next'. -- To unsubscribe from this list: send the line unsubscribe linux-kernel in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html Please read the FAQ at http://www.tux.org/lkml/
Re: [PATCH] efivarfs: Implement exclusive access for {get,set}_variable
On Thu, Oct 11, 2012 at 02:42:36PM +0100, Matt Fleming wrote: On Thu, 2012-10-11 at 21:19 +0800, Jeremy Kerr wrote: 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 jeremy.k...@canonical.com --- drivers/firmware/efivars.c | 68 +++-- 1 file changed, 43 insertions(+), 25 deletions(-) Thanks, applied to 'next'. Should this be backported to the stable kernels? thanks, greg k-h -- To unsubscribe from this list: send the line unsubscribe linux-kernel in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html Please read the FAQ at http://www.tux.org/lkml/
Re: [PATCH] efivarfs: Implement exclusive access for {get,set}_variable
Hi Greg, Should this be backported to the stable kernels? No, the efivarfs code that this touches was only recently committed; it won't be in any of the stable series. Cheers, Jeremy -- To unsubscribe from this list: send the line unsubscribe linux-efi in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html