mirror of https://lore.kernel.org/lkml/
 help / color / mirror / Atom feed
* [PATCH 1/1] PCMCIA: resource, fix lock imbalance
@ 2010-03-16 14:57 Jiri Slaby
  2010-03-16 16:57 ` Dominik Brodowski
  0 siblings, 1 reply; 2+ messages in thread
From: Jiri Slaby @ 2010-03-16 14:57 UTC (permalink / raw)
  To: linux; +Cc: linux-pcmcia, linux-kernel, jirislaby

Stanse found that one error path (when alloc_skb fails) in netdev_tx
omits to unlock hw_priv->hwlock. Fix that by moving away from unlock in
each fail path. Unlock at one place instead.

Introduced in 94a819f80297e1f635a7cde4ed5317612e512ba7
(pcmcia: assert locking to struct pcmcia_device)

Signed-off-by: Jiri Slaby <jslaby@suse.cz>
Cc: Dominik Brodowski <linux@dominikbrodowski.net>
---
 drivers/pcmcia/pcmcia_resource.c |   28 ++++++++++++++++------------
 1 files changed, 16 insertions(+), 12 deletions(-)

diff --git a/drivers/pcmcia/pcmcia_resource.c b/drivers/pcmcia/pcmcia_resource.c
index b2df041..add962a 100644
--- a/drivers/pcmcia/pcmcia_resource.c
+++ b/drivers/pcmcia/pcmcia_resource.c
@@ -256,6 +256,7 @@ int pcmcia_modify_configuration(struct pcmcia_device *p_dev,
 {
 	struct pcmcia_socket *s;
 	config_t *c;
+	int ret;
 
 	s = p_dev->socket;
 
@@ -264,13 +265,13 @@ int pcmcia_modify_configuration(struct pcmcia_device *p_dev,
 
 	if (!(s->state & SOCKET_PRESENT)) {
 		dev_dbg(&s->dev, "No card present\n");
-		mutex_unlock(&s->ops_mutex);
-		return -ENODEV;
+		ret = -ENODEV;
+		goto unlock;
 	}
 	if (!(c->state & CONFIG_LOCKED)) {
 		dev_dbg(&s->dev, "Configuration isnt't locked\n");
-		mutex_unlock(&s->ops_mutex);
-		return -EACCES;
+		ret = -EACCES;
+		goto unlock;
 	}
 
 	if (mod->Attributes & CONF_IRQ_CHANGE_VALID) {
@@ -286,7 +287,8 @@ int pcmcia_modify_configuration(struct pcmcia_device *p_dev,
 
 	if (mod->Attributes & CONF_VCC_CHANGE_VALID) {
 		dev_dbg(&s->dev, "changing Vcc is not allowed at this time\n");
-		return -EINVAL;
+		ret = -EINVAL;
+		goto unlock;
 	}
 
 	/* We only allow changing Vpp1 and Vpp2 to the same value */
@@ -294,21 +296,21 @@ int pcmcia_modify_configuration(struct pcmcia_device *p_dev,
 	    (mod->Attributes & CONF_VPP2_CHANGE_VALID)) {
 		if (mod->Vpp1 != mod->Vpp2) {
 			dev_dbg(&s->dev, "Vpp1 and Vpp2 must be the same\n");
-			mutex_unlock(&s->ops_mutex);
-			return -EINVAL;
+			ret = -EINVAL;
+			goto unlock;
 		}
 		s->socket.Vpp = mod->Vpp1;
 		if (s->ops->set_socket(s, &s->socket)) {
-			mutex_unlock(&s->ops_mutex);
 			dev_printk(KERN_WARNING, &s->dev,
 				   "Unable to set VPP\n");
-			return -EIO;
+			ret = -EIO;
+			goto unlock;
 		}
 	} else if ((mod->Attributes & CONF_VPP1_CHANGE_VALID) ||
 		   (mod->Attributes & CONF_VPP2_CHANGE_VALID)) {
 		dev_dbg(&s->dev, "changing Vcc is not allowed at this time\n");
-		mutex_unlock(&s->ops_mutex);
-		return -EINVAL;
+		ret = -EINVAL;
+		goto unlock;
 	}
 
 	if (mod->Attributes & CONF_IO_CHANGE_WIDTH) {
@@ -332,9 +334,11 @@ int pcmcia_modify_configuration(struct pcmcia_device *p_dev,
 			s->ops->set_io_map(s, &io_on);
 		}
 	}
+	ret = 0;
+unlock:
 	mutex_unlock(&s->ops_mutex);
 
-	return 0;
+	return ret;
 } /* modify_configuration */
 EXPORT_SYMBOL(pcmcia_modify_configuration);
 
-- 
1.7.0.1



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

* Re: [PATCH 1/1] PCMCIA: resource, fix lock imbalance
  2010-03-16 14:57 [PATCH 1/1] PCMCIA: resource, fix lock imbalance Jiri Slaby
@ 2010-03-16 16:57 ` Dominik Brodowski
  0 siblings, 0 replies; 2+ messages in thread
