All of lore.kernel.org
 help / color / mirror / Atom feed
* [patch] efi: locking fix in efivar_entry_set_safe()
@ 2013-04-30  7:43 ` Dan Carpenter
  0 siblings, 0 replies; 4+ messages in thread
From: Dan Carpenter @ 2013-04-30  7:43 UTC (permalink / raw)
  To: Matt Fleming, Tom Gundersen
  Cc: linux-efi-u79uwXL29TY76Z2rM5mHXA, kernel-janitors-u79uwXL29TY76Z2rM5mHXA

The intent is that if we aren't allowed to block because we're in an
NMI or an emergency then we only take the lock if it is uncontended.

Part of the problem is the test is reversed so we return -EBUSY if we
acquire the lock.

Signed-off-by: Dan Carpenter <dan.carpenter@oracle.com>
---
Static checker stuff.  I haven't tested this.

diff --git a/drivers/firmware/efi/vars.c b/drivers/firmware/efi/vars.c
index 1d80c1c..f34d8fe 100644
--- a/drivers/firmware/efi/vars.c
+++ b/drivers/firmware/efi/vars.c
@@ -622,10 +622,12 @@ int efivar_entry_set_safe(efi_char16_t *name, efi_guid_t vendor, u32 attributes,
 	if (!ops->query_variable_store)
 		return -ENOSYS;
 
-	if (!block && spin_trylock_irqsave(&__efivars->lock, flags))
-		return -EBUSY;
-	else
+	if (!block) {
+		if (!spin_trylock_irqsave(&__efivars->lock, flags))
+			return -EBUSY;
+	} else {
 		spin_lock_irqsave(&__efivars->lock, flags);
+	}
 
 	status = check_var_size(attributes, size + ucs2_strsize(name, 1024));
 	if (status != EFI_SUCCESS) {

^ permalink raw reply related	[flat|nested] 4+ messages in thread

* [patch] efi: locking fix in efivar_entry_set_safe()
@ 2013-04-30  7:43 ` Dan Carpenter
  0 siblings, 0 replies; 4+ messages in thread
From: Dan Carpenter @ 2013-04-30  7:43 UTC (permalink / raw)
  To: Matt Fleming, Tom Gundersen
  Cc: linux-efi-u79uwXL29TY76Z2rM5mHXA, kernel-janitors-u79uwXL29TY76Z2rM5mHXA

The intent is that if we aren't allowed to block because we're in an
NMI or an emergency then we only take the lock if it is uncontended.

Part of the problem is the test is reversed so we return -EBUSY if we
acquire the lock.

Signed-off-by: Dan Carpenter <dan.carpenter-QHcLZuEGTsvQT0dZR+AlfA@public.gmane.org>
---
Static checker stuff.  I haven't tested this.

