[PATCH 0/6] Some simple Coverity fixes

All of lore.kernel.org
 help / color / mirror / Atom feed

* [PATCH 0/6] Some simple Coverity fixes
@ 2013-07-04 19:55 Mikko Rapeli
  2013-07-04 19:55 ` [PATCH 1/6] trinity.c: fix uninitialized variable Mikko Rapeli
                   ` (5 more replies)
  0 siblings, 6 replies; 11+ messages in thread
From: Mikko Rapeli @ 2013-07-04 19:55 UTC (permalink / raw)
  To: trinity

I registered trinity to Coverity scan and fixed a few simple looking findings.
Hopefully the leaks were not intentional. Some harder looking int64 range and
other bugs which had comments I don't dare to touch but maybe you should have
a quick look at the Coverity output anyway.

Tested these only shortly by running trinity as normal user.

Cheers,

Mikko Rapeli (6):
  trinity.c: fix uninitialized variable
  trinity.c: log errors if socket calls fail
  perf_event_open: initialize chars
  perf_event_open.c: close dir's on exit paths
  sockets.c: don't leak cachefile on return paths
  maps.c: only close() if fd is valid

 maps.c                     |  3 ++-
 sockets.c                  | 10 +++++++---
 syscalls/perf_event_open.c | 12 ++++++++++--
 trinity.c                  | 10 ++++++++--
 4 files changed, 27 insertions(+), 8 deletions(-)

-- 
1.8.3.1

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

* [PATCH 1/6] trinity.c: fix uninitialized variable
  2013-07-04 19:55 [PATCH 0/6] Some simple Coverity fixes Mikko Rapeli
@ 2013-07-04 19:55 ` Mikko Rapeli
  2013-07-05 14:44   ` Dave Jones
  2013-07-04 19:55 ` [PATCH 2/6] " Mikko Rapeli
                   ` (4 subsequent siblings)
  5 siblings, 1 reply; 11+ messages in thread
From: Mikko Rapeli @ 2013-07-04 19:55 UTC (permalink / raw)
  To: trinity

Coverity says:

CID 1042350 (#1 of 1): Uninitialized scalar variable (UNINIT)
23. uninit_use_in_call: Using uninitialized value "ling": field "ling"."l_linger" is uninitialized when calling "setsockopt(int, int, int, void const *, socklen_t)".

Signed-off-by: Mikko Rapeli <mikko.rapeli@iki.fi>
---
 trinity.c | 1 +
 1 file changed, 1 insertion(+)

diff --git a/trinity.c b/trinity.c
index 93e7819..3f80020 100644
--- a/trinity.c
+++ b/trinity.c
@@ -250,6 +250,7 @@ cleanup_fds:
 
 	for (i = 0; i < nr_sockets; i++) {
 		struct linger ling;
+		memset(&ling, 0, sizeof(ling));
 
 		ling.l_onoff = FALSE;	/* linger active */
 		setsockopt(shm->socket_fds[i], SOL_SOCKET, SO_LINGER, &ling, sizeof(struct linger));
-- 
1.8.3.1

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

* Re: [PATCH 1/6] trinity.c: fix uninitialized variable
  2013-07-04 19:55 ` [PATCH 1/6] trinity.c: fix uninitialized variable Mikko Rapeli
@ 2013-07-05 14:44   ` Dave Jones
  2013-07-05 16:15     ` [PATCHv2 1/2] " Mikko Rapeli
  2013-07-05 16:15     ` [PATCHv2 2/2] trinity.c: log errors if socket calls fail Mikko Rapeli
  0 siblings, 2 replies; 11+ messages in thread
From: Dave Jones @ 2013-07-05 14:44 UTC (permalink / raw)
  To: Mikko Rapeli; +Cc: trinity

On Thu, Jul 04, 2013 at 10:55:39PM +0300, Mikko Rapeli wrote:
 > Coverity says:
 > 
 > CID 1042350 (#1 of 1): Uninitialized scalar variable (UNINIT)
 > 23. uninit_use_in_call: Using uninitialized value "ling": field "ling"."l_linger" is uninitialized when calling "setsockopt(int, int, int, void const *, socklen_t)".
 > 
 > Signed-off-by: Mikko Rapeli <mikko.rapeli@iki.fi>
 > ---
 >  trinity.c | 1 +
 >  1 file changed, 1 insertion(+)
 > 
 > diff --git a/trinity.c b/trinity.c
 > index 93e7819..3f80020 100644
 > --- a/trinity.c
 > +++ b/trinity.c
 > @@ -250,6 +250,7 @@ cleanup_fds:
 >  
 >  	for (i = 0; i < nr_sockets; i++) {
 >  		struct linger ling;
 > +		memset(&ling, 0, sizeof(ling));
 >  
 >  		ling.l_onoff = FALSE;	/* linger active */
 >  		setsockopt(shm->socket_fds[i], SOL_SOCKET, SO_LINGER, &ling, sizeof(struct linger));

Let's just do this as 

	struct linger ling = { .l_onoff = FALSE, };

That should have the same effect.

This should be a harmless bug anyway, because l_linger only matters when we're turning
linger on afaik.

	Dave

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

* [PATCHv2 1/2] trinity.c: fix uninitialized variable
  2013-07-05 14:44   ` Dave Jones
@ 2013-07-05 16:15     ` Mikko Rapeli
  2013-07-05 16:15     ` [PATCHv2 2/2] trinity.c: log errors if socket calls fail Mikko Rapeli
  1 sibling, 0 replies; 11+ messages in thread
From: Mikko Rapeli @ 2013-07-05 16:15 UTC (permalink / raw)
  To: trinity

Coverity says:

CID 1042350 (#1 of 1): Uninitialized scalar variable (UNINIT)
23. uninit_use_in_call: Using uninitialized value "ling": field "ling"."l_linger" is uninitialized when calling "setsockopt(int, int, int, void const *, socklen_t)".

Signed-off-by: Mikko Rapeli <mikko.rapeli@iki.fi>
---
 trinity.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/trinity.c b/trinity.c
index 93e7819..32717c0 100644
--- a/trinity.c
+++ b/trinity.c
@@ -249,7 +249,7 @@ int main(int argc, char* argv[])
 cleanup_fds:
 
 	for (i = 0; i < nr_sockets; i++) {
-		struct linger ling;
+		struct linger ling = { .l_onoff = FALSE, };
 
 		ling.l_onoff = FALSE;	/* linger active */
 		setsockopt(shm->socket_fds[i], SOL_SOCKET, SO_LINGER, &ling, sizeof(struct linger));
-- 
1.8.3.1

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

* [PATCHv2 2/2] trinity.c: log errors if socket calls fail
  2013-07-05 14:44   ` Dave Jones
  2013-07-05 16:15     ` [PATCHv2 1/2] " Mikko Rapeli
@ 2013-07-05 16:15     ` Mikko Rapeli
  1 sibling, 0 replies; 11+ messages in thread
From: Mikko Rapeli @ 2013-07-05 16:15 UTC (permalink / raw)
  To: trinity

Maybe that's all that needs to be done at this point.

Coverity CID 1042335 (#1 of 1): Unchecked return value from library
(CHECKED_RETURN)

Signed-off-by: Mikko Rapeli <mikko.rapeli@iki.fi>
---
 trinity.c | 9 +++++++--
 1 file changed, 7 insertions(+), 2 deletions(-)

diff --git a/trinity.c b/trinity.c
index 32717c0..b3829fe 100644
--- a/trinity.c
+++ b/trinity.c
@@ -249,11 +249,16 @@ int main(int argc, char* argv[])
 cleanup_fds:
 
 	for (i = 0; i < nr_sockets; i++) {
+		int r = 0;
 		struct linger ling = { .l_onoff = FALSE, };
 
 		ling.l_onoff = FALSE;	/* linger active */
-		setsockopt(shm->socket_fds[i], SOL_SOCKET, SO_LINGER, &ling, sizeof(struct linger));
-		shutdown(shm->socket_fds[i], SHUT_RDWR);
+		r = setsockopt(shm->socket_fds[i], SOL_SOCKET, SO_LINGER, &ling, sizeof(struct linger));
+		if (r)
+			perror("setsockopt");
+		r = shutdown(shm->socket_fds[i], SHUT_RDWR);
+		if (r)
+			perror("shutdown");
 		close(shm->socket_fds[i]);
 	}
 
-- 
1.8.3.1

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

* [PATCH 2/6] trinity.c: log errors if socket calls fail
  2013-07-04 19:55 [PATCH 0/6] Some simple Coverity fixes Mikko Rapeli
  2013-07-04 19:55 ` [PATCH 1/6] trinity.c: fix uninitialized variable Mikko Rapeli
@ 2013-07-04 19:55 ` Mikko Rapeli
  2013-07-05 14:46   ` Dave Jones
  2013-07-04 19:55 ` [PATCH 3/6] perf_event_open: initialize chars Mikko Rapeli
                   ` (3 subsequent siblings)
  5 siblings, 1 reply; 11+ messages in thread
From: Mikko Rapeli @ 2013-07-04 19:55 UTC (permalink / raw)
  To: trinity

Maybe that's all that needs to be done at this point.

Coverity CID 1042335 (#1 of 1): Unchecked return value from library
(CHECKED_RETURN)

Signed-off-by: Mikko Rapeli <mikko.rapeli@iki.fi>
---
 trinity.c | 9 +++++++--
 1 file changed, 7 insertions(+), 2 deletions(-)

diff --git a/trinity.c b/trinity.c
index 3f80020..de61dcb 100644
--- a/trinity.c
+++ b/trinity.c
@@ -249,12 +249,17 @@ int main(int argc, char* argv[])
 cleanup_fds:
 
 	for (i = 0; i < nr_sockets; i++) {
+		int r = 0;
 		struct linger ling;
 		memset(&ling, 0, sizeof(ling));
 
 		ling.l_onoff = FALSE;	/* linger active */
-		setsockopt(shm->socket_fds[i], SOL_SOCKET, SO_LINGER, &ling, sizeof(struct linger));
-		shutdown(shm->socket_fds[i], SHUT_RDWR);
+		r = setsockopt(shm->socket_fds[i], SOL_SOCKET, SO_LINGER, &ling, sizeof(struct linger));
+		if (r)
+			perror("setsockopt");
+		r = shutdown(shm->socket_fds[i], SHUT_RDWR);
+		if (r)
+			perror("shutdown");
 		close(shm->socket_fds[i]);
 	}
 
-- 
1.8.3.1

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

* Re: [PATCH 2/6] trinity.c: log errors if socket calls fail
  2013-07-04 19:55 ` [PATCH 2/6] " Mikko Rapeli
@ 2013-07-05 14:46   ` Dave Jones
  0 siblings, 0 replies; 11+ messages in thread
From: Dave Jones @ 2013-07-05 14:46 UTC (permalink / raw)
  To: Mikko Rapeli; +Cc: trinity

On Thu, Jul 04, 2013 at 10:55:40PM +0300, Mikko Rapeli wrote:
 > Maybe that's all that needs to be done at this point.
 > 
 > Coverity CID 1042335 (#1 of 1): Unchecked return value from library
 > (CHECKED_RETURN)
 > 
 > Signed-off-by: Mikko Rapeli <mikko.rapeli@iki.fi>
 > ---
 >  trinity.c | 9 +++++++--
 >  1 file changed, 7 insertions(+), 2 deletions(-)
 > 
 > diff --git a/trinity.c b/trinity.c
 > index 3f80020..de61dcb 100644
 > --- a/trinity.c
 > +++ b/trinity.c
 > @@ -249,12 +249,17 @@ int main(int argc, char* argv[])
 >  cleanup_fds:
 >  
 >  	for (i = 0; i < nr_sockets; i++) {
 > +		int r = 0;
 >  		struct linger ling;
 >  		memset(&ling, 0, sizeof(ling));
 >  
 >  		ling.l_onoff = FALSE;	/* linger active */
 > -		setsockopt(shm->socket_fds[i], SOL_SOCKET, SO_LINGER, &ling, sizeof(struct linger));
 > -		shutdown(shm->socket_fds[i], SHUT_RDWR);
 > +		r = setsockopt(shm->socket_fds[i], SOL_SOCKET, SO_LINGER, &ling, sizeof(struct linger));
 > +		if (r)
 > +			perror("setsockopt");
 > +		r = shutdown(shm->socket_fds[i], SHUT_RDWR);
 > +		if (r)
 > +			perror("shutdown");
 >  		close(shm->socket_fds[i]);
 >  	}

Resubmit this one after redoing that linger diff too, as it'll need rebasing.

thanks,

	Dave

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

* [PATCH 3/6] perf_event_open: initialize chars
  2013-07-04 19:55 [PATCH 0/6] Some simple Coverity fixes Mikko Rapeli
  2013-07-04 19:55 ` [PATCH 1/6] trinity.c: fix uninitialized variable Mikko Rapeli
  2013-07-04 19:55 ` [PATCH 2/6] " Mikko Rapeli
@ 2013-07-04 19:55 ` Mikko Rapeli
  2013-07-04 19:55 ` [PATCH 4/6] perf_event_open.c: close dir's on exit paths Mikko Rapeli
                   ` (2 subsequent siblings)
  5 siblings, 0 replies; 11+ messages in thread
From: Mikko Rapeli @ 2013-07-04 19:55 UTC (permalink / raw)
  To: trinity

Just in case if they get used like in Coverity CID 1042349 and 1042348.

Signed-off-by: Mikko Rapeli <mikko.rapeli@iki.fi>
---
 syscalls/perf_event_open.c | 8 ++++++--
 1 file changed, 6 insertions(+), 2 deletions(-)

diff --git a/syscalls/perf_event_open.c b/syscalls/perf_event_open.c
index 5df6bed..ba5c872 100644
--- a/syscalls/perf_event_open.c
+++ b/syscalls/perf_event_open.c
@@ -219,8 +219,12 @@ static int init_pmus(void) {
 
 	DIR *dir,*event_dir,*format_dir;
 	struct dirent *entry,*event_entry,*format_entry;
-	char dir_name[BUFSIZ],event_name[BUFSIZ],event_value[BUFSIZ],
-		temp_name[BUFSIZ],format_name[BUFSIZ],format_value[BUFSIZ];
+	char dir_name[BUFSIZ] = "";
+	char event_name[BUFSIZ] = "";
+	char event_value[BUFSIZ] = "";
+	char temp_name[BUFSIZ] = "";
+	char format_name[BUFSIZ] = "";
+	char format_value[BUFSIZ] = "";
 	int type,pmu_num=0,format_num=0,generic_num=0;
 	FILE *fff;
 	int result;
-- 
1.8.3.1

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

* [PATCH 4/6] perf_event_open.c: close dir's on exit paths
  2013-07-04 19:55 [PATCH 0/6] Some simple Coverity fixes Mikko Rapeli
                   ` (2 preceding siblings ...)
  2013-07-04 19:55 ` [PATCH 3/6] perf_event_open: initialize chars Mikko Rapeli
@ 2013-07-04 19:55 ` Mikko Rapeli
  2013-07-04 19:55 ` [PATCH 5/6] sockets.c: don't leak cachefile on return paths Mikko Rapeli
  2013-07-04 19:55 ` [PATCH 6/6] maps.c: only close() if fd is valid Mikko Rapeli
  5 siblings, 0 replies; 11+ messages in thread
From: Mikko Rapeli @ 2013-07-04 19:55 UTC (permalink / raw)
  To: trinity

Don't leaky fd's so much. Fixes Coverity CID's 1042345, 1042346 and 1042347.

Signed-off-by: Mikko Rapeli <mikko.rapeli@iki.fi>
---
 syscalls/perf_event_open.c | 4 ++++
 1 file changed, 4 insertions(+)

diff --git a/syscalls/perf_event_open.c b/syscalls/perf_event_open.c
index ba5c872..edc64eb 100644
--- a/syscalls/perf_event_open.c
+++ b/syscalls/perf_event_open.c
@@ -304,6 +304,8 @@ static int init_pmus(void) {
 							sizeof(struct format_type));
 			if (pmus[pmu_num].formats==NULL) {
 				pmus[pmu_num].num_formats=0;
+				closedir(dir);
+				closedir(format_dir);
 				return -1;
 			}
 
@@ -368,6 +370,8 @@ static int init_pmus(void) {
 				sizeof(struct generic_event_type));
 			if (pmus[pmu_num].generic_events==NULL) {
 				pmus[pmu_num].num_generic_events=0;
+				closedir(dir);
+				closedir(event_dir);
 				return -1;
 			}
 
-- 
1.8.3.1

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

* [PATCH 5/6] sockets.c: don't leak cachefile on return paths
  2013-07-04 19:55 [PATCH 0/6] Some simple Coverity fixes Mikko Rapeli
                   ` (3 preceding siblings ...)
  2013-07-04 19:55 ` [PATCH 4/6] perf_event_open.c: close dir's on exit paths Mikko Rapeli
@ 2013-07-04 19:55 ` Mikko Rapeli
  2013-07-04 19:55 ` [PATCH 6/6] maps.c: only close() if fd is valid Mikko Rapeli
  5 siblings, 0 replies; 11+ messages in thread
From: Mikko Rapeli @ 2013-07-04 19:55 UTC (permalink / raw)
  To: trinity

Fixes Coverity CID's 1042341 and 1042342.

Signed-off-by: Mikko Rapeli <mikko.rapeli@iki.fi>
---
 sockets.c | 10 +++++++---
 1 file changed, 7 insertions(+), 3 deletions(-)

diff --git a/sockets.c b/sockets.c
index 7eade96..75acc5b 100644
--- a/sockets.c
+++ b/sockets.c
@@ -99,8 +99,10 @@ void generate_sockets(void)
 
 	while (nr_to_create > 0) {
 
-		if (shm->exit_reason != STILL_RUNNING)
+		if (shm->exit_reason != STILL_RUNNING) {
+			close(cachefile);
 			return;
+		}
 
 		/* Pretend we're child 0 and we've called sys_socket */
 		sanitise_socket(0);
@@ -213,6 +215,7 @@ void open_sockets(void)
 			if (domain != specific_proto) {
 				printf("ignoring socket cachefile due to specific protocol request, and stale data in cachefile.\n");
 				generate_sockets();
+				close(cachefile);
 				return;
 			}
 		}
@@ -231,9 +234,10 @@ regenerate:
 		}
 
 		/* check for ctrl-c */
-		if (shm->exit_reason != STILL_RUNNING)
+		if (shm->exit_reason != STILL_RUNNING) {
+			close(cachefile);
 			return;
-
+		}
 	}
 
 	if (nr_sockets < NR_SOCKET_FDS) {
-- 
1.8.3.1

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

* [PATCH 6/6] maps.c: only close() if fd is valid
  2013-07-04 19:55 [PATCH 0/6] Some simple Coverity fixes Mikko Rapeli
                   ` (4 preceding siblings ...)
  2013-07-04 19:55 ` [PATCH 5/6] sockets.c: don't leak cachefile on return paths Mikko Rapeli
@ 2013-07-04 19:55 ` Mikko Rapeli
  5 siblings, 0 replies; 11+ messages in thread
From: Mikko Rapeli @ 2013-07-04 19:55 UTC (permalink / raw)
  To: trinity

Fixes Coverity CID 1042340.

Signed-off-by: Mikko Rapeli <mikko.rapeli@iki.fi>
---
 maps.c | 3 ++-
 1 file changed, 2 insertions(+), 1 deletion(-)

diff --git a/maps.c b/maps.c
index 62b2328..b61575e 100644
--- a/maps.c
+++ b/maps.c
@@ -114,7 +114,8 @@ static void * alloc_zero_map(struct map *map, int prot, const char *name)
 	output(2, "mapping[%d]: (zeropage %s) %p (%lu bytes)\n",
 			num_mappings - 1, name, tmpmap->ptr, size);
 
-	close(fd);
+	if (fd >= 0)
+		close(fd);
 	return tmpmap;
 }
 
-- 
1.8.3.1

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

end of thread, other threads:[~2013-07-05 16:15 UTC | newest]

Thread overview: 11+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
2013-07-04 19:55 [PATCH 0/6] Some simple Coverity fixes Mikko Rapeli
2013-07-04 19:55 ` [PATCH 1/6] trinity.c: fix uninitialized variable Mikko Rapeli
2013-07-05 14:44   ` Dave Jones
2013-07-05 16:15     ` [PATCHv2 1/2] " Mikko Rapeli
2013-07-05 16:15     ` [PATCHv2 2/2] trinity.c: log errors if socket calls fail Mikko Rapeli
2013-07-04 19:55 ` [PATCH 2/6] " Mikko Rapeli
2013-07-05 14:46   ` Dave Jones
2013-07-04 19:55 ` [PATCH 3/6] perf_event_open: initialize chars Mikko Rapeli
2013-07-04 19:55 ` [PATCH 4/6] perf_event_open.c: close dir's on exit paths Mikko Rapeli
2013-07-04 19:55 ` [PATCH 5/6] sockets.c: don't leak cachefile on return paths Mikko Rapeli
2013-07-04 19:55 ` [PATCH 6/6] maps.c: only close() if fd is valid Mikko Rapeli

This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.