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 <patrakov@gmail.com>
Signed-off-by: Ahmed S. Darwish <darwish.07@gmail.com>
Signed-off-by: Arun Raghavan <arun@arunraghavan.net>
This commit is contained in:
Ahmed S. Darwish 2016-06-17 21:54:54 +02:00 committed by Arun Raghavan
parent f58e8c405c
commit 3922bbe7eb

View file

@ -93,8 +93,8 @@ struct shm_marker {
uint64_t _reserved4; uint64_t _reserved4;
} PA_GCC_PACKED; } PA_GCC_PACKED;
static inline size_t shm_marker_size(pa_shm *m) { static inline size_t shm_marker_size(pa_mem_type_t type) {
if (m->type == PA_MEM_TYPE_SHARED_POSIX) if (type == PA_MEM_TYPE_SHARED_POSIX)
return PA_ALIGN(sizeof(struct shm_marker)); return PA_ALIGN(sizeof(struct shm_marker));
return 0; 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->type = type;
m->size = size + shm_marker_size(m); m->size = size + shm_marker_size(type);
m->do_unlink = do_unlink; m->do_unlink = do_unlink;
if (ftruncate(fd, (off_t) m->size) < 0) { 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) { if (type == PA_MEM_TYPE_SHARED_POSIX) {
/* We store our PID at the end of the shm block, so that we /* We store our PID at the end of the shm block, so that we
* can check for dead shm segments later */ * 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->pid, (int) getpid());
pa_atomic_store(&marker->marker, SHM_MARKER); 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 || 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_ALIGN((size_t) st.st_size) != (size_t) st.st_size) {
pa_log("Invalid shared memory segment size"); pa_log("Invalid shared memory segment size");
goto fail; 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) if (shm_attach(&seg, PA_MEM_TYPE_SHARED_POSIX, id, -1, false, true) < 0)
continue; continue;
if (seg.size < shm_marker_size(&seg)) { if (seg.size < shm_marker_size(seg.type)) {
pa_shm_free(&seg); pa_shm_free(&seg);
continue; 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) { if (pa_atomic_load(&m->marker) != SHM_MARKER) {
pa_shm_free(&seg); pa_shm_free(&seg);