From b038eb64348f2500e7e0ad9c3aa6e310e41cfe2e Mon Sep 17 00:00:00 2001 From: Nathan Hjelm Date: Thu, 28 May 2015 11:26:13 -0600 Subject: [PATCH] btl/openib: more coverity fixes CID 1301390 Dereference before null check (REVERSE_INULL) endpoint can not be NULL here. Remove NULL check. CID 1269836 Unintentional integer overflow (OVERFLOW_BEFORE_WIDEN) CID 1301388 Bad bit shift operation (BAD_SHIFT) Add ull to integer constants to ensure the math is done in 64-bits not 32. CID 715749 Explicit null dereferenced (FORWARD_NULL) As far as I can tell this parser function does not accept a line that does match key = value. If that is the case then value should never be NULL. If it is it is a parse error. Updated the code to reflect this. Also modified the intify function to do something more sane (strtol vs atoi with hex detection). CID 1269820 Dereference null return value (NULL_RETURNS) This is a false positive as strchr will never return NULL here. It makes sense, though, to quiet the warning by changing the do {} while () loop to a while () loop. CID 1269780 Dereference after null check (FORWARD_NULL) Just return an error if the endpoint's cpc data is NULL. Signed-off-by: Nathan Hjelm --- opal/mca/btl/openib/btl_openib_component.c | 8 ++-- opal/mca/btl/openib/btl_openib_ini.c | 48 +++++++------------ .../openib/connect/btl_openib_connect_udcm.c | 10 ++-- 3 files changed, 25 insertions(+), 41 deletions(-) diff --git a/opal/mca/btl/openib/btl_openib_component.c b/opal/mca/btl/openib/btl_openib_component.c index 50ea399a30..85ba474241 100644 --- a/opal/mca/btl/openib/btl_openib_component.c +++ b/opal/mca/btl/openib/btl_openib_component.c @@ -1544,8 +1544,8 @@ static uint64_t calculate_max_reg (const char *device_name) } else if (!strncmp(device_name, "mlx4", 4)) { if (0 == stat("/sys/module/mlx4_core/parameters/log_num_mtt", &statinfo)) { - mtts_per_seg = 1 << read_module_param("/sys/module/mlx4_core/parameters/log_mtts_per_seg", 1, 63); - num_mtt = 1 << read_module_param("/sys/module/mlx4_core/parameters/log_mtts_per_seg", 1, 63); + mtts_per_seg = 1ull << read_module_param("/sys/module/mlx4_core/parameters/log_mtts_per_seg", 1, 63); + num_mtt = 1ull << read_module_param("/sys/module/mlx4_core/parameters/log_mtts_per_seg", 1, 63); if (1 == num_mtt) { /* NTH: is 19 a minimum? when log_num_mtt is set to 0 use 19 */ num_mtt = 1 << 19; @@ -1557,7 +1557,7 @@ static uint64_t calculate_max_reg (const char *device_name) } else if (!strncmp(device_name, "mthca", 5)) { if (0 == stat("/sys/module/ib_mthca/parameters/num_mtt", &statinfo)) { - mtts_per_seg = 1 << read_module_param("/sys/module/ib_mthca/parameters/log_mtts_per_seg", 1, 63); + mtts_per_seg = 1ull << read_module_param("/sys/module/ib_mthca/parameters/log_mtts_per_seg", 1, 63); num_mtt = read_module_param("/sys/module/ib_mthca/parameters/num_mtt", 1 << 20, (uint64_t) -1); reserved_mtt = read_module_param("/sys/module/ib_mthca/parameters/fmr_reserved_mtts", 0, (uint64_t) -1); @@ -3544,7 +3544,7 @@ static void handle_wc(mca_btl_openib_device_t* device, const uint32_t cq, return; error: - if(endpoint && endpoint->endpoint_proc && endpoint->endpoint_proc->proc_opal) + if(endpoint->endpoint_proc && endpoint->endpoint_proc->proc_opal) remote_proc = endpoint->endpoint_proc->proc_opal; /* For iWARP, the TCP connection is tied to the QP once the QP is diff --git a/opal/mca/btl/openib/btl_openib_ini.c b/opal/mca/btl/openib/btl_openib_ini.c index 43b84bddc0..a61cf28e27 100644 --- a/opal/mca/btl/openib/btl_openib_ini.c +++ b/opal/mca/btl/openib/btl_openib_ini.c @@ -1,3 +1,4 @@ +/* -*- Mode: C; c-basic-offset:4 ; indent-tabs-mode:nil -*- */ /* * Copyright (c) 2004-2005 The Trustees of Indiana University and Indiana * University Research and Technology @@ -11,7 +12,7 @@ * All rights reserved. * Copyright (c) 2006-2013 Cisco Systems, Inc. All rights reserved. * Copyright (c) 2008 Mellanox Technologies. All rights reserved. - * Copyright (c) 2012-2013 Los Alamos National Security, LLC. All rights + * Copyright (c) 2012-2015 Los Alamos National Security, LLC. All rights * reserved. * Copyright (c) 2014 Intel, Inc. All rights reserved * Copyright (c) 2014-2015 Research Organization for Information Science @@ -345,25 +346,20 @@ static int parse_line(parsed_section_values_t *sv) /* Next we get the value */ val = btl_openib_ini_yylex(); - if (BTL_OPENIB_INI_PARSE_SINGLE_WORD == val || - BTL_OPENIB_INI_PARSE_VALUE == val) { - value = strdup(btl_openib_ini_yytext); - - /* Now we need to see the newline */ - val = btl_openib_ini_yylex(); - if (BTL_OPENIB_INI_PARSE_NEWLINE != val && - BTL_OPENIB_INI_PARSE_DONE != val) { - opal_show_help("help-mpi-btl-openib.txt", "ini file:expected newline", true); - free(value); - return OPAL_ERROR; - } + if (BTL_OPENIB_INI_PARSE_SINGLE_WORD != val && BTL_OPENIB_INI_PARSE_VALUE != val) { + return OPAL_ERROR; } + value = strdup(btl_openib_ini_yytext); + + /* Now we need to see the newline */ + val = btl_openib_ini_yylex(); + /* If we did not get EOL or EOF, something is wrong */ - else if (BTL_OPENIB_INI_PARSE_DONE != val && - BTL_OPENIB_INI_PARSE_NEWLINE != val) { - opal_show_help("help-mpi-btl-openib.txt", "ini file:expected newline", true); - return OPAL_ERROR; + if (BTL_OPENIB_INI_PARSE_NEWLINE != val && BTL_OPENIB_INI_PARSE_DONE != val) { + opal_show_help("help-mpi-btl-openib.txt", "ini file:expected newline", true); + free(value); + return OPAL_ERROR; } /* Ok, we got a good parse. Now figure out what it is and save @@ -621,19 +617,7 @@ static int save_section(parsed_section_values_t *s) */ int opal_btl_openib_ini_intify(char *str) { - while (isspace(*str)) { - ++str; - } - - /* If it's hex, use sscanf() */ - if (strlen(str) > 3 && 0 == strncasecmp("0x", str, 2)) { - unsigned int i; - sscanf(str, "%X", &i); - return (int) i; - } - - /* Nope -- just decimal, so use atoi() */ - return atoi(str); + return strtol (str, NULL, 0); } @@ -676,13 +660,13 @@ int opal_btl_openib_ini_intify_list(char *value, uint32_t **values, int *len) /* Iterate over the values and save them */ str = value; comma = strchr(str, ','); - do { + while (NULL != comma) { *comma = '\0'; (*values)[*len] = (uint32_t) opal_btl_openib_ini_intify(str); ++(*len); str = comma + 1; comma = strchr(str, ','); - } while (NULL != comma); + } /* Get the last value (i.e., the value after the last comma, because it won't have been snarfed in the loop) */ diff --git a/opal/mca/btl/openib/connect/btl_openib_connect_udcm.c b/opal/mca/btl/openib/connect/btl_openib_connect_udcm.c index cb83d36909..85113739a0 100644 --- a/opal/mca/btl/openib/connect/btl_openib_connect_udcm.c +++ b/opal/mca/btl/openib/connect/btl_openib_connect_udcm.c @@ -3,7 +3,7 @@ * Copyright (c) 2007-2013 Cisco Systems, Inc. All rights reserved. * Copyright (c) 2008-2009 Mellanox Technologies. All rights reserved. * Copyright (c) 2009 IBM Corporation. All rights reserved. - * Copyright (c) 2011-2014 Los Alamos National Security, LLC. All rights + * Copyright (c) 2011-2015 Los Alamos National Security, LLC. All rights * reserved. * Copyright (c) 2014-2015 Research Organization for Information Science * and Technology (RIST). All rights reserved. @@ -1804,11 +1804,11 @@ static int udcm_handle_connect(mca_btl_openib_endpoint_t *lcl_ep, udcm_endpoint_t *udep = UDCM_ENDPOINT_DATA(lcl_ep); int rc = OPAL_ERROR; - do { - if (NULL == udep) { - break; - } + if (NULL == udep) { + return OPAL_ERROR; + } + do { opal_mutex_lock (&udep->udep_lock); if (true == udep->recv_req) {