diff --git a/drivers/firmware/efi/vars.c b/drivers/firmware/efi/vars.c
index 1d80c1c..f34d8fe 100644
--- a/drivers/firmware/efi/vars.c
+++ b/drivers/firmware/efi/vars.c
@@ -622,10 +622,12 @@ int efivar_entry_set_safe(efi_char16_t *name, efi_guid_t vendor, u32 attributes,
 	if (!ops->query_variable_store)
 		return -ENOSYS;
 
-	if (!block && spin_trylock_irqsave(&__efivars->lock, flags))
-		return -EBUSY;
-	else
+	if (!block) {
+		if (!spin_trylock_irqsave(&__efivars->lock, flags))
+			return -EBUSY;
+	} else {
 		spin_lock_irqsave(&__efivars->lock, flags);
+	}
 
 	status = check_var_size(attributes, size + ucs2_strsize(name, 1024));
 	if (status != EFI_SUCCESS) {

^ permalink raw reply related	[flat|nested] 4+ messages in thread

* Re: [patch] efi: locking fix in efivar_entry_set_safe()
       [not found] ` <20130430074312.GD7237-mgFCXtclrQlZLf2FXnZxJA@public.gmane.org>
@ 2013-04-30 11:33     ` Matt Fleming
  0 siblings, 0 replies; 4+ messages in thread
From: Matt Fleming @ 2013-04-30 11:33 UTC (permalink / raw)
  To: Dan Carpenter
  Cc: Tom Gundersen, linux-efi-u79uwXL29TY76Z2rM5mHXA,
	kernel-janitors-u79uwXL29TY76Z2rM5mHXA, Seiji Aguchi

On 30/04/13 08:43, Dan Carpenter wrote:
> The intent is that if we aren't allowed to block because we're in an
> NMI or an emergency then we only take the lock if it is uncontended.
> 
> Part of the problem is the test is reversed so we return -EBUSY if we
> acquire the lock.
> 
> Signed-off-by: Dan Carpenter <dan.carpenter@oracle.com>
> ---
> Static checker stuff.  I haven't tested this.

This looks correct to me. Thanks!

> diff --git a/drivers/firmware/efi/vars.c b/drivers/firmware/efi/vars.c
> index 1d80c1c..f34d8fe 100644
> --- a/drivers/firmware/efi/vars.c
> +++ b/drivers/firmware/efi/vars.c
> @@ -622,10 +622,12 @@ int efivar_entry_set_safe(efi_char16_t *name, efi_guid_t vendor, u32 attributes,
>  	if (!ops->query_variable_store)
>  		return -ENOSYS;
>  
> -	if (!block && spin_trylock_irqsave(&__efivars->lock, flags))
> -		return -EBUSY;
> -	else
> +	if (!block) {
> +		if (!spin_trylock_irqsave(&__efivars->lock, flags))
> +			return -EBUSY;
> +	} else {
>  		spin_lock_irqsave(&__efivars->lock, flags);
> +	}
>  
>  	status = check_var_size(attributes, size + ucs2_strsize(name, 1024));
>  	if (status != EFI_SUCCESS) {


-- 
Matt Fleming, Intel Open Source Technology Center

^ permalink raw reply	[flat|nested] 4+ messages in thread

* Re: [patch] efi: locking fix in efivar_entry_set_safe()
@ 2013-04-30 11:33     ` Matt Fleming
  0 siblings, 0 replies; 4+ messages in thread
From: Matt Fleming @ 2013-04-30 11:33 UTC (permalink / raw)
  To: Dan Carpenter
  Cc: Tom Gundersen, linux-efi-u79uwXL29TY76Z2rM5mHXA,
	kernel-janitors-u79uwXL29TY76Z2rM5mHXA, Seiji Aguchi

On 30/04/13 08:43, Dan Carpenter wrote:
> The intent is that if we aren't allowed to block because we're in an
> NMI or an emergency then we only take the lock if it is uncontended.
> 
> Part of the problem is the test is reversed so we return -EBUSY if we
> acquire the lock.
> 
> Signed-off-by: Dan Carpenter <dan.carpenter-QHcLZuEGTsvQT0dZR+AlfA@public.gmane.org>
> ---
> Static checker stuff.  I haven't tested this.

This looks correct to me. Thanks!

> diff --git a/drivers/firmware/efi/vars.c b/drivers/firmware/efi/vars.c
> index 1d80c1c..f34d8fe 100644
> --- a/drivers/firmware/efi/vars.c
> +++ b/drivers/firmware/efi/vars.c
> @@ -622,10 +622,12 @@ int efivar_entry_set_safe(efi_char16_t *name, efi_guid_t vendor, u32 attributes,
>  	if (!ops->query_variable_store)
>  		return -ENOSYS;
>  
> -	if (!block && spin_trylock_irqsave(&__efivars->lock, flags))
> -		return -EBUSY;
> -	else
> +	if (!block) {
> +		if (!spin_trylock_irqsave(&__efivars->lock, flags))
> +			return -EBUSY;
> +	} else {
>  		spin_lock_irqsave(&__efivars->lock, flags);
> +	}
>  
>  	status = check_var_size(attributes, size + ucs2_strsize(name, 1024));
>  	if (status != EFI_SUCCESS) {


-- 
Matt Fleming, Intel Open Source Technology Center

^ permalink raw reply	[flat|nested] 4+ messages in thread

end of thread, other threads:[~2013-04-30 11:33 UTC | newest]

Thread overview: 4+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2013-04-30  7:43 [patch] efi: locking fix in efivar_entry_set_safe() Dan Carpenter
2013-04-30  7:43 ` Dan Carpenter
     [not found] ` <20130430074312.GD7237-mgFCXtclrQlZLf2FXnZxJA@public.gmane.org>
2013-04-30 11:33   ` Matt Fleming
2013-04-30 11:33     ` Matt Fleming

This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.