public inbox for kernel-janitors@vger.kernel.org
 help / color / mirror / Atom feed
* [patch] efi: locking fix in efivar_entry_set_safe()
@ 2013-04-30  7:43 Dan Carpenter
       [not found] ` <20130430074312.GD7237-mgFCXtclrQlZLf2FXnZxJA@public.gmane.org>
  0 siblings, 1 reply; 2+ 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] 2+ 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; 2+ 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] 2+ messages in thread

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

Thread overview: 2+ 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
     [not found] ` <20130430074312.GD7237-mgFCXtclrQlZLf2FXnZxJA@public.gmane.org>
2013-04-30 11:33   ` Matt Fleming

This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox