inconsistent null checking in ipx_ioctl()

inconsistent null checking in ipx_ioctl()

[Dan Carpenter]

Hi, I'm working on some new Smatch stuff and going through some warnings
in old code.

----
This is a semi-automatic email about new static checker warnings.

The patch b0d0d915d1d1: "ipx: remove the BKL" from Jan 25, 2011, 
leads to the following Smatch complaint:

net/ipx/af_ipx.c:1928 ipx_ioctl()
	 error: we previously assumed 'sk' could be null (see line 1913)

net/ipx/af_ipx.c
  1912			rc = -EINVAL;
  1913			if (sk)
                           ^^^^
Check.

  1914				rc = sock_get_timestamp(sk, argp);
  1915			break;
  1916		case SIOCGIFDSTADDR:
  1917		case SIOCSIFDSTADDR:
  1918		case SIOCGIFBRDADDR:
  1919		case SIOCSIFBRDADDR:
  1920		case SIOCGIFNETMASK:
  1921		case SIOCSIFNETMASK:
  1922			rc = -EINVAL;
  1923			break;
  1924		default:
  1925			rc = -ENOIOCTLCMD;
  1926			break;
  1927		}
  1928		release_sock(sk);
                ^^^^^^^^^^^^^^^^^
The lock and release functions dereference "sk".  Probably the check
can be removed.  The rest of the function dereferences "sk" without
checking.  A lot of this code goes back to 2.6.12.

  1929	
  1930		return rc;

regards,
dan carpenter

Re: inconsistent null checking in ipx_ioctl()

[David Miller]

From: Dan Carpenter <dan.carpenter@oracle.com>
Date: Mon, 14 May 2012 23:56:18 +0300

> Hi, I'm working on some new Smatch stuff and going through some warnings
> in old code.
> 
> ----
> This is a semi-automatic email about new static checker warnings.
> 
> The patch b0d0d915d1d1: "ipx: remove the BKL" from Jan 25, 2011, 
> leads to the following Smatch complaint:
> 
> net/ipx/af_ipx.c:1928 ipx_ioctl()
> 	 error: we previously assumed 'sk' could be null (see line 1913)

Thanks Dan, I've commited the following:

--------------------
ipx: Remove spurious NULL checking in ipx_ioctl().

We already unconditionally dereference 'sk' via lock_sock(sk) earlier
in this function, and our caller (sock_do_ioctl()) makes takes similar
liberties.

Reported-by: Dan Carpenter <dan.carpenter@oracle.com>
Signed-off-by: David S. Miller <davem@davemloft.net>
---
 net/ipx/af_ipx.c |    4 +---
 1 file changed, 1 insertion(+), 3 deletions(-)

diff --git a/net/ipx/af_ipx.c b/net/ipx/af_ipx.c
index 824d4a3..dfd6faa 100644
--- a/net/ipx/af_ipx.c
+++ b/net/ipx/af_ipx.c
@@ -1901,9 +1901,7 @@ static int ipx_ioctl(struct socket *sock, unsigned int cmd, unsigned long arg)
 			      (const unsigned short __user *)argp);
 		break;
 	case SIOCGSTAMP:
-		rc = -EINVAL;
-		if (sk)
-			rc = sock_get_timestamp(sk, argp);
+		rc = sock_get_timestamp(sk, argp);
 		break;
 	case SIOCGIFDSTADDR:
 	case SIOCSIFDSTADDR:
-- 
1.7.10.1