[pulseaudio-tickets] [Bug 77595] memblockq: Assertion 'length % bq->base == 0' failed, pulseaudio aborts

[bugzilla-daemon at freedesktop.org]

https://bugs.freedesktop.org/show_bug.cgi?id=77595

--- Comment #3 from David Henningsson <david.henningsson at canonical.com> ---
Okay, so we're writing 64K, starting at index 1. This later fails when we're
trying to drop 1 byte to get to the first package.

Why do we allow writing there at all? Looking at pa_memblockq_push, there is
already a check for length, so we should probably add one for index too. There
is no point in allowing an unaligned index if we don't allow unaligned lengths
(right?), so the first step should be:

=============
@@ -287,7 +287,8 @@ int pa_memblockq_push(pa_memblockq* bq, const pa_memchunk
*uchunk) {
     pa_assert(uchunk->length > 0);
     pa_assert(uchunk->index + uchunk->length <=
pa_memblock_get_length(uchunk->memblock));

-    pa_assert_se(uchunk->length % bq->base == 0);
+    pa_assert(uchunk->length % bq->base == 0);
+    pa_assert(uchunk->index % bq->base == 0);

     if (!can_push(bq, uchunk->length))
         return -1;
=============

(This assertion has no side effect, so remove the _se suffix.)

...however, this only makes us crash slightly earlier.

So, the desired client behaviour is probably to return with an error message in
case the client tries to do something we don't allow, like this:

==============
diff --git a/src/pulse/stream.c b/src/pulse/stream.c
index 8e35c29..bb94180 100644
--- a/src/pulse/stream.c
+++ b/src/pulse/stream.c
@@ -1481,6 +1481,8 @@ int pa_stream_write(
     PA_CHECK_VALIDITY(s->context, s->direction == PA_STREAM_PLAYBACK ||
s->direction == PA_STREAM_UPLOAD, PA_ERR_BADSTATE);
     PA_CHECK_VALIDITY(s->context, seek <= PA_SEEK_RELATIVE_END,
PA_ERR_INVALID);
     PA_CHECK_VALIDITY(s->context, s->direction == PA_STREAM_PLAYBACK || (seek
== PA_SEEK_RELATIVE && offset == 0), PA_ERR_INVALID);
+    PA_CHECK_VALIDITY(s->context, offset % pa_frame_size(&s->sample_spec) ==
0, PA_ERR_INVALID);
+    PA_CHECK_VALIDITY(s->context, length % pa_frame_size(&s->sample_spec) ==
0, PA_ERR_INVALID);
     PA_CHECK_VALIDITY(s->context,
                       !s->write_memblock ||
                       ((data >= s->write_data) &&
==============


However, that still won't stop us from malicious clients trying to crash the
daemon...

-- 
You are receiving this mail because:
You are the QA Contact for the bug.
You are the assignee for the bug.
-------------- next part --------------
An HTML attachment was scrubbed...
URL: <http://lists.freedesktop.org/archives/pulseaudio-bugs/attachments/20140829/0dda2d20/attachment.html>