From d13def78ccef6dbc25c2e197089ee5fc4d7b82c3 Mon Sep 17 00:00:00 2001 From: Eric van Gyzen Date: Wed, 15 Jul 2020 13:17:16 +0000 Subject: [PATCH] Fix Coverity issues in OFED read_ibdiag_config NULL deref read_ibdiag_config mem leak ib_mad_inv_field_str Missing comma in a string array initialization print_node_header NULL deref diff_node_ports copy-paste error ibportstate.c main() missing break in switch set_thresholds NULL ptr deref dump_unicast_tables leaks mapnd umad_cm_attr_str dead code __ibv_close_device close(-1) check return value of listen() mlx5 bitmap.h - bad bit shift - UB get_dst_addr check return value of inet_pton osm_perfmgr_init check return value of cl_spinlock_init osm_port_new memory leak on error path sa_mad_ctrl_rcv_callback missing break in switch case I did not include CID numbers because these were found by an internal run at Isilon. Reviewed by: cem kib MFC after: 2 weeks Sponsored by: Dell EMC Isilon Differential Revision: https://reviews.freebsd.org/D25676 --- contrib/ofed/infiniband-diags/src/ibdiag_common.c | 8 ++++++++ contrib/ofed/infiniband-diags/src/ibdiag_sa.c | 2 +- contrib/ofed/infiniband-diags/src/iblinkinfo.c | 5 +++-- contrib/ofed/infiniband-diags/src/ibportstate.c | 1 + contrib/ofed/infiniband-diags/src/ibqueryerrors.c | 7 +++++++ contrib/ofed/infiniband-diags/src/ibroute.c | 3 ++- contrib/ofed/libibumad/umad_str.c | 3 --- contrib/ofed/libibverbs/device.c | 3 --- contrib/ofed/libibverbs/examples/rc_pingpong.c | 6 +++++- contrib/ofed/libibverbs/examples/srq_pingpong.c | 6 +++++- contrib/ofed/libibverbs/examples/uc_pingpong.c | 6 +++++- contrib/ofed/libibverbs/examples/ud_pingpong.c | 6 +++++- contrib/ofed/libibverbs/examples/xsrq_pingpong.c | 6 +++++- contrib/ofed/libmlx5/bitmap.h | 8 ++++---- contrib/ofed/librdmacm/examples/mckey.c | 3 +-- contrib/ofed/opensm/opensm/osm_perfmgr.c | 4 +++- contrib/ofed/opensm/opensm/osm_port.c | 4 +++- contrib/ofed/opensm/opensm/osm_sa_mad_ctrl.c | 1 + 18 files changed, 59 insertions(+), 23 deletions(-) diff --git a/contrib/ofed/infiniband-diags/src/ibdiag_common.c b/contrib/ofed/infiniband-diags/src/ibdiag_common.c index ddaee8c5b559..28dedc19b7ff 100644 --- a/contrib/ofed/infiniband-diags/src/ibdiag_common.c +++ b/contrib/ofed/infiniband-diags/src/ibdiag_common.c @@ -120,6 +120,7 @@ static inline int val_str_true(const char *val_str) void read_ibdiag_config(const char *file) { char buf[1024]; + char orig_buf[1024]; FILE *config_fd = NULL; char *p_prefix, *p_last; char *name; @@ -142,8 +143,14 @@ void read_ibdiag_config(const char *file) if (*p_prefix == '#') continue; /* ignore comment lines */ + strlcpy(orig_buf, buf, sizeof(orig_buf)); name = strtok_r(p_prefix, "=", &p_last); val_str = strtok_r(NULL, "\n", &p_last); + if (!name || !val_str) { + fprintf(stderr, "%s: malformed line in \"%s\":\n%s\n", + prog_name, file, orig_buf); + continue; + } if (strncmp(name, "CA", strlen("CA")) == 0) { free(ibd_ca); @@ -165,6 +172,7 @@ void read_ibdiag_config(const char *file) ibd_sakey = strtoull(val_str, 0, 0); } else if (strncmp(name, "nd_format", strlen("nd_format")) == 0) { + free(ibd_nd_format); ibd_nd_format = strdup(val_str); } } diff --git a/contrib/ofed/infiniband-diags/src/ibdiag_sa.c b/contrib/ofed/infiniband-diags/src/ibdiag_sa.c index ea272a976d75..7154ac1b7d2c 100644 --- a/contrib/ofed/infiniband-diags/src/ibdiag_sa.c +++ b/contrib/ofed/infiniband-diags/src/ibdiag_sa.c @@ -222,7 +222,7 @@ static const char *ib_mad_inv_field_str[] = { "MAD Reserved", "MAD Reserved", "MAD Reserved", - "MAD Invalid value in Attribute field(s) or Attribute Modifier" + "MAD Invalid value in Attribute field(s) or Attribute Modifier", "MAD UNKNOWN ERROR" }; #define MAD_ERR_UNKNOWN (ARR_SIZE(ib_mad_inv_field_str) - 1) diff --git a/contrib/ofed/infiniband-diags/src/iblinkinfo.c b/contrib/ofed/infiniband-diags/src/iblinkinfo.c index 40e012d44769..13ac3e2eabc5 100644 --- a/contrib/ofed/infiniband-diags/src/iblinkinfo.c +++ b/contrib/ofed/infiniband-diags/src/iblinkinfo.c @@ -293,7 +293,8 @@ void print_node_header(ibnd_node_t *node, int *out_header_flag, printf("%s%s: %s:\n", out_prefix ? out_prefix : "", nodetype_str(node), remap); - (*out_header_flag)++; + if (out_header_flag) + (*out_header_flag)++; free(remap); } } @@ -397,7 +398,7 @@ void diff_node_ports(ibnd_node_t * fabric1_node, ibnd_node_t * fabric2_node, } if (output_diff && fabric2_port) { - print_node_header(fabric1_node, + print_node_header(fabric2_node, head_print, NULL); print_port(fabric2_node, diff --git a/contrib/ofed/infiniband-diags/src/ibportstate.c b/contrib/ofed/infiniband-diags/src/ibportstate.c index 06bd5d217550..7bea3398fb76 100644 --- a/contrib/ofed/infiniband-diags/src/ibportstate.c +++ b/contrib/ofed/infiniband-diags/src/ibportstate.c @@ -564,6 +564,7 @@ int main(int argc, char **argv) printf("Port is already in enable state\n"); goto close_port; } + /* FALLTHROUGH */ case ENABLE: case RESET: /* Polling */ diff --git a/contrib/ofed/infiniband-diags/src/ibqueryerrors.c b/contrib/ofed/infiniband-diags/src/ibqueryerrors.c index 2329f9147718..3efaf135e62f 100644 --- a/contrib/ofed/infiniband-diags/src/ibqueryerrors.c +++ b/contrib/ofed/infiniband-diags/src/ibqueryerrors.c @@ -130,6 +130,7 @@ static void set_thres(char *name, uint32_t val) static void set_thresholds(char *threshold_file) { char buf[1024]; + char orig_buf[1024]; int val = 0; FILE *thresf = fopen(threshold_file, "r"); char *p_prefix, *p_last; @@ -156,8 +157,14 @@ static void set_thresholds(char *threshold_file) if (*p_prefix == '#') continue; /* ignore comment lines */ + strlcpy(orig_buf, buf, sizeof(orig_buf)); name = strtok_r(p_prefix, "=", &p_last); val_str = strtok_r(NULL, "\n", &p_last); + if (!name || !val_str) { + fprintf(stderr, "malformed line in \"%s\":\n%s\n", + threshold_file, orig_buf); + continue; + } val = strtoul(val_str, NULL, 0); set_thres(name, val); diff --git a/contrib/ofed/infiniband-diags/src/ibroute.c b/contrib/ofed/infiniband-diags/src/ibroute.c index 8e4544edb6e3..62abb9a8e1ae 100644 --- a/contrib/ofed/infiniband-diags/src/ibroute.c +++ b/contrib/ofed/infiniband-diags/src/ibroute.c @@ -354,6 +354,8 @@ char *dump_unicast_tables(ib_portid_t * portid, int startlid, int endlid) " (%s):\n", startlid, endlid, portid2str(portid), nodeguid, mapnd); + free(mapnd); + DEBUG("Switch top is 0x%x\n", top); printf(" Lid Out Destination\n"); @@ -390,7 +392,6 @@ char *dump_unicast_tables(ib_portid_t * portid, int startlid, int endlid) } printf("%d %slids dumped \n", n, dump_all ? "" : "valid "); - free(mapnd); return 0; } diff --git a/contrib/ofed/libibumad/umad_str.c b/contrib/ofed/libibumad/umad_str.c index 0a014f7e2021..412441375a2f 100644 --- a/contrib/ofed/libibumad/umad_str.c +++ b/contrib/ofed/libibumad/umad_str.c @@ -246,7 +246,6 @@ static const char * umad_sm_attr_str(__be16 attr_id) default: return (umad_common_attr_str(attr_id)); } - return (""); } static const char * umad_sa_attr_str(__be16 attr_id) @@ -301,7 +300,6 @@ static const char * umad_sa_attr_str(__be16 attr_id) default: return (umad_common_attr_str(attr_id)); } - return (""); } static const char * umad_cm_attr_str(__be16 attr_id) @@ -336,7 +334,6 @@ static const char * umad_cm_attr_str(__be16 attr_id) default: return (umad_common_attr_str(attr_id)); } - return (""); } const char * umad_attribute_str(uint8_t mgmt_class, __be16 attr_id) diff --git a/contrib/ofed/libibverbs/device.c b/contrib/ofed/libibverbs/device.c index 8b52dd51306b..d5cd2173cd8b 100644 --- a/contrib/ofed/libibverbs/device.c +++ b/contrib/ofed/libibverbs/device.c @@ -264,7 +264,6 @@ int __ibv_close_device(struct ibv_context *context) { int async_fd = context->async_fd; int cmd_fd = context->cmd_fd; - int cq_fd = -1; struct verbs_context *context_ex; struct verbs_device *verbs_device = verbs_get_device(context->device); @@ -279,8 +278,6 @@ int __ibv_close_device(struct ibv_context *context) close(async_fd); close(cmd_fd); - if (abi_ver <= 2) - close(cq_fd); return 0; } diff --git a/contrib/ofed/libibverbs/examples/rc_pingpong.c b/contrib/ofed/libibverbs/examples/rc_pingpong.c index d01bdb80092a..fc95311aaf40 100644 --- a/contrib/ofed/libibverbs/examples/rc_pingpong.c +++ b/contrib/ofed/libibverbs/examples/rc_pingpong.c @@ -273,7 +273,11 @@ static struct pingpong_dest *pp_server_exch_dest(struct pingpong_context *ctx, return NULL; } - listen(sockfd, 1); + if (listen(sockfd, 1)) { + perror("listen() failed"); + close(sockfd); + return NULL; + } connfd = accept(sockfd, NULL, NULL); close(sockfd); if (connfd < 0) { diff --git a/contrib/ofed/libibverbs/examples/srq_pingpong.c b/contrib/ofed/libibverbs/examples/srq_pingpong.c index 1b40aa6076c0..2df8a2e3959c 100644 --- a/contrib/ofed/libibverbs/examples/srq_pingpong.c +++ b/contrib/ofed/libibverbs/examples/srq_pingpong.c @@ -283,7 +283,11 @@ static struct pingpong_dest *pp_server_exch_dest(struct pingpong_context *ctx, return NULL; } - listen(sockfd, 1); + if (listen(sockfd, 1)) { + perror("listen() failed"); + close(sockfd); + return NULL; + } connfd = accept(sockfd, NULL, NULL); close(sockfd); if (connfd < 0) { diff --git a/contrib/ofed/libibverbs/examples/uc_pingpong.c b/contrib/ofed/libibverbs/examples/uc_pingpong.c index fb030dc535b8..d6b2c5fdc26a 100644 --- a/contrib/ofed/libibverbs/examples/uc_pingpong.c +++ b/contrib/ofed/libibverbs/examples/uc_pingpong.c @@ -247,7 +247,11 @@ static struct pingpong_dest *pp_server_exch_dest(struct pingpong_context *ctx, return NULL; } - listen(sockfd, 1); + if (listen(sockfd, 1)) { + perror("listen() failed"); + close(sockfd); + return NULL; + } connfd = accept(sockfd, NULL, NULL); close(sockfd); if (connfd < 0) { diff --git a/contrib/ofed/libibverbs/examples/ud_pingpong.c b/contrib/ofed/libibverbs/examples/ud_pingpong.c index 419925ab1d40..cb9f6037bb1f 100644 --- a/contrib/ofed/libibverbs/examples/ud_pingpong.c +++ b/contrib/ofed/libibverbs/examples/ud_pingpong.c @@ -245,7 +245,11 @@ static struct pingpong_dest *pp_server_exch_dest(struct pingpong_context *ctx, return NULL; } - listen(sockfd, 1); + if (listen(sockfd, 1)) { + perror("listen() failed"); + close(sockfd); + return NULL; + } connfd = accept(sockfd, NULL, NULL); close(sockfd); if (connfd < 0) { diff --git a/contrib/ofed/libibverbs/examples/xsrq_pingpong.c b/contrib/ofed/libibverbs/examples/xsrq_pingpong.c index b3c7f703441f..31b2d0edeb36 100644 --- a/contrib/ofed/libibverbs/examples/xsrq_pingpong.c +++ b/contrib/ofed/libibverbs/examples/xsrq_pingpong.c @@ -630,7 +630,11 @@ static int pp_server_connect(int port) return 1; } - listen(sockfd, ctx.num_clients); + if (listen(sockfd, ctx.num_clients)) { + perror("listen() failed"); + close(sockfd); + return 1; + } for (i = 0; i < ctx.num_clients; i++) { connfd = accept(sockfd, NULL, NULL); diff --git a/contrib/ofed/libmlx5/bitmap.h b/contrib/ofed/libmlx5/bitmap.h index 4b1869053e84..f7e50375ab36 100644 --- a/contrib/ofed/libmlx5/bitmap.h +++ b/contrib/ofed/libmlx5/bitmap.h @@ -95,17 +95,17 @@ static inline uint32_t mlx5_find_first_zero_bit(const unsigned long *addr, static inline void mlx5_set_bit(unsigned int nr, unsigned long *addr) { - addr[(nr / BITS_PER_LONG)] |= (1 << (nr % BITS_PER_LONG)); + addr[(nr / BITS_PER_LONG)] |= (1UL << (nr % BITS_PER_LONG)); } -static inline void mlx5_clear_bit(unsigned int nr, unsigned long *addr) +static inline void mlx5_clear_bit(unsigned int nr, unsigned long *addr) { - addr[(nr / BITS_PER_LONG)] &= ~(1 << (nr % BITS_PER_LONG)); + addr[(nr / BITS_PER_LONG)] &= ~(1UL << (nr % BITS_PER_LONG)); } static inline int mlx5_test_bit(unsigned int nr, const unsigned long *addr) { - return !!(addr[(nr / BITS_PER_LONG)] & (1 << (nr % BITS_PER_LONG))); + return !!(addr[(nr / BITS_PER_LONG)] & (1UL << (nr % BITS_PER_LONG))); } #endif diff --git a/contrib/ofed/librdmacm/examples/mckey.c b/contrib/ofed/librdmacm/examples/mckey.c index b39a2e0c9371..65c80d4efc7f 100644 --- a/contrib/ofed/librdmacm/examples/mckey.c +++ b/contrib/ofed/librdmacm/examples/mckey.c @@ -469,8 +469,7 @@ static int get_dst_addr(char *dst, struct sockaddr *addr) sib = (struct sockaddr_ib *) addr; memset(sib, 0, sizeof *sib); sib->sib_family = AF_IB; - inet_pton(AF_INET6, dst, &sib->sib_addr); - return 0; + return inet_pton(AF_INET6, dst, &sib->sib_addr) != 1; } static int run(void) diff --git a/contrib/ofed/opensm/opensm/osm_perfmgr.c b/contrib/ofed/opensm/opensm/osm_perfmgr.c index 3116cb3a49b8..6cd8bb7cfcff 100644 --- a/contrib/ofed/opensm/opensm/osm_perfmgr.c +++ b/contrib/ofed/opensm/opensm/osm_perfmgr.c @@ -1935,7 +1935,9 @@ ib_api_status_t osm_perfmgr_init(osm_perfmgr_t * pm, osm_opensm_t * osm, pm->state = p_opt->perfmgr ? PERFMGR_STATE_ENABLED : PERFMGR_STATE_DISABLE; pm->sweep_state = PERFMGR_SWEEP_SLEEP; - cl_spinlock_init(&pm->lock); + status = cl_spinlock_init(&pm->lock); + if (status != IB_SUCCESS) + goto Exit; pm->sweep_time_s = p_opt->perfmgr_sweep_time_s; pm->max_outstanding_queries = p_opt->perfmgr_max_outstanding_queries; pm->ignore_cas = p_opt->perfmgr_ignore_cas; diff --git a/contrib/ofed/opensm/opensm/osm_port.c b/contrib/ofed/opensm/opensm/osm_port.c index 35010e31bbfd..47b9e920d29e 100644 --- a/contrib/ofed/opensm/opensm/osm_port.c +++ b/contrib/ofed/opensm/opensm/osm_port.c @@ -161,8 +161,10 @@ osm_port_t *osm_port_new(IN const ib_node_info_t * p_ni, only the singular part that has this GUID is owned. */ p_physp = osm_node_get_physp_ptr(p_parent_node, port_num); - if (!p_physp) + if (!p_physp) { + osm_port_delete(&p_port); return NULL; + } CL_ASSERT(port_guid == osm_physp_get_port_guid(p_physp)); p_port->p_physp = p_physp; diff --git a/contrib/ofed/opensm/opensm/osm_sa_mad_ctrl.c b/contrib/ofed/opensm/opensm/osm_sa_mad_ctrl.c index dbab4a95ecf2..2df6f18237ff 100644 --- a/contrib/ofed/opensm/opensm/osm_sa_mad_ctrl.c +++ b/contrib/ofed/opensm/opensm/osm_sa_mad_ctrl.c @@ -373,6 +373,7 @@ static void sa_mad_ctrl_rcv_callback(IN osm_madw_t * p_madw, IN void *context, case IB_MAD_METHOD_GETMULTI: #endif is_get_request = TRUE; + /* FALLTHROUGH */ case IB_MAD_METHOD_SET: case IB_MAD_METHOD_DELETE: /* if we are closing down simply do nothing */