From 35aefc9e84c11461b16648f237d9a83c0c381edd Mon Sep 17 00:00:00 2001 From: Pawel Wodkowski Date: Wed, 28 Mar 2018 20:07:54 +0200 Subject: [PATCH] app,lib: fix checking mmap return value Some places use NULL to check for mmap failure but mmap return MAP_FAILED in this case. Change-Id: I4796fa52421da53c94223a9e8cc26ac04968f1d8 Signed-off-by: Pawel Wodkowski Reviewed-on: https://review.gerrithub.io/405648 Reviewed-by: Dariusz Stojaczyk Reviewed-by: Daniel Verkamp Reviewed-by: Shuhei Matsumoto Tested-by: SPDK Automated Test System --- app/iscsi_top/iscsi_top.cpp | 6 +++--- lib/env_dpdk/pci.c | 4 ++-- lib/iscsi/conn.c | 33 ++++++++++++++++++++++++--------- lib/trace/trace.c | 2 +- 4 files changed, 30 insertions(+), 15 deletions(-) diff --git a/app/iscsi_top/iscsi_top.cpp b/app/iscsi_top/iscsi_top.cpp index b861959c5..50449b596 100644 --- a/app/iscsi_top/iscsi_top.cpp +++ b/app/iscsi_top/iscsi_top.cpp @@ -93,8 +93,8 @@ print_connections(void) conns_size = sizeof(*conns) * MAX_ISCSI_CONNECTIONS; conns_ptr = mmap(NULL, conns_size, PROT_READ, MAP_SHARED, fd, 0); - if (conns_ptr == NULL) { - fprintf(stderr, "Cannot mmap shared memory\n"); + if (conns_ptr == MAP_FAILED) { + fprintf(stderr, "Cannot mmap shared memory (%d)\n", errno); exit(1); } @@ -160,7 +160,7 @@ int main(int argc, char **argv) history_ptr = mmap(NULL, sizeof(*histories), PROT_READ, MAP_SHARED, history_fd, 0); if (history_ptr == MAP_FAILED) { - fprintf(stderr, "Unable to mmap history shm\n"); + fprintf(stderr, "Unable to mmap history shm (%d).\n", errno); exit(1); } diff --git a/lib/env_dpdk/pci.c b/lib/env_dpdk/pci.c index 26cb0cc16..1dc1d9edc 100644 --- a/lib/env_dpdk/pci.c +++ b/lib/env_dpdk/pci.c @@ -480,8 +480,8 @@ spdk_pci_device_claim(const struct spdk_pci_addr *pci_addr) dev_map = mmap(NULL, sizeof(int), PROT_READ | PROT_WRITE, MAP_SHARED, dev_fd, 0); - if (dev_map == NULL) { - fprintf(stderr, "could not mmap dev %s\n", dev_name); + if (dev_map == MAP_FAILED) { + fprintf(stderr, "could not mmap dev %s (%d)\n", dev_name, errno); close(dev_fd); return -1; } diff --git a/lib/iscsi/conn.c b/lib/iscsi/conn.c index 7f12a06a4..a3572981f 100644 --- a/lib/iscsi/conn.c +++ b/lib/iscsi/conn.c @@ -60,7 +60,7 @@ static int g_connections_per_lcore; static uint32_t *g_num_connections; -struct spdk_iscsi_conn *g_conns_array; +struct spdk_iscsi_conn *g_conns_array = MAP_FAILED; static char g_shm_name[64]; static pthread_mutex_t g_conns_mutex; @@ -118,7 +118,7 @@ spdk_find_iscsi_connection_by_id(int cid) int spdk_initialize_iscsi_conns(void) { - size_t conns_size; + size_t conns_size = sizeof(struct spdk_iscsi_conn) * MAX_ISCSI_CONNECTIONS; int conns_array_fd, rc; uint32_t i, last_core; @@ -134,20 +134,21 @@ int spdk_initialize_iscsi_conns(void) conns_array_fd = shm_open(g_shm_name, O_RDWR | O_CREAT, 0600); if (conns_array_fd < 0) { SPDK_ERRLOG("could not shm_open %s\n", g_shm_name); - return -1; + goto err; } - conns_size = sizeof(struct spdk_iscsi_conn) * MAX_ISCSI_CONNECTIONS; - if (ftruncate(conns_array_fd, conns_size) != 0) { SPDK_ERRLOG("could not ftruncate\n"); - close(conns_array_fd); - shm_unlink(g_shm_name); - return -1; + goto err; } g_conns_array = mmap(0, conns_size, PROT_READ | PROT_WRITE, MAP_SHARED, conns_array_fd, 0); + if (g_conns_array == MAP_FAILED) { + fprintf(stderr, "could not mmap cons array file %s (%d)\n", g_shm_name, errno); + goto err; + } + memset(g_conns_array, 0, conns_size); for (i = 0; i < MAX_ISCSI_CONNECTIONS; i++) { @@ -159,10 +160,24 @@ int spdk_initialize_iscsi_conns(void) if (!g_num_connections) { SPDK_ERRLOG("Could not allocate array size=%u for g_num_connections\n", last_core + 1); - return -1; + goto err; } return 0; + +err: + if (g_conns_array != MAP_FAILED) { + munmap(g_conns_array, conns_size); + g_conns_array = MAP_FAILED; + } + + if (conns_array_fd >= 0) { + close(conns_array_fd); + shm_unlink(g_shm_name); + } + + pthread_mutex_destroy(&g_conns_mutex); + return -1; } static void diff --git a/lib/trace/trace.c b/lib/trace/trace.c index 7c0d63f27..fe180032d 100644 --- a/lib/trace/trace.c +++ b/lib/trace/trace.c @@ -106,7 +106,7 @@ spdk_trace_init(const char *shm_name) g_trace_histories = mmap(NULL, sizeof(*g_trace_histories), PROT_READ | PROT_WRITE, MAP_SHARED, g_trace_fd, 0); - if (g_trace_histories == NULL) { + if (g_trace_histories == MAP_FAILED) { fprintf(stderr, "could not mmap shm\n"); goto trace_init_err; }