From 8ed89704969af8ce7758224fffc73b74ee0ab470 Mon Sep 17 00:00:00 2001 From: Mathieu Desnoyers Date: Mon, 21 Sep 2015 13:43:08 -0400 Subject: [PATCH] Fix: sysconf() unchecked return value Fix Coverity bug: CID 1021259 (#1 of 1): Improper use of negative value (NEGATIVE_RETURNS)5. negative_returns: sysconf(_SC_PAGESIZE) is passed to a parameter that cannot be negative. Signed-off-by: Mathieu Desnoyers --- liblttng-ust/lttng-ust-comm.c | 13 ++++++++++-- libringbuffer/ring_buffer_backend.c | 23 ++++++++++++++++----- tests/ust-basic-tracing/ust-basic-tracing.c | 20 +++++++++++++++--- tests/ust-multi-test/ust-multi-test.c | 20 +++++++++++++++--- 4 files changed, 63 insertions(+), 13 deletions(-) diff --git a/liblttng-ust/lttng-ust-comm.c b/liblttng-ust/lttng-ust-comm.c index fbdee7d3..2712a8d8 100644 --- a/liblttng-ust/lttng-ust-comm.c +++ b/liblttng-ust/lttng-ust-comm.c @@ -927,7 +927,12 @@ void cleanup_sock_info(struct sock_info *sock_info, int exiting) long page_size; page_size = sysconf(_SC_PAGE_SIZE); - if (page_size > 0) { + if (page_size <= 0) { + if (!page_size) { + errno = EINVAL; + } + PERROR("Error in sysconf(_SC_PAGE_SIZE)"); + } else { ret = munmap(sock_info->wait_shm_mmap, page_size); if (ret) { ERR("Error unmapping wait shm"); @@ -1109,7 +1114,11 @@ char *get_map_shm(struct sock_info *sock_info) char *wait_shm_mmap; page_size = sysconf(_SC_PAGE_SIZE); - if (page_size < 0) { + if (page_size <= 0) { + if (!page_size) { + errno = EINVAL; + } + PERROR("Error in sysconf(_SC_PAGE_SIZE)"); goto error; } diff --git a/libringbuffer/ring_buffer_backend.c b/libringbuffer/ring_buffer_backend.c index 7d3a3780..6d2d9507 100644 --- a/libringbuffer/ring_buffer_backend.c +++ b/libringbuffer/ring_buffer_backend.c @@ -19,6 +19,7 @@ */ #define _GNU_SOURCE +#include #include #include @@ -49,6 +50,7 @@ int lib_ring_buffer_backend_allocate(const struct lttng_ust_lib_ring_buffer_conf unsigned long subbuf_size, mmap_offset = 0; unsigned long num_subbuf_alloc; unsigned long i; + long page_size; subbuf_size = chanb->subbuf_size; num_subbuf_alloc = num_subbuf; @@ -56,6 +58,11 @@ int lib_ring_buffer_backend_allocate(const struct lttng_ust_lib_ring_buffer_conf if (extra_reader_sb) num_subbuf_alloc++; + page_size = sysconf(_SC_PAGE_SIZE); + if (page_size <= 0) { + goto page_size_error; + } + align_shm(shmobj, __alignof__(struct lttng_ust_lib_ring_buffer_backend_pages_shmp)); set_shmp(bufb->array, zalloc_shm(shmobj, sizeof(struct lttng_ust_lib_ring_buffer_backend_pages_shmp) * num_subbuf_alloc)); @@ -64,9 +71,9 @@ int lib_ring_buffer_backend_allocate(const struct lttng_ust_lib_ring_buffer_conf /* * This is the largest element (the buffer pages) which needs to - * be aligned on PAGE_SIZE. + * be aligned on page size. */ - align_shm(shmobj, PAGE_SIZE); + align_shm(shmobj, page_size); set_shmp(bufb->memory_map, zalloc_shm(shmobj, subbuf_size * num_subbuf_alloc)); if (caa_unlikely(!shmp(handle, bufb->memory_map))) @@ -123,6 +130,7 @@ free_array: memory_map_error: /* bufb->array will be freed by shm teardown */ array_error: +page_size_error: return -ENOMEM; } @@ -196,7 +204,7 @@ void channel_backend_reset(struct channel_backend *chanb) * @name: channel name * @config: client ring buffer configuration * @parent: dentry of parent directory, %NULL for root directory - * @subbuf_size: size of sub-buffers (> PAGE_SIZE, power of 2) + * @subbuf_size: size of sub-buffers (> page size, power of 2) * @num_subbuf: number of sub-buffers (power of 2) * @lttng_ust_shm_handle: shared memory handle * @@ -218,12 +226,17 @@ int channel_backend_init(struct channel_backend *chanb, unsigned int i; int ret; size_t shmsize = 0, num_subbuf_alloc; + long page_size; if (!name) return -EPERM; + page_size = sysconf(_SC_PAGE_SIZE); + if (page_size <= 0) { + return -ENOMEM; + } /* Check that the subbuffer size is larger than a page. */ - if (subbuf_size < PAGE_SIZE) + if (subbuf_size < page_size) return -EINVAL; /* @@ -266,7 +279,7 @@ int channel_backend_init(struct channel_backend *chanb, num_subbuf_alloc = num_subbuf + 1; shmsize += offset_align(shmsize, __alignof__(struct lttng_ust_lib_ring_buffer_backend_pages_shmp)); shmsize += sizeof(struct lttng_ust_lib_ring_buffer_backend_pages_shmp) * num_subbuf_alloc; - shmsize += offset_align(shmsize, PAGE_SIZE); + shmsize += offset_align(shmsize, page_size); shmsize += subbuf_size * num_subbuf_alloc; shmsize += offset_align(shmsize, __alignof__(struct lttng_ust_lib_ring_buffer_backend_pages)); shmsize += sizeof(struct lttng_ust_lib_ring_buffer_backend_pages) * num_subbuf_alloc; diff --git a/tests/ust-basic-tracing/ust-basic-tracing.c b/tests/ust-basic-tracing/ust-basic-tracing.c index cc0fb89b..15368ed7 100644 --- a/tests/ust-basic-tracing/ust-basic-tracing.c +++ b/tests/ust-basic-tracing/ust-basic-tracing.c @@ -647,7 +647,11 @@ int update_futex(int fd, int active) int ret; page_size = sysconf(_SC_PAGE_SIZE); - if (page_size < 0) { + if (page_size <= 0) { + if (!page_size) { + errno = EINVAL; + } + perror("Error in sysconf(_SC_PAGE_SIZE)"); goto error; } wait_shm_mmap = mmap(NULL, page_size, PROT_READ | PROT_WRITE, @@ -717,6 +721,7 @@ int main(int argc, const char **argv) const char *outputpath; const char **event_names; unsigned int nr_events; + long page_size; if (argc < 2) { printf("Usage:\n"); @@ -759,6 +764,15 @@ int main(int argc, const char **argv) return -1; } + page_size = sysconf(_SC_PAGE_SIZE); + if (page_size <= 0) { + if (!page_size) { + errno = EINVAL; + } + perror("Error in sysconf(_SC_PAGE_SIZE)"); + return -1; + } + if (geteuid() == 0) { ret = mkdir(LTTNG_RUNDIR, S_IRWXU | S_IRWXG | S_IROTH | S_IXOTH); if (ret && errno != EEXIST) { @@ -766,7 +780,7 @@ int main(int argc, const char **argv) return -1; } wait_shm_fd = get_wait_shm(DEFAULT_GLOBAL_APPS_WAIT_SHM_PATH, - sysconf(_SC_PAGE_SIZE), 1); + page_size, 1); if (wait_shm_fd < 0) { perror("global wait shm error"); return -1; @@ -792,7 +806,7 @@ int main(int argc, const char **argv) snprintf(local_apps_wait_shm_path, PATH_MAX, DEFAULT_HOME_APPS_WAIT_SHM_PATH, getuid()); wait_shm_fd = get_wait_shm(local_apps_wait_shm_path, - sysconf(_SC_PAGE_SIZE), 0); + page_size, 0); if (wait_shm_fd < 0) { perror("local wait shm error"); return -1; diff --git a/tests/ust-multi-test/ust-multi-test.c b/tests/ust-multi-test/ust-multi-test.c index c003c5a9..01137439 100644 --- a/tests/ust-multi-test/ust-multi-test.c +++ b/tests/ust-multi-test/ust-multi-test.c @@ -643,7 +643,11 @@ int update_futex(int fd, int active) int ret; page_size = sysconf(_SC_PAGE_SIZE); - if (page_size < 0) { + if (page_size <= 0) { + if (!page_size) { + errno = EINVAL; + } + perror("Error in sysconf(_SC_PAGE_SIZE)"); goto error; } wait_shm_mmap = mmap(NULL, page_size, PROT_READ | PROT_WRITE, @@ -706,6 +710,7 @@ int main(int argc, char **argv) int ret, wait_shm_fd; struct sigaction act; mode_t old_umask = 0; + long page_size; set_ulimit(); @@ -738,6 +743,15 @@ int main(int argc, char **argv) return -1; } + page_size = sysconf(_SC_PAGE_SIZE); + if (page_size <= 0) { + if (!page_size) { + errno = EINVAL; + } + perror("Error in sysconf(_SC_PAGE_SIZE)"); + return -1; + } + if (geteuid() == 0) { ret = mkdir(LTTNG_RUNDIR, S_IRWXU | S_IRWXG | S_IROTH | S_IXOTH); if (ret && errno != EEXIST) { @@ -745,7 +759,7 @@ int main(int argc, char **argv) return -1; } wait_shm_fd = get_wait_shm(DEFAULT_GLOBAL_APPS_WAIT_SHM_PATH, - sysconf(_SC_PAGE_SIZE), 1); + page_size, 1); if (wait_shm_fd < 0) { perror("global wait shm error"); return -1; @@ -771,7 +785,7 @@ int main(int argc, char **argv) snprintf(local_apps_wait_shm_path, PATH_MAX, DEFAULT_HOME_APPS_WAIT_SHM_PATH, getuid()); wait_shm_fd = get_wait_shm(local_apps_wait_shm_path, - sysconf(_SC_PAGE_SIZE), 0); + page_size, 0); if (wait_shm_fd < 0) { perror("local wait shm error"); return -1; -- 2.34.1