[PATCH wpan-tools 0/4] Coverity fixes for wpan-ping

[PATCH wpan-tools 0/4] Coverity fixes for wpan-ping

[Stefan Schmidt]

Hello.

I finished setting up Travis CI for builds and Coverity Scan service for wpan-tools on a private
GitHub fork. If nobody has a strong opinion against this I would like to set it up on the main repo
so we can have a first step towards some automated QA infrastructure.

But lets first have a look at the results from this first run. As you can see on the our first
run revealed 4 defects in 23321 lines of code (after the C preprocessor). With a defect density
of 0.17 this is way below industry average which is set to one here. In other words this means
we only have 1.7 defects per 10000 lines of code.

Three of these have been problems in error pathes of the code (leak, no check on return value, etc)
and one have been a false positive in the main iwpan.c file. Coverity was not able to understand
that we set err in our netlinks callbacks and thought this code bit would never stop as we start
with err = 1

while (err > 0)
	nl_recvmsgs(state->nl_sock, cb);

I marked it as false positive and fixed the remaining three defects with the patches below.

Stefan Schmidt (4):
  wpan-ping: remove unused struct sockaddr_nl variable
  wpan-ping: check return value for setsockopt
  wpan-ping: do not try to send data back to origin if we got an error
    from recvfrom
  wpan-ping: avoid leaking socket handle on error path

 wpan-ping/wpan-ping.c | 9 +++++++--
 1 file changed, 7 insertions(+), 2 deletions(-)

regards
Stefan Schmidt
-- 
2.4.3

[PATCH wpan-tools 1/4] wpan-ping: remove unused struct sockaddr_nl variable

[Stefan Schmidt]

Signed-off-by: Stefan Schmidt <stefan@osg.samsung.com>
---
 wpan-ping/wpan-ping.c | 1 -
 1 file changed, 1 deletion(-)