From: Dominik Brodowski @ 2010-03-16 16:57 UTC (permalink / raw)
  To: Jiri Slaby; +Cc: linux-pcmcia, linux-kernel, jirislaby

ACK, applied -- even though this code path is actually unused in current
kernels.

Thanks,
	Dominik

On Tue, Mar 16, 2010 at 03:57:44PM +0100, Jiri Slaby wrote:
> Stanse found that one error path (when alloc_skb fails) in netdev_tx
> omits to unlock hw_priv->hwlock. Fix that by moving away from unlock in
> each fail path. Unlock at one place instead.
> 
> Introduced in 94a819f80297e1f635a7cde4ed5317612e512ba7
> (pcmcia: assert locking to struct pcmcia_device)
> 
> Signed-off-by: Jiri Slaby <jslaby@suse.cz>
> Cc: Dominik Brodowski <linux@dominikbrodowski.net>
> ---
>  drivers/pcmcia/pcmcia_resource.c |   28 ++++++++++++++++------------
>  1 files changed, 16 insertions(+), 12 deletions(-)
> 
> diff --git a/drivers/pcmcia/pcmcia_resource.c b/drivers/pcmcia/pcmcia_resource.c
> index b2df041..add962a 100644
> --- a/drivers/pcmcia/pcmcia_resource.c
> +++ b/drivers/pcmcia/pcmcia_resource.c
> @@ -256,6 +256,7 @@ int pcmcia_modify_configuration(struct pcmcia_device *p_dev,
>  {
>  	struct pcmcia_socket *s;
>  	config_t *c;
> +	int ret;
>  
>  	s = p_dev->socket;
>  
> @@ -264,13 +265,13 @@ int pcmcia_modify_configuration(struct pcmcia_device *p_dev,
>  
>  	if (!(s->state & SOCKET_PRESENT)) {
>  		dev_dbg(&s->dev, "No card present\n");
> -		mutex_unlock(&s->ops_mutex);
> -		return -ENODEV;
> +		ret = -ENODEV;
> +		goto unlock;
>  	}
>  	if (!(c->state & CONFIG_LOCKED)) {
>  		dev_dbg(&s->dev, "Configuration isnt't locked\n");
> -		mutex_unlock(&s->ops_mutex);
> -		return -EACCES;
> +		ret = -EACCES;
> +		goto unlock;
>  	}
>  
>  	if (mod->Attributes & CONF_IRQ_CHANGE_VALID) {
> @@ -286,7 +287,8 @@ int pcmcia_modify_configuration(struct pcmcia_device *p_dev,
>  
>  	if (mod->Attributes & CONF_VCC_CHANGE_VALID) {
>  		dev_dbg(&s->dev, "changing Vcc is not allowed at this time\n");
> -		return -EINVAL;
> +		ret = -EINVAL;
> +		goto unlock;
>  	}
>  
>  	/* We only allow changing Vpp1 and Vpp2 to the same value */
> @@ -294,21 +296,21 @@ int pcmcia_modify_configuration(struct pcmcia_device *p_dev,
>  	    (mod->Attributes & CONF_VPP2_CHANGE_VALID)) {
>  		if (mod->Vpp1 != mod->Vpp2) {
>  			dev_dbg(&s->dev, "Vpp1 and Vpp2 must be the same\n");
> -			mutex_unlock(&s->ops_mutex);
> -			return -EINVAL;
> +			ret = -EINVAL;
> +			goto unlock;
>  		}
>  		s->socket.Vpp = mod->Vpp1;
>  		if (s->ops->set_socket(s, &s->socket)) {
> -			mutex_unlock(&s->ops_mutex);
>  			dev_printk(KERN_WARNING, &s->dev,
>  				   "Unable to set VPP\n");
> -			return -EIO;
> +			ret = -EIO;
> +			goto unlock;
>  		}
>  	} else if ((mod->Attributes & CONF_VPP1_CHANGE_VALID) ||
>  		   (mod->Attributes & CONF_VPP2_CHANGE_VALID)) {
>  		dev_dbg(&s->dev, "changing Vcc is not allowed at this time\n");
> -		mutex_unlock(&s->ops_mutex);
> -		return -EINVAL;
> +		ret = -EINVAL;
> +		goto unlock;
>  	}
>  
>  	if (mod->Attributes & CONF_IO_CHANGE_WIDTH) {
> @@ -332,9 +334,11 @@ int pcmcia_modify_configuration(struct pcmcia_device *p_dev,
>  			s->ops->set_io_map(s, &io_on);
>  		}
>  	}
> +	ret = 0;
> +unlock:
>  	mutex_unlock(&s->ops_mutex);
>  
> -	return 0;
> +	return ret;
>  } /* modify_configuration */
>  EXPORT_SYMBOL(pcmcia_modify_configuration);
>  
> -- 
> 1.7.0.1
> 
> 

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

end of thread, other threads:[~2010-03-16 16:58 UTC | newest]

Thread overview: 2+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2010-03-16 14:57 [PATCH 1/1] PCMCIA: resource, fix lock imbalance Jiri Slaby
2010-03-16 16:57 ` Dominik Brodowski

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

all inboxes | Powered by JetHome®