[patch] unlock_kernel() on error path in sx_fw_ioctl()

[patch] unlock_kernel() on error path in sx_fw_ioctl()

[Dan Carpenter]

If we return directly with -EPERM then lock_kernel() is still held.

This was found with a code checker (http://repo.or.cz/w/smatch.git/). 
Compile tested only, sorry.

regards,
dan carpenter

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

--- orig/drivers/char/sx.c	2009-02-03 08:27:59.000000000 +0300
+++ devel/drivers/char/sx.c	2009-02-03 08:28:31.000000000 +0300
@@ -1747,7 +1747,8 @@
  		break;
  	case SXIO_DO_RAMTEST:
  		if (sx_initialized)	/* Already initialized: better not ramtest the board.  */
-			return -EPERM;
+			rc = -EPERM;
+			break;
  		if (IS_SX_BOARD(board)) {
  			rc = do_memtest(board, 0, 0x7000);
  			if (!rc)

Re: [patch] unlock_kernel() on error path in sx_fw_ioctl()

[Andrew Morton]

On Tue, 3 Feb 2009 11:14:21 +0300 (EAT) Dan Carpenter <error27@gmail.com> wrote:

> If we return directly with -EPERM then lock_kernel() is still held.
> 
> This was found with a code checker (http://repo.or.cz/w/smatch.git/). 
> Compile tested only, sorry.
> 
> regards,
> dan carpenter
> 
> Signed-off-by: Dan Carpenter <error27@gmail.com>
> 
> --- orig/drivers/char/sx.c	2009-02-03 08:27:59.000000000 +0300
> +++ devel/drivers/char/sx.c	2009-02-03 08:28:31.000000000 +0300
> @@ -1747,7 +1747,8 @@
>   		break;
>   	case SXIO_DO_RAMTEST:
>   		if (sx_initialized)	/* Already initialized: better not ramtest the board.  */
> -			return -EPERM;
> +			rc = -EPERM;
> +			break;
>   		if (IS_SX_BOARD(board)) {
>   			rc = do_memtest(board, 0, 0x7000);
>   			if (!rc)

Yawn.  return-deep-inside-a-large-function strikes again.

We might as well fix the other one while we're there:

From: Andrew Morton <akpm@linux-foundation.org>

fix another such path - missed func_exit().

Cc: Dan Carpenter <error27@gmail.com>
Signed-off-by: Andrew Morton <akpm@linux-foundation.org>
---

 drivers/char/sx.c |    5 +++--
 1 file changed, 3 insertions(+), 2 deletions(-)

diff -puN drivers/char/sx.c~sxc-fix-missed-unlock_kernel-on-error-path-in-sx_fw_ioctl-fix drivers/char/sx.c
--- a/drivers/char/sx.c~sxc-fix-missed-unlock_kernel-on-error-path-in-sx_fw_ioctl-fix
+++ a/drivers/char/sx.c
@@ -1713,8 +1713,8 @@ static long sx_fw_ioctl(struct file *fil
 		for (i = 0; i < SX_NBOARDS; i++)
 			sx_dprintk(SX_DEBUG_FIRMWARE, "<%x> ", boards[i].flags);
 		sx_dprintk(SX_DEBUG_FIRMWARE, "\n");
-		unlock_kernel();
-		return -EIO;
+		rc = -EIO;
+		goto out;
 	}
 
 	switch (cmd) {
@@ -1845,6 +1845,7 @@ static long sx_fw_ioctl(struct file *fil
 		rc = -ENOTTY;
 		break;
 	}
+out:
 	unlock_kernel();
 	func_exit();
 	return rc;
_

Re: [patch] unlock_kernel() on error path in sx_fw_ioctl()

[Rogier Wolff]

On Tue, Feb 03, 2009 at 08:58:57PM -0800, Andrew Morton wrote:
> fix another such path - missed func_exit().

This is not critical. It's just debug. When DEBUG_CALLS (or something
like that) is active, it will print "leaving function ... ". 

Indeed if it's missing, you might start to think why DID it return,
but not print the message... or. howcome it's still in that function
doing stuff that isn't there. Anyway, looking into it would provide
you more info that it was returning the error.....

Anyway, nice to clean it up "while we're at it". 

	Roger. 


-- 
** R.E.Wolff@BitWizard.nl ** http://www.BitWizard.nl/ ** +31-15-2600998 **
**    Delftechpark 26 2628 XH  Delft, The Netherlands. KVK: 27239233    **
*-- BitWizard writes Linux device drivers for any device you may have! --*
Q: It doesn't work. A: Look buddy, doesn't work is an ambiguous statement. 
Does it sit on the couch all day? Is it unemployed? Please be specific! 
Define 'it' and what it isn't doing. --------- Adapted from lxrbot FAQ