diff --git a/wpan-ping/wpan-ping.c b/wpan-ping/wpan-ping.c
index 5429ca3..8f781c1 100644
--- a/wpan-ping/wpan-ping.c
+++ b/wpan-ping/wpan-ping.c
@@ -151,7 +151,6 @@ static void nl802154_cleanup(struct config *conf)
 static int nl_msg_cb(struct nl_msg* msg, void* arg)
 {
 	struct config *conf = arg;
-	struct sockaddr_nl nla;
 	struct nlmsghdr *nlh = nlmsg_hdr(msg);
 	struct nlattr *attrs[NL802154_ATTR_MAX+1];
 	uint64_t temp;
-- 
2.4.3

[PATCH wpan-tools 2/4] wpan-ping: check return value for setsockopt

[Stefan Schmidt]

In case we fail to set the receive timeout print the error.

CID: 32113
Signed-off-by: Stefan Schmidt <stefan@osg.samsung.com>
---
 wpan-ping/wpan-ping.c | 5 ++++-
 1 file changed, 4 insertions(+), 1 deletion(-)

diff --git a/wpan-ping/wpan-ping.c b/wpan-ping/wpan-ping.c
index 8f781c1..8f4c13c 100644
--- a/wpan-ping/wpan-ping.c
+++ b/wpan-ping/wpan-ping.c
@@ -254,7 +254,10 @@ static int measure_roundtrip(struct config *conf, int sd) {
 	/* 500ms seconds packet receive timeout */
 	timeout.tv_sec = 0;
 	timeout.tv_usec = 500000;
-	setsockopt(sd, SOL_SOCKET, SO_RCVTIMEO, (struct timeval *)&timeout,sizeof(struct timeval));
+	ret = setsockopt(sd, SOL_SOCKET, SO_RCVTIMEO, (struct timeval *)&timeout,sizeof(struct timeval));
+	if (ret < 0) {
+		perror("setsockopt receive timeout");
+	}
 
 	count = 0;
 	for (i = 0; i < conf->packets; i++) {
-- 
2.4.3

[PATCH wpan-tools 3/4] wpan-ping: do not try to send data back to origin if we got an error from recvfrom

[Stefan Schmidt]

In the case of a length of -1 from recvfrom we print out the error code but we
never left the current loop iteration. That might be problematic when we try to
send the data back with sendto. Instead we now continue with the next iteration
from now on if we get an error.

CID: 32115
Signed-off-by: Stefan Schmidt <stefan@osg.samsung.com>
---
 wpan-ping/wpan-ping.c | 2 ++
 1 file changed, 2 insertions(+)

diff --git a/wpan-ping/wpan-ping.c b/wpan-ping/wpan-ping.c
index 8f4c13c..616e0dd 100644
--- a/wpan-ping/wpan-ping.c
+++ b/wpan-ping/wpan-ping.c
@@ -345,12 +345,14 @@ static void init_server(struct config *conf, int sd) {
 		len = recvfrom(sd, buf, MAX_PAYLOAD_LEN, 0, (struct sockaddr *)&src, &addrlen);
 		if (len < 0) {
 			perror("recvfrom");
+			continue;
 		}
 		//dump_packet(buf, len);
 		/* Send same packet back */
 		len = sendto(sd, buf, len, 0, (struct sockaddr *)&src, addrlen);
 		if (len < 0) {
 			perror("sendto");
+			continue;
 		}
 	}
 	free(buf);
-- 
2.4.3

[PATCH wpan-tools 4/4] wpan-ping: avoid leaking socket handle on error path

[Stefan Schmidt]

If the bind failed we would return without closing the socket. Better do
so now.

CID: 32116
Signed-off-by: Stefan Schmidt <stefan@osg.samsung.com>
---
 wpan-ping/wpan-ping.c | 1 +
 1 file changed, 1 insertion(+)

diff --git a/wpan-ping/wpan-ping.c b/wpan-ping/wpan-ping.c
index 616e0dd..b454ddb 100644
--- a/wpan-ping/wpan-ping.c
+++ b/wpan-ping/wpan-ping.c
@@ -372,6 +372,7 @@ static int init_network(struct config *conf) {
 	ret = bind(sd, (struct sockaddr *)&conf->src, sizeof(conf->src));
 	if (ret) {
 		perror("bind");
+		close(sd);
 		return 1;
 	}
 
-- 
2.4.3

Re: [PATCH wpan-tools 0/4] Coverity fixes for wpan-ping

[Stefan Schmidt]

Hello.

On 10/11/15 22:42, Stefan Schmidt wrote:
> Hello.
>
> I finished setting up Travis CI for builds and Coverity Scan service for wpan-tools on a private
> GitHub fork. If nobody has a strong opinion against this I would like to set it up on the main repo
> so we can have a first step towards some automated QA infrastructure.

This is done now. Will send a separate mail with more details about it.

> But lets first have a look at the results from this first run. As you can see on the our first
> run revealed 4 defects in 23321 lines of code (after the C preprocessor). With a defect density
> of 0.17 this is way below industry average which is set to one here. In other words this means
> we only have 1.7 defects per 10000 lines of code.
>
> Three of these have been problems in error pathes of the code (leak, no check on return value, etc)
> and one have been a false positive in the main iwpan.c file. Coverity was not able to understand
> that we set err in our netlinks callbacks and thought this code bit would never stop as we start
> with err = 1
>
> while (err > 0)
> 	nl_recvmsgs(state->nl_sock, cb);
>
> I marked it as false positive and fixed the remaining three defects with the patches below.
>
> Stefan Schmidt (4):
>    wpan-ping: remove unused struct sockaddr_nl variable
>    wpan-ping: check return value for setsockopt
>    wpan-ping: do not try to send data back to origin if we got an error
>      from recvfrom
>    wpan-ping: avoid leaking socket handle on error path
>
>   wpan-ping/wpan-ping.c | 9 +++++++--
>   1 file changed, 7 insertions(+), 2 deletions(-)
>

Pushed these now.

regards
Stefan Schmidt