From e9bc6169cefd68815107af30aab116ed4d1d58ae Mon Sep 17 00:00:00 2001 From: Elias Kozah Date: Tue, 27 Aug 2024 21:37:43 -0400 Subject: [PATCH] prov/opx: Resolve coverity scan defects uncovered after upstream Signed-off-by: Elias Kozah --- prov/opx/src/fi_opx_domain.c | 7 +++- prov/opx/src/fi_opx_rma.c | 55 +++++++++++++++++++++----------- prov/opx/src/fi_opx_tid_domain.c | 6 +++- 3 files changed, 47 insertions(+), 21 deletions(-) diff --git a/prov/opx/src/fi_opx_domain.c b/prov/opx/src/fi_opx_domain.c index 6a0f7cbd291..cffe97b40b6 100644 --- a/prov/opx/src/fi_opx_domain.c +++ b/prov/opx/src/fi_opx_domain.c @@ -492,7 +492,7 @@ int fi_opx_domain(struct fid_fabric *fabric, strncpy(opx_domain->unique_job_key_str, env_var_uuid, OPX_JOB_KEY_STR_SIZE-1); opx_domain->unique_job_key_str[OPX_JOB_KEY_STR_SIZE-1] = '\0'; - sscanf(opx_domain->unique_job_key_str, "%02hhx%02hhx%02hhx%02hhx%02hhx%02hhx%02hhx%02hhx%02hhx%02hhx-%02hhx%02hhx%02hhx%02hhx%02hhx%02hhx", + int elements_read = sscanf(opx_domain->unique_job_key_str, "%02hhx%02hhx%02hhx%02hhx%02hhx%02hhx%02hhx%02hhx%02hhx%02hhx%02hhx%02hhx%02hhx%02hhx%02hhx%02hhx", &opx_domain->unique_job_key[0], &opx_domain->unique_job_key[1], &opx_domain->unique_job_key[2], @@ -509,6 +509,11 @@ int fi_opx_domain(struct fid_fabric *fabric, &opx_domain->unique_job_key[13], &opx_domain->unique_job_key[14], &opx_domain->unique_job_key[15]); + if (elements_read == EOF) { + FI_WARN(fi_opx_global.prov, FI_LOG_DOMAIN, "Error: sscanf encountered an input failure (EOF), unable to parse the unique job key string.\n"); + errno = FI_EINVAL; + goto err; + } FI_INFO(fi_opx_global.prov, FI_LOG_DOMAIN, "Domain unique job key set to %s\n", opx_domain->unique_job_key_str); //TODO: Print out a summary of all domain settings wtih FI_INFO diff --git a/prov/opx/src/fi_opx_rma.c b/prov/opx/src/fi_opx_rma.c index a2355cef4b4..0dae5a3bbf0 100644 --- a/prov/opx/src/fi_opx_rma.c +++ b/prov/opx/src/fi_opx_rma.c @@ -469,9 +469,14 @@ ssize_t fi_opx_writev_internal(struct fid_ep *ep, const struct iovec *iov, void cc->hit_zero = fi_opx_hit_zero; struct fi_opx_mr **mr_ptr_array = (struct fi_opx_mr **)desc; - const uint64_t mr_ptr_present = (mr_ptr_array != NULL); - struct fi_opx_mr *mr_ptr = mr_ptr_present ? *mr_ptr_array : NULL; for (index = 0; index < count; ++index) { + struct fi_opx_mr *mr_ptr; + if (mr_ptr_array != NULL) { + mr_ptr = *mr_ptr_array; + ++mr_ptr_array; + } else { + mr_ptr = NULL; + } struct fi_opx_hmem_iov hmem_iov; const uint64_t is_hmem = fi_opx_hmem_iov_init(iov[index].iov_base, iov[index].iov_len, @@ -484,8 +489,6 @@ ssize_t fi_opx_writev_internal(struct fid_ep *ep, const struct iovec *iov, void lock_required, caps, reliability, hfi1_type); addr_offset += iov[index].iov_len; - ++mr_ptr_array; - mr_ptr = mr_ptr_present ? *mr_ptr_array : NULL; } return 0; @@ -603,12 +606,17 @@ ssize_t fi_opx_writemsg_internal(struct fid_ep *ep, const struct fi_msg_rma *msg uintptr_t msg_iov_vaddr = (uintptr_t)msg->msg_iov[msg_iov_index].iov_base; struct fi_opx_mr **mr_ptr_array = (struct fi_opx_mr **)msg->desc; - const uint64_t mr_ptr_present = (mr_ptr_array != NULL); - struct fi_opx_mr *mr_ptr = mr_ptr_present ? *mr_ptr_array : NULL; struct fi_opx_hmem_iov iov; - uint64_t is_hmem = fi_opx_hmem_iov_init((void *)msg_iov_vaddr, msg_iov_bytes, mr_ptr, &iov); while (msg_iov_bytes != 0 && rma_iov_bytes != 0) { + struct fi_opx_mr *mr_ptr; + if (mr_ptr_array != NULL) { + mr_ptr = *mr_ptr_array; + ++mr_ptr_array; + } else { + mr_ptr = NULL; + } + uint64_t is_hmem = fi_opx_hmem_iov_init((void *)msg_iov_vaddr, msg_iov_bytes, mr_ptr, &iov); size_t len = (msg_iov_bytes <= rma_iov_bytes) ? msg_iov_bytes : rma_iov_bytes; iov.buf = msg_iov_vaddr; iov.len = len; @@ -624,8 +632,6 @@ ssize_t fi_opx_writemsg_internal(struct fid_ep *ep, const struct fi_msg_rma *msg ++msg_iov_index; msg_iov_bytes = msg->msg_iov[msg_iov_index].iov_len; msg_iov_vaddr = (uintptr_t)msg->msg_iov[msg_iov_index].iov_base; - ++mr_ptr_array; - mr_ptr = mr_ptr_present ? *mr_ptr_array : NULL; is_hmem = fi_opx_hmem_iov_init((void *)msg_iov_vaddr, msg_iov_bytes, mr_ptr, &iov); } @@ -789,18 +795,21 @@ ssize_t fi_opx_readv(struct fid_ep *ep, const struct iovec *iov, void **desc, /* max 8 descriptors (iovecs) per readv_internal */ struct fi_opx_mr **mr_ptr_array = (struct fi_opx_mr **)desc; - const uint64_t mr_ptr_present = (mr_ptr_array != NULL); - struct fi_opx_mr *mr_ptr = mr_ptr_present ? *mr_ptr_array : NULL; const size_t full_count = count >> 3; for (index = 0; index < full_count; index += 8) { for (int i = 0; i < 8; ++i) { + struct fi_opx_mr *mr_ptr; + if (mr_ptr_array != NULL) { + mr_ptr = *mr_ptr_array; + ++mr_ptr_array; + } else { + mr_ptr = NULL; + } hmem_iface = fi_opx_hmem_get_iface(iov[index + i].iov_base, mr_ptr, &hmem_device); hmem_iovs[i].buf = (uintptr_t) iov[index + i].iov_base; hmem_iovs[i].len = iov[index + i].iov_len; hmem_iovs[i].iface = hmem_iface; hmem_iovs[i].device = hmem_device; - ++mr_ptr_array; - mr_ptr = mr_ptr_present ? *mr_ptr_array : NULL; } fi_opx_readv_internal(opx_ep, hmem_iovs, 8, opx_addr, addr_v, key_v, NULL, 0, NULL, NULL, cc, FI_VOID, FI_NOOP, @@ -811,13 +820,18 @@ ssize_t fi_opx_readv(struct fid_ep *ep, const struct iovec *iov, void **desc, /* if 'partial_ndesc' is zero, the fi_opx_readv_internal() will fence */ const size_t partial_ndesc = count & 0x07ull; for (int i = 0; i < partial_ndesc; ++i) { + struct fi_opx_mr *mr_ptr; + if (mr_ptr_array != NULL) { + mr_ptr = *mr_ptr_array; + ++mr_ptr_array; + } else { + mr_ptr = NULL; + } hmem_iface = fi_opx_hmem_get_iface(iov[index + i].iov_base, mr_ptr, &hmem_device); hmem_iovs[i].buf = (uintptr_t) iov[index + i].iov_base; hmem_iovs[i].len = iov[index + i].iov_len; hmem_iovs[i].iface = hmem_iface; hmem_iovs[i].device = hmem_device; - ++mr_ptr_array; - mr_ptr = mr_ptr_present ? *mr_ptr_array : NULL; } fi_opx_readv_internal(opx_ep, hmem_iovs, partial_ndesc, opx_addr, addr_v, key_v, opx_context, tx_op_flags, opx_ep->rx->cq, opx_ep->read_cntr, cc, @@ -920,10 +934,15 @@ ssize_t fi_opx_readmsg_internal(struct fid_ep *ep, const struct fi_msg_rma *msg, cc->hit_zero = fi_opx_hit_zero; struct fi_opx_mr **mr_ptr_array = (struct fi_opx_mr **)msg->desc; - const uint64_t mr_ptr_present = (mr_ptr_array != NULL); - struct fi_opx_mr *mr_ptr = mr_ptr_present ? *mr_ptr_array : NULL; while (src_iov_index < src_iov_count) { for (niov = 0; niov < 8; ++niov) { + struct fi_opx_mr *mr_ptr; + if (mr_ptr_array != NULL) { + mr_ptr = *mr_ptr_array; + ++mr_ptr_array; + } else { + mr_ptr = NULL; + } const size_t len = (dst_iov_bytes <= src_iov_bytes) ? dst_iov_bytes : src_iov_bytes; fi_opx_hmem_iov_init(dst_iov_vaddr, len, mr_ptr, &iov[niov]); @@ -987,8 +1006,6 @@ ssize_t fi_opx_readmsg_internal(struct fid_ep *ep, const struct fi_msg_rma *msg, ++dst_iov_index; dst_iov_bytes = msg->msg_iov[dst_iov_index].iov_len; dst_iov_vaddr = msg->msg_iov[dst_iov_index].iov_base; - ++mr_ptr_array; - mr_ptr = (mr_ptr_present) ? *mr_ptr_array : NULL; } } else { dst_iov_vaddr = (void *)((uintptr_t)dst_iov_vaddr + len); diff --git a/prov/opx/src/fi_opx_tid_domain.c b/prov/opx/src/fi_opx_tid_domain.c index 03024c82f50..946edbab2d2 100644 --- a/prov/opx/src/fi_opx_tid_domain.c +++ b/prov/opx/src/fi_opx_tid_domain.c @@ -149,7 +149,11 @@ int opx_close_tid_domain(struct opx_tid_domain *tid_domain, int locked) } dlist_remove(&tid_domain->list_entry); - ofi_domain_close(&tid_domain->util_domain); + int ret = ofi_domain_close(&tid_domain->util_domain); + if (ret != 0) { + FI_WARN(fi_opx_global.prov, FI_LOG_DOMAIN, "Error closing domain: %d\n", ret); + } + free(tid_domain); return 0;