[pulseaudio-discuss] [PATCH 1/2] shm: Fix use of uninitialized value: segment's shared-memory type

[Ahmed S. Darwish]

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 at gmail.com>
Signed-off-by: Ahmed S. Darwish <darwish.07 at gmail.com>
---
 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 bcf7182..9dea0a5 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);
-- 
2.8.3