public inbox for linux-kernel@vger.kernel.org
 help / color / mirror / Atom feed
* double unlock in rng_dev_read()
@ 2009-12-23 13:15 Dan Carpenter
  2009-12-23 14:36 ` Herbert Xu
  0 siblings, 1 reply; 5+ messages in thread
From: Dan Carpenter @ 2009-12-23 13:15 UTC (permalink / raw)
  To: Matt Mackall; +Cc: Herbert Xu, linux-kernel

It seems like we unlock rng_mutex twice (2.6.33-rc1).

drivers/char/hw_random/core.c
   151                  mutex_unlock(&rng_mutex);
   152
   153                  if (need_resched())
   154                          schedule_timeout_interruptible(1);
   155
   156                  if (signal_pending(current)) {
   157                          err = -ERESTARTSYS;
   158                          goto out;
   159                  }
   160          }
   161  out_unlock:
   162          mutex_unlock(&rng_mutex);
 
regards,
dan carpenter

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

* Re: double unlock in rng_dev_read()
  2009-12-23 13:15 double unlock in rng_dev_read() Dan Carpenter
@ 2009-12-23 14:36 ` Herbert Xu
  2009-12-23 14:53   ` Dan Carpenter
  0 siblings, 1 reply; 5+ messages in thread
From: Herbert Xu @ 2009-12-23 14:36 UTC (permalink / raw)
  To: Dan Carpenter, Matt Mackall, linux-kernel

On Wed, Dec 23, 2009 at 03:15:52PM +0200, Dan Carpenter wrote:
> It seems like we unlock rng_mutex twice (2.6.33-rc1).
> 
> drivers/char/hw_random/core.c
>    151                  mutex_unlock(&rng_mutex);
>    152
>    153                  if (need_resched())
>    154                          schedule_timeout_interruptible(1);
>    155
>    156                  if (signal_pending(current)) {
>    157                          err = -ERESTARTSYS;
>    158                          goto out;
>    159                  }
>    160          }
>    161  out_unlock:
>    162          mutex_unlock(&rng_mutex);

Hmm, are you sure you didn't mistake out for out_unlock? :)
-- 
Visit Openswan at http://www.openswan.org/
Email: Herbert Xu ~{PmV>HI~} <herbert@gondor.apana.org.au>
Home Page: http://gondor.apana.org.au/~herbert/
PGP Key: http://gondor.apana.org.au/~herbert/pubkey.txt

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

* Re: double unlock in rng_dev_read()
  2009-12-23 14:36 ` Herbert Xu
@ 2009-12-23 14:53   ` Dan Carpenter
  2009-12-23 15:23     ` Herbert Xu
  0 siblings, 1 reply; 5+ messages in thread
From: Dan Carpenter @ 2009-12-23 14:53 UTC (permalink / raw)
  To: Herbert Xu; +Cc: Matt Mackall, linux-kernel

On Wed, Dec 23, 2009 at 10:36:58PM +0800, Herbert Xu wrote:
> On Wed, Dec 23, 2009 at 03:15:52PM +0200, Dan Carpenter wrote:
> > It seems like we unlock rng_mutex twice (2.6.33-rc1).
> > 
> > drivers/char/hw_random/core.c
> >    151                  mutex_unlock(&rng_mutex);
> >    152
> >    153                  if (need_resched())
> >    154                          schedule_timeout_interruptible(1);
> >    155
> >    156                  if (signal_pending(current)) {
> >    157                          err = -ERESTARTSYS;
> >    158                          goto out;
> >    159                  }
> >    160          }
> >    161  out_unlock:
> >    162          mutex_unlock(&rng_mutex);
> 
> Hmm, are you sure you didn't mistake out for out_unlock? :)

No no.  I mean when size hits zero we are rng_mutex is unlocked.

regards,
dan carpenter

> -- 
> Visit Openswan at http://www.openswan.org/
> Email: Herbert Xu ~{PmV>HI~} <herbert@gondor.apana.org.au>
> Home Page: http://gondor.apana.org.au/~herbert/
> PGP Key: http://gondor.apana.org.au/~herbert/pubkey.txt

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

