Re: [PATCH] efivarfs: Implement exclusive access for {get,set}_variable

2012-10-11 Thread Jeremy Kerr

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

2012-10-11 Thread Greg KH
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

2012-10-11 Thread Matt Fleming
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

2012-10-11 Thread Jeremy Kerr
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

2012-10-11 Thread Jeremy Kerr
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

2012-10-11 Thread Matt Fleming
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

2012-10-11 Thread Greg KH
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

2012-10-11 Thread Jeremy Kerr

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