From 7e349977463bbc34858f20503afe231aa3afceb6 Mon Sep 17 00:00:00 2001 From: Nathan Hjelm Date: Mon, 1 Jun 2015 15:38:54 -0600 Subject: [PATCH 1/2] event/libevent2022: fix coverity issue CID 1269841 Out-of-bounds access (OVERRUN) Correct issue. If the string being concatingated fills the remaining buffer then a \0 is written past the end of the string. In practice this should never happen but it should be fixed. I re-organized the code a bit to clear this error. Signed-off-by: Nathan Hjelm --- .../libevent2022/libevent2022_component.c | 18 ++++++++---------- 1 file changed, 8 insertions(+), 10 deletions(-) diff --git a/opal/mca/event/libevent2022/libevent2022_component.c b/opal/mca/event/libevent2022/libevent2022_component.c index 632a5ad0a4..c87e51772c 100644 --- a/opal/mca/event/libevent2022/libevent2022_component.c +++ b/opal/mca/event/libevent2022/libevent2022_component.c @@ -125,7 +125,7 @@ static int libevent2022_register (void) const struct eventop** _eventop = eventops; char available_eventops[1024] = "none"; char *help_msg = NULL; - int ret, len = 1024; + int ret; /* Retrieve the upper level specified event system, if any. * Default to select() on OS X and poll() everywhere else because @@ -153,17 +153,15 @@ static int libevent2022_register (void) */ if (NULL != (*_eventop)) { - available_eventops[0] = '\0'; - } + const int len = sizeof (available_eventops); + int cur_len = snprintf (available_eventops, len, "%s", (*(_eventop++))->name); - while( NULL != (*_eventop) ) { - if( available_eventops[0] != '\0' ) { - (void) strncat (available_eventops, ", ", len); + for (int i = 1 ; eventops[i] && cur_len < len ; ++i) { + cur_len += snprintf (available_eventops + cur_len, len - cur_len, ", %s", + eventops[i]->name); } - (void) strncat (available_eventops, (*_eventop)->name, - len); - _eventop++; /* go to the next available eventop */ - len = 1024 - strlen(available_eventops); + /* ensure the available_eventops string is always NULL-terminated */ + available_eventops[len - 1] = '\0'; } #ifdef __APPLE__ From f72b6d45c750124f535006f602d3a4dd7ad251ee Mon Sep 17 00:00:00 2001 From: Nathan Hjelm Date: Mon, 1 Jun 2015 15:51:03 -0600 Subject: [PATCH 2/2] crs/none: fix coverity issues CID 1301389 Resource leak (RESOURCE_LEAK) There is no conceivable reason to strdup cr_argv[0] in either location. Removed the calls to strdup. CID 741357 Resource leak (RESOURCE_LEAK) cr_argv was created by opal_argv_split (tmp_argv[0], ' '). Why should we call opal_argv_join (' ') on this array. Leak fixed by printing out tmp_argv[0] instead of calling opal_argv_join. CID 741358 Resource leak (RESOURCE_LEAK) The code does not handle exec failure correctly. The error should be communicated to the parent process but the function in question is only called by the parent. This calls into question some of the structure of the function in general (like what is the point of returning the child process id). That said, I will go ahead and add the opal_argv_free to quiet this error. Signed-off-by: Nathan Hjelm --- opal/mca/crs/none/crs_none_module.c | 18 ++++++++++-------- 1 file changed, 10 insertions(+), 8 deletions(-) diff --git a/opal/mca/crs/none/crs_none_module.c b/opal/mca/crs/none/crs_none_module.c index 42573a30ec..c05327359f 100644 --- a/opal/mca/crs/none/crs_none_module.c +++ b/opal/mca/crs/none/crs_none_module.c @@ -1,8 +1,11 @@ +/* -*- Mode: C; c-basic-offset:4 ; indent-tabs-mode:nil -*- */ /* * Copyright (c) 2004-2010 The Trustees of Indiana University. * All rights reserved. * * Copyright (c) 2015 Cisco Systems, Inc. All rights reserved. + * Copyright (c) 2015 Los Alamos National Security, LLC. All rights + * reserved. * $COPYRIGHT$ * * Additional copyrights may follow @@ -101,8 +104,7 @@ int opal_crs_none_restart(opal_crs_base_snapshot_t *base_snapshot, bool spawn_ch opal_output(0, "crs:none: checkpoint(): Error: Unable to open the file (%s)", base_snapshot->metadata_filename); - exit_status = OPAL_ERROR; - goto cleanup; + return OPAL_ERROR; } } @@ -131,10 +133,9 @@ int opal_crs_none_restart(opal_crs_base_snapshot_t *base_snapshot, bool spawn_ch if( !spawn_child ) { opal_output_verbose(10, opal_crs_base_framework.framework_output, "crs:none: none_restart: exec :(%s, %s):", - strdup(cr_argv[0]), - opal_argv_join(cr_argv, ' ')); + cr_argv[0], tmp_argv[0]); - status = execvp(strdup(cr_argv[0]), cr_argv); + status = execvp(cr_argv[0], cr_argv); if(status < 0) { opal_output(opal_crs_base_framework.framework_output, @@ -152,10 +153,11 @@ int opal_crs_none_restart(opal_crs_base_snapshot_t *base_snapshot, bool spawn_ch } cleanup: - if (NULL != base_snapshot->metadata) { - fclose(base_snapshot->metadata); + if (cr_argv) { + opal_argv_free (cr_argv); } - base_snapshot->metadata = NULL; + + fclose(base_snapshot->metadata); return exit_status; }