* Re: double unlock in rng_dev_read()
  2009-12-23 14:53   ` Dan Carpenter
@ 2009-12-23 15:23     ` Herbert Xu
  2009-12-24 10:51       ` Dan Carpenter
  0 siblings, 1 reply; 5+ messages in thread
From: Herbert Xu @ 2009-12-23 15:23 UTC (permalink / raw)
  To: Dan Carpenter, Matt Mackall, linux-kernel

On Wed, Dec 23, 2009 at 04:53:36PM +0200, Dan Carpenter wrote:
>
> No no.  I mean when size hits zero we are rng_mutex is unlocked.

Good catch! I'll add this patch to the tree.  Please take a look
at it.  Thanks!

commit f5908267b67917b8cbd98b27fd2be9b5f62ec76f
Author: Herbert Xu <herbert@gondor.apana.org.au>
Date:   Wed Dec 23 23:22:34 2009 +0800

    hwrng: core - Fix double unlock in rng_dev_read
    
    When the loop terminates with size == 0 in rng_dev_read we will
    unlock the rng mutex twice.
    
    Reported-by: Dan Carpenter <error27@gmail.com>
    Signed-off-by: Herbert Xu <herbert@gondor.apana.org.au>

diff --git a/drivers/char/hw_random/core.c b/drivers/char/hw_random/core.c
index e989f67..3d9c61e 100644
--- a/drivers/char/hw_random/core.c
+++ b/drivers/char/hw_random/core.c
@@ -158,10 +158,11 @@ static ssize_t rng_dev_read(struct file *filp, char __user *buf,
 			goto out;
 		}
 	}
-out_unlock:
-	mutex_unlock(&rng_mutex);
 out:
 	return ret ? : err;
+out_unlock:
+	mutex_unlock(&rng_mutex);
+	goto out;
 }
 
-- 
Visit Openswan at http://www.openswan.org/
Email: Herbert Xu ~{PmV>HI~} <herbert@gondor.apana.org.au>
Home Page: http://gondor.apana.org.au/~herbert/
PGP Key: http://gondor.apana.org.au/~herbert/pubkey.txt

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

* Re: double unlock in rng_dev_read()
  2009-12-23 15:23     ` Herbert Xu
@ 2009-12-24 10:51       ` Dan Carpenter
  0 siblings, 0 replies; 5+ messages in thread
From: Dan Carpenter @ 2009-12-24 10:51 UTC (permalink / raw)
  To: Herbert Xu; +Cc: Matt Mackall, linux-kernel

On Wed, Dec 23, 2009 at 11:23:55PM +0800, Herbert Xu wrote:
> On Wed, Dec 23, 2009 at 04:53:36PM +0200, Dan Carpenter wrote:
> >
> > No no.  I mean when size hits zero we are rng_mutex is unlocked.
> 
> Good catch! I'll add this patch to the tree.  Please take a look
> at it.  Thanks!
> 

Great.

Acked-by: Dan Carpenter <error27@gmail.com>

regards,
dan carpenter


> commit f5908267b67917b8cbd98b27fd2be9b5f62ec76f
> Author: Herbert Xu <herbert@gondor.apana.org.au>
> Date:   Wed Dec 23 23:22:34 2009 +0800
> 
>     hwrng: core - Fix double unlock in rng_dev_read
>     
>     When the loop terminates with size == 0 in rng_dev_read we will
>     unlock the rng mutex twice.
>     
>     Reported-by: Dan Carpenter <error27@gmail.com>
>     Signed-off-by: Herbert Xu <herbert@gondor.apana.org.au>
> 
> diff --git a/drivers/char/hw_random/core.c b/drivers/char/hw_random/core.c
> index e989f67..3d9c61e 100644
> --- a/drivers/char/hw_random/core.c
> +++ b/drivers/char/hw_random/core.c
> @@ -158,10 +158,11 @@ static ssize_t rng_dev_read(struct file *filp, char __user *buf,
>  			goto out;
>  		}
>  	}
> -out_unlock:
> -	mutex_unlock(&rng_mutex);
>  out:
>  	return ret ? : err;
> +out_unlock:
> +	mutex_unlock(&rng_mutex);
> +	goto out;
>  }
>  
> -- 
> Visit Openswan at http://www.openswan.org/
> Email: Herbert Xu ~{PmV>HI~} <herbert@gondor.apana.org.au>
> Home Page: http://gondor.apana.org.au/~herbert/
> PGP Key: http://gondor.apana.org.au/~herbert/pubkey.txt

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

end of thread, other threads:[~2009-12-24 10:59 UTC | newest]

Thread overview: 5+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
2009-12-23 13:15 double unlock in rng_dev_read() Dan Carpenter
2009-12-23 14:36 ` Herbert Xu
2009-12-23 14:53   ` Dan Carpenter
2009-12-23 15:23     ` Herbert Xu
2009-12-24 10:51       ` Dan Carpenter

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