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
This commit is contained in:
Eric van Gyzen 2020-07-15 13:17:16 +00:00
parent ea6c594cbc
commit d13def78cc
Notes: svn2git 2020-12-20 02:59:44 +00:00
svn path=/head/; revision=363220
18 changed files with 59 additions and 23 deletions

View file

@ -120,6 +120,7 @@ static inline int val_str_true(const char *val_str)
void read_ibdiag_config(const char *file) void read_ibdiag_config(const char *file)
{ {
char buf[1024]; char buf[1024];
char orig_buf[1024];
FILE *config_fd = NULL; FILE *config_fd = NULL;
char *p_prefix, *p_last; char *p_prefix, *p_last;
char *name; char *name;
@ -142,8 +143,14 @@ void read_ibdiag_config(const char *file)
if (*p_prefix == '#') if (*p_prefix == '#')
continue; /* ignore comment lines */ continue; /* ignore comment lines */
strlcpy(orig_buf, buf, sizeof(orig_buf));
name = strtok_r(p_prefix, "=", &p_last); name = strtok_r(p_prefix, "=", &p_last);
val_str = strtok_r(NULL, "\n", &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) { if (strncmp(name, "CA", strlen("CA")) == 0) {
free(ibd_ca); free(ibd_ca);
@ -165,6 +172,7 @@ void read_ibdiag_config(const char *file)
ibd_sakey = strtoull(val_str, 0, 0); ibd_sakey = strtoull(val_str, 0, 0);
} else if (strncmp(name, "nd_format", } else if (strncmp(name, "nd_format",
strlen("nd_format")) == 0) { strlen("nd_format")) == 0) {
free(ibd_nd_format);
ibd_nd_format = strdup(val_str); ibd_nd_format = strdup(val_str);
} }
} }

View file

@ -222,7 +222,7 @@ static const char *ib_mad_inv_field_str[] = {
"MAD Reserved", "MAD Reserved",
"MAD Reserved", "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" "MAD UNKNOWN ERROR"
}; };
#define MAD_ERR_UNKNOWN (ARR_SIZE(ib_mad_inv_field_str) - 1) #define MAD_ERR_UNKNOWN (ARR_SIZE(ib_mad_inv_field_str) - 1)

View file

@ -293,7 +293,8 @@ void print_node_header(ibnd_node_t *node, int *out_header_flag,
printf("%s%s: %s:\n", printf("%s%s: %s:\n",
out_prefix ? out_prefix : "", out_prefix ? out_prefix : "",
nodetype_str(node), remap); nodetype_str(node), remap);
(*out_header_flag)++; if (out_header_flag)
(*out_header_flag)++;
free(remap); 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) { if (output_diff && fabric2_port) {
print_node_header(fabric1_node, print_node_header(fabric2_node,
head_print, head_print,
NULL); NULL);
print_port(fabric2_node, print_port(fabric2_node,

View file

@ -564,6 +564,7 @@ int main(int argc, char **argv)
printf("Port is already in enable state\n"); printf("Port is already in enable state\n");
goto close_port; goto close_port;
} }
/* FALLTHROUGH */
case ENABLE: case ENABLE:
case RESET: case RESET:
/* Polling */ /* Polling */

View file

@ -130,6 +130,7 @@ static void set_thres(char *name, uint32_t val)
static void set_thresholds(char *threshold_file) static void set_thresholds(char *threshold_file)
{ {
char buf[1024]; char buf[1024];
char orig_buf[1024];
int val = 0; int val = 0;
FILE *thresf = fopen(threshold_file, "r"); FILE *thresf = fopen(threshold_file, "r");
char *p_prefix, *p_last; char *p_prefix, *p_last;
@ -156,8 +157,14 @@ static void set_thresholds(char *threshold_file)
if (*p_prefix == '#') if (*p_prefix == '#')
continue; /* ignore comment lines */ continue; /* ignore comment lines */
strlcpy(orig_buf, buf, sizeof(orig_buf));
name = strtok_r(p_prefix, "=", &p_last); name = strtok_r(p_prefix, "=", &p_last);
val_str = strtok_r(NULL, "\n", &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); val = strtoul(val_str, NULL, 0);
set_thres(name, val); set_thres(name, val);

View file

@ -354,6 +354,8 @@ char *dump_unicast_tables(ib_portid_t * portid, int startlid, int endlid)
" (%s):\n", startlid, endlid, portid2str(portid), nodeguid, " (%s):\n", startlid, endlid, portid2str(portid), nodeguid,
mapnd); mapnd);
free(mapnd);
DEBUG("Switch top is 0x%x\n", top); DEBUG("Switch top is 0x%x\n", top);
printf(" Lid Out Destination\n"); 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 "); printf("%d %slids dumped \n", n, dump_all ? "" : "valid ");
free(mapnd);
return 0; return 0;
} }

View file

@ -246,7 +246,6 @@ static const char * umad_sm_attr_str(__be16 attr_id)
default: default:
return (umad_common_attr_str(attr_id)); return (umad_common_attr_str(attr_id));
} }
return ("<unknown>");
} }
static const char * umad_sa_attr_str(__be16 attr_id) 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: default:
return (umad_common_attr_str(attr_id)); return (umad_common_attr_str(attr_id));
} }
return ("<unknown>");
} }
static const char * umad_cm_attr_str(__be16 attr_id) 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: default:
return (umad_common_attr_str(attr_id)); return (umad_common_attr_str(attr_id));
} }
return ("<unknown>");
} }
const char * umad_attribute_str(uint8_t mgmt_class, __be16 attr_id) const char * umad_attribute_str(uint8_t mgmt_class, __be16 attr_id)

View file

@ -264,7 +264,6 @@ int __ibv_close_device(struct ibv_context *context)
{ {
int async_fd = context->async_fd; int async_fd = context->async_fd;
int cmd_fd = context->cmd_fd; int cmd_fd = context->cmd_fd;
int cq_fd = -1;
struct verbs_context *context_ex; struct verbs_context *context_ex;
struct verbs_device *verbs_device = verbs_get_device(context->device); 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(async_fd);
close(cmd_fd); close(cmd_fd);
if (abi_ver <= 2)
close(cq_fd);
return 0; return 0;
} }

View file

@ -273,7 +273,11 @@ static struct pingpong_dest *pp_server_exch_dest(struct pingpong_context *ctx,
return NULL; return NULL;
} }
listen(sockfd, 1); if (listen(sockfd, 1)) {
perror("listen() failed");
close(sockfd);
return NULL;
}
connfd = accept(sockfd, NULL, NULL); connfd = accept(sockfd, NULL, NULL);
close(sockfd); close(sockfd);
if (connfd < 0) { if (connfd < 0) {

View file

@ -283,7 +283,11 @@ static struct pingpong_dest *pp_server_exch_dest(struct pingpong_context *ctx,
return NULL; return NULL;
} }
listen(sockfd, 1); if (listen(sockfd, 1)) {
perror("listen() failed");
close(sockfd);
return NULL;
}
connfd = accept(sockfd, NULL, NULL); connfd = accept(sockfd, NULL, NULL);
close(sockfd); close(sockfd);
if (connfd < 0) { if (connfd < 0) {

View file

@ -247,7 +247,11 @@ static struct pingpong_dest *pp_server_exch_dest(struct pingpong_context *ctx,
return NULL; return NULL;
} }
listen(sockfd, 1); if (listen(sockfd, 1)) {
perror("listen() failed");
close(sockfd);
return NULL;
}
connfd = accept(sockfd, NULL, NULL); connfd = accept(sockfd, NULL, NULL);
close(sockfd); close(sockfd);
if (connfd < 0) { if (connfd < 0) {

View file

@ -245,7 +245,11 @@ static struct pingpong_dest *pp_server_exch_dest(struct pingpong_context *ctx,
return NULL; return NULL;
} }
listen(sockfd, 1); if (listen(sockfd, 1)) {
perror("listen() failed");
close(sockfd);
return NULL;
}
connfd = accept(sockfd, NULL, NULL); connfd = accept(sockfd, NULL, NULL);
close(sockfd); close(sockfd);
if (connfd < 0) { if (connfd < 0) {

View file

@ -630,7 +630,11 @@ static int pp_server_connect(int port)
return 1; 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++) { for (i = 0; i < ctx.num_clients; i++) {
connfd = accept(sockfd, NULL, NULL); connfd = accept(sockfd, NULL, NULL);

View file

@ -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) 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) 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 #endif

View file

@ -469,8 +469,7 @@ static int get_dst_addr(char *dst, struct sockaddr *addr)
sib = (struct sockaddr_ib *) addr; sib = (struct sockaddr_ib *) addr;
memset(sib, 0, sizeof *sib); memset(sib, 0, sizeof *sib);
sib->sib_family = AF_IB; sib->sib_family = AF_IB;
inet_pton(AF_INET6, dst, &sib->sib_addr); return inet_pton(AF_INET6, dst, &sib->sib_addr) != 1;
return 0;
} }
static int run(void) static int run(void)

View file

@ -1935,7 +1935,9 @@ ib_api_status_t osm_perfmgr_init(osm_perfmgr_t * pm, osm_opensm_t * osm,
pm->state = pm->state =
p_opt->perfmgr ? PERFMGR_STATE_ENABLED : PERFMGR_STATE_DISABLE; p_opt->perfmgr ? PERFMGR_STATE_ENABLED : PERFMGR_STATE_DISABLE;
pm->sweep_state = PERFMGR_SWEEP_SLEEP; 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->sweep_time_s = p_opt->perfmgr_sweep_time_s;
pm->max_outstanding_queries = p_opt->perfmgr_max_outstanding_queries; pm->max_outstanding_queries = p_opt->perfmgr_max_outstanding_queries;
pm->ignore_cas = p_opt->perfmgr_ignore_cas; pm->ignore_cas = p_opt->perfmgr_ignore_cas;

View file

@ -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. only the singular part that has this GUID is owned.
*/ */
p_physp = osm_node_get_physp_ptr(p_parent_node, port_num); 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; return NULL;
}
CL_ASSERT(port_guid == osm_physp_get_port_guid(p_physp)); CL_ASSERT(port_guid == osm_physp_get_port_guid(p_physp));
p_port->p_physp = p_physp; p_port->p_physp = p_physp;

View file

@ -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: case IB_MAD_METHOD_GETMULTI:
#endif #endif
is_get_request = TRUE; is_get_request = TRUE;
/* FALLTHROUGH */
case IB_MAD_METHOD_SET: case IB_MAD_METHOD_SET:
case IB_MAD_METHOD_DELETE: case IB_MAD_METHOD_DELETE:
/* if we are closing down simply do nothing */ /* if we are closing down simply do nothing */