* inconsistent null checking in ipx_ioctl()
@ 2012-05-14 20:56 Dan Carpenter
2012-05-19 4:51 ` David Miller
0 siblings, 1 reply; 2+ messages in thread
From: Dan Carpenter @ 2012-05-14 20:56 UTC (permalink / raw)
To: netdev
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
^ permalink raw reply [flat|nested] 2+ messages in thread
* Re: inconsistent null checking in ipx_ioctl()
2012-05-14 20:56 inconsistent null checking in ipx_ioctl() Dan Carpenter
@ 2012-05-19 4:51 ` David Miller
0 siblings, 0 replies; 2+ messages in thread
From: David Miller @ 2012-05-19 4:51 UTC (permalink / raw)
To: dan.carpenter; +Cc: netdev
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
^ permalink raw reply related [flat|nested] 2+ messages in thread
end of thread, other threads:[~2012-05-19 4:51 UTC | newest]
Thread overview: 2+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2012-05-14 20:56 inconsistent null checking in ipx_ioctl() Dan Carpenter
2012-05-19 4:51 ` David Miller
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).