From 3922bbe7eb94232cc097bc3b7a91f06b2db93df2 Mon Sep 17 00:00:00 2001 From: "Ahmed S. Darwish" Date: Fri, 17 Jun 2016 21:54:54 +0200 Subject: shm: Fix use of uninitialized value: segment's shared-memory type As shown by valgrind ==10615== Conditional jump or move depends on uninitialised value(s) ==10615== at 0x5CC0483: shm_marker_size (shm.c:97) ==10615== by 0x5CC1685: shm_attach (shm.c:381) ==10615== by 0x5CC1990: pa_shm_cleanup (shm.c:453) ==10615== by 0x5CC068E: sharedmem_create (shm.c:150) ... Solution is to fix the shm_marker_size() signature itself: At certain code paths like shm_attach(), we don't want to initialize _any_ field in the passed SHM segment descriptor except after making sure all error exit conditions have been passed. Reported-by: Alexander E. Patrakov Signed-off-by: Ahmed S. Darwish Signed-off-by: Arun Raghavan --- src/pulsecore/shm.c | 14 +++++++------- 1 file changed, 7 insertions(+), 7 deletions(-) diff --git a/src/pulsecore/shm.c b/src/pulsecore/shm.c index bcf718224..9dea0a52a 100644 --- a/src/pulsecore/shm.c +++ b/src/pulsecore/shm.c @@ -93,8 +93,8 @@ struct shm_marker { uint64_t _reserved4; } PA_GCC_PACKED; -static inline size_t shm_marker_size(pa_shm *m) { - if (m->type == PA_MEM_TYPE_SHARED_POSIX) +static inline size_t shm_marker_size(pa_mem_type_t type) { + if (type == PA_MEM_TYPE_SHARED_POSIX) return PA_ALIGN(sizeof(struct shm_marker)); return 0; @@ -174,7 +174,7 @@ static int sharedmem_create(pa_shm *m, pa_mem_type_t type, size_t size, mode_t m } m->type = type; - m->size = size + shm_marker_size(m); + m->size = size + shm_marker_size(type); m->do_unlink = do_unlink; if (ftruncate(fd, (off_t) m->size) < 0) { @@ -194,7 +194,7 @@ static int sharedmem_create(pa_shm *m, pa_mem_type_t type, size_t size, mode_t m if (type == PA_MEM_TYPE_SHARED_POSIX) { /* We store our PID at the end of the shm block, so that we * can check for dead shm segments later */ - marker = (struct shm_marker*) ((uint8_t*) m->ptr + m->size - shm_marker_size(m)); + marker = (struct shm_marker*) ((uint8_t*) m->ptr + m->size - shm_marker_size(type)); pa_atomic_store(&marker->pid, (int) getpid()); pa_atomic_store(&marker->marker, SHM_MARKER); } @@ -378,7 +378,7 @@ static int shm_attach(pa_shm *m, pa_mem_type_t type, unsigned id, int memfd_fd, } if (st.st_size <= 0 || - st.st_size > (off_t) MAX_SHM_SIZE + (off_t) shm_marker_size(m) || + st.st_size > (off_t) MAX_SHM_SIZE + (off_t) shm_marker_size(type) || PA_ALIGN((size_t) st.st_size) != (size_t) st.st_size) { pa_log("Invalid shared memory segment size"); goto fail; @@ -453,12 +453,12 @@ int pa_shm_cleanup(void) { if (shm_attach(&seg, PA_MEM_TYPE_SHARED_POSIX, id, -1, false, true) < 0) continue; - if (seg.size < shm_marker_size(&seg)) { + if (seg.size < shm_marker_size(seg.type)) { pa_shm_free(&seg); continue; } - m = (struct shm_marker*) ((uint8_t*) seg.ptr + seg.size - shm_marker_size(&seg)); + m = (struct shm_marker*) ((uint8_t*) seg.ptr + seg.size - shm_marker_size(seg.type)); if (pa_atomic_load(&m->marker) != SHM_MARKER) { pa_shm_free(&seg); -- cgit v1.2.1