* [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
* [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
* [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
* 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
* 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
* [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
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 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).