[pulseaudio-discuss] [PATCH 1/3] pstream: Don't split (non-SHM) memblocks

[Peter Meerwald]

On Thu, 5 Mar 2015, David Henningsson wrote:

> In case SHM is full or disabled, audio data is sent through the
> io/srbchannel. When this channel in turn gets full, memblocks
> could previously be split up. This could lead to crashes in case
> the split was on non-frame boundaries (in combination with full
> memblock queues).
> 
> BugLink: https://bugs.freedesktop.org/show_bug.cgi?id=88452
> Signed-off-by: David Henningsson <david.henningsson at canonical.com>
> ---
>  src/pulsecore/pstream.c | 67 +++++++++++++++++++++++--------------------------
>  1 file changed, 31 insertions(+), 36 deletions(-)
> 
> diff --git a/src/pulsecore/pstream.c b/src/pulsecore/pstream.c
> index b0ed5a7..8c05a87 100644
> --- a/src/pulsecore/pstream.c
> +++ b/src/pulsecore/pstream.c
> @@ -682,6 +682,36 @@ fail:
>      return -1;
>  }
>  
> +static void memblock_complete(pa_pstream *p, struct pstream_read *re) {
> +    size_t l;
> +    pa_memchunk chunk;
> +    int64_t offset;
> +
> +    if (!p->receive_memblock_callback)
> +        return;
> +
> +    /* Is this memblock data? Then pass it to the user */
> +    l = re->index - PA_PSTREAM_DESCRIPTOR_SIZE;
> +    if (l <= 0)

l is an unsigned data type; the difference should not become negative and 
the check should just be for (l == 0)

> +        return;
> +
> +    chunk.memblock = re->memblock;
> +    chunk.index = 0;
> +    chunk.length = l;
> +
> +    offset = (int64_t) (
> +             (((uint64_t) ntohl(re->descriptor[PA_PSTREAM_DESCRIPTOR_OFFSET_HI])) << 32) |
> +             (((uint64_t) ntohl(re->descriptor[PA_PSTREAM_DESCRIPTOR_OFFSET_LO]))));
> +
> +    p->receive_memblock_callback(
> +        p,
> +        ntohl(re->descriptor[PA_PSTREAM_DESCRIPTOR_CHANNEL]),
> +        offset,
> +        ntohl(re->descriptor[PA_PSTREAM_DESCRIPTOR_FLAGS]) & PA_FLAG_SEEKMASK,
> +        &chunk,
> +        p->receive_memblock_callback_userdata);
> +}
> +
>  static int do_read(pa_pstream *p, struct pstream_read *re) {
>      void *d;
>      size_t l;
> @@ -831,47 +861,12 @@ static int do_read(pa_pstream *p, struct pstream_read *re) {
>          }
>  
>      } else if (re->index > PA_PSTREAM_DESCRIPTOR_SIZE) {
> -        /* Frame payload available */
> -
> -        if (re->memblock && p->receive_memblock_callback) {
> -
> -            /* Is this memblock data? Than pass it to the user */
> -            l = (re->index - (size_t) r) < PA_PSTREAM_DESCRIPTOR_SIZE ? (size_t) (re->index - PA_PSTREAM_DESCRIPTOR_SIZE) : (size_t) r;
> -
> -            if (l > 0) {
> -                pa_memchunk chunk;
> -
> -                chunk.memblock = re->memblock;
> -                chunk.index = re->index - PA_PSTREAM_DESCRIPTOR_SIZE - l;
> -                chunk.length = l;
> -
> -                if (p->receive_memblock_callback) {
> -                    int64_t offset;
> -
> -                    offset = (int64_t) (
> -                            (((uint64_t) ntohl(re->descriptor[PA_PSTREAM_DESCRIPTOR_OFFSET_HI])) << 32) |
> -                            (((uint64_t) ntohl(re->descriptor[PA_PSTREAM_DESCRIPTOR_OFFSET_LO]))));
> -
> -                    p->receive_memblock_callback(
> -                        p,
> -                        ntohl(re->descriptor[PA_PSTREAM_DESCRIPTOR_CHANNEL]),
> -                        offset,
> -                        ntohl(re->descriptor[PA_PSTREAM_DESCRIPTOR_FLAGS]) & PA_FLAG_SEEKMASK,
> -                        &chunk,
> -                        p->receive_memblock_callback_userdata);
> -                }
> -
> -                /* Drop seek info for following callbacks */
> -                re->descriptor[PA_PSTREAM_DESCRIPTOR_FLAGS] =
> -                    re->descriptor[PA_PSTREAM_DESCRIPTOR_OFFSET_HI] =
> -                    re->descriptor[PA_PSTREAM_DESCRIPTOR_OFFSET_LO] = 0;
> -            }
> -        }
>  
>          /* Frame complete */
>          if (re->index >= ntohl(re->descriptor[PA_PSTREAM_DESCRIPTOR_LENGTH]) + PA_PSTREAM_DESCRIPTOR_SIZE) {
>  
>              if (re->memblock) {
> +                memblock_complete(p, re);
>  
>                  /* This was a memblock frame. We can unref the memblock now */
>                  pa_memblock_unref(re->memblock);
> -- 
> 1.9.1
> 
> _______________________________________________
> pulseaudio-discuss mailing list
> pulseaudio-discuss at lists.freedesktop.org
> http://lists.freedesktop.org/mailman/listinfo/pulseaudio-discuss
> 

-- 

Peter Meerwald
+43-664-2444418 (mobile)