[pulseaudio-commits] 3 commits - src/pulsecore

Arun Raghavan arun at kemper.freedesktop.org
Tue Jun 21 12:11:10 UTC 2016


 src/pulsecore/pstream.c       |    2 +-
 src/pulsecore/shm.c           |   14 +++++++-------
 src/pulsecore/sink-input.c    |    2 +-
 src/pulsecore/source-output.c |    2 +-
 4 files changed, 10 insertions(+), 10 deletions(-)

New commits:
commit effb3f1d234dc7cffb1be15051ff7194caa39f1a
Author: Arun Raghavan <arun at arunraghavan.net>
Date:   Tue Jun 21 17:19:39 2016 +0530

    sink-input,source-output: Fix crasher while setting property
    
    We were missing a case where a property is first set, and then cleared
    by setting a NULL value.
    
    Signed-off-by: Arun Raghavan <arun at arunraghavan.net>

diff --git a/src/pulsecore/sink-input.c b/src/pulsecore/sink-input.c
index 361b445..c7d99ef 100644
--- a/src/pulsecore/sink-input.c
+++ b/src/pulsecore/sink-input.c
@@ -1435,7 +1435,7 @@ void pa_sink_input_set_property(pa_sink_input *i, const char *key, const char *v
 
     if (pa_proplist_contains(i->proplist, key)) {
         old_value = pa_xstrdup(pa_proplist_gets(i->proplist, key));
-        if (old_value) {
+        if (value && old_value) {
             if (pa_streq(value, old_value))
                 goto finish;
         } else
diff --git a/src/pulsecore/source-output.c b/src/pulsecore/source-output.c
index d74a60e..6af1543 100644
--- a/src/pulsecore/source-output.c
+++ b/src/pulsecore/source-output.c
@@ -1087,7 +1087,7 @@ void pa_source_output_set_property(pa_source_output *o, const char *key, const c
 
     if (pa_proplist_contains(o->proplist, key)) {
         old_value = pa_xstrdup(pa_proplist_gets(o->proplist, key));
-        if (old_value) {
+        if (value && old_value) {
             if (pa_streq(value, old_value))
                 goto finish;
         } else

commit a07b6a8cda7fd088b60ab59186b486a7b0987282
Author: Ahmed S. Darwish <darwish.07 at gmail.com>
Date:   Fri Jun 17 21:56:41 2016 +0200

    pstream: Fix use of uninitialized value: ancillary fd cleanup flag
    
    As reported by valrgrind
    
      ==30002== Conditional jump or move depends on uninitialised value(s)
      ==30002==    at 0x5CB883C: pa_cmsg_ancil_data_close_fds (pstream.c:193)
      ==30002==    by 0x5CBB161: do_write (pstream.c:759)
      ==30002==    by 0x5CB8B51: do_pstream_read_write (pstream.c:233)
      ==30002==    by 0x5CB8EE8: io_callback (pstream.c:279)
      ...
    
    The pa_cmsg_ancil_data structure has two main guards:
    'creds_valid', which implies that it holds credentials
    information, and 'nfd', which implies it holds file descriptors.
    
    When code paths create a credentials ancillary data structure,
    they just set the 'nfd' guard to zero. Typically, the rest of
    pa_cmsg_ancil_data fields related to fds are _all_ left
    _uninitialized_.
    
    pa_cmsg_ancil_data_close_fds() has broken the above contract:
    it accesses the new 'close_fds_on_cleanup' flag, which is related
    to file descriptors, without checking the 'nfd == 0' guard first.
    Fix this inconsistency.
    
    Reported-by: Alexander E. Patrakov <patrakov at gmail.com>
    Signed-off-by: Ahmed S. Darwish <darwish.07 at gmail.com>
    Signed-off-by: Arun Raghavan <arun at arunraghavan.net>

diff --git a/src/pulsecore/pstream.c b/src/pulsecore/pstream.c
index 1ea3c5b..bbff2f6 100644
--- a/src/pulsecore/pstream.c
+++ b/src/pulsecore/pstream.c
@@ -190,7 +190,7 @@ struct pa_pstream {
  * it guarantees necessary cleanups after fds close.. This method is
  * also multiple-invocations safe. */
 void pa_cmsg_ancil_data_close_fds(struct pa_cmsg_ancil_data *ancil) {
-    if (ancil && ancil->close_fds_on_cleanup) {
+    if (ancil && ancil->nfd > 0 && ancil->close_fds_on_cleanup) {
         int i;
 
         pa_assert(ancil->nfd <= MAX_ANCIL_DATA_FDS);

commit 3922bbe7eb94232cc097bc3b7a91f06b2db93df2
Author: Ahmed S. Darwish <darwish.07 at gmail.com>
Date:   Fri Jun 17 21:54:54 2016 +0200

    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 at gmail.com>
    Signed-off-by: Ahmed S. Darwish <darwish.07 at gmail.com>
    Signed-off-by: Arun Raghavan <arun at arunraghavan.net>

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);



More information about the pulseaudio-commits mailing list