mp3dec: Avoid possible endless loop.
authorAndre Noll <maan@systemlinux.org>
Wed, 31 Aug 2011 03:32:04 +0000 (05:32 +0200)
committerAndre Noll <maan@systemlinux.org>
Fri, 9 Sep 2011 21:28:11 +0000 (23:28 +0200)
If an error occurs during frame decode at the end of an 8K buffer,
the code in mp3dec.c might loop forever because we miss to consume
the data of all frames that have been decoded so far during this
iteration of the scheduler loop.

The two callers of used_mad_buffer_bytes() both call btr_consume()
next, and this fix requires to call the same two function once more
from another location. So it is natural to move the btr_consume()
call into used_mad_buffer_bytes() and rename the latter function
to mp3dec_consume().

mp3dec_filter.c

index 3ad9025..5c4b161 100644 (file)
@@ -50,16 +50,18 @@ static int handle_decode_error(struct private_mp3dec_data *pmd)
        return 0;
 }
 
-static size_t used_mad_buffer_bytes(struct mad_stream *s, size_t max)
+static void mp3dec_consume(struct btr_node *btrn, struct mad_stream *s,
+               size_t max)
 {
-       size_t rv;
+       size_t used;
 
        if (!s->next_frame)
-               return max;
-       /* we still have some data */
-       rv = s->next_frame - s->buffer;
-       assert(rv <= max);
-       return rv;
+               used = max;
+       else { /* we still have some data */
+               used = s->next_frame - s->buffer;
+               assert(used <= max);
+       }
+       btr_consume(btrn, used);
 }
 
 static void mp3dec_close(struct filter_node *fn)
@@ -82,7 +84,7 @@ static void mp3dec_post_select(__a_unused struct sched *s, struct task *t)
        int i, ret;
        struct private_mp3dec_data *pmd = fn->private_data;
        struct btr_node *btrn = fn->btrn;
-       size_t loaded = 0, used, len, iqs;
+       size_t loaded = 0, len, iqs;
        char *inbuffer, *outbuffer;
 
 next_buffer:
@@ -106,8 +108,7 @@ next_buffer:
 next_frame:
        ret = mad_header_decode(&pmd->frame.header, &pmd->stream);
        if (ret < 0) {
-               used = used_mad_buffer_bytes(&pmd->stream, len);
-               btr_consume(btrn, used);
+               mp3dec_consume(btrn, &pmd->stream, len);
                if (pmd->stream.error == MAD_ERROR_BUFLEN) {
                        if (len == iqs && btr_no_parent(btrn)) {
                                ret = -E_MP3DEC_EOF;
@@ -139,12 +140,12 @@ decode:
                        ret = -E_MP3DEC_CORRUPT;
                        if (fn->min_iqs > MP3DEC_MAX_FRAME)
                                goto err;
+                       mp3dec_consume(btrn, &pmd->stream, len);
                        return;
                }
                if (pmd->stream.error != MAD_ERROR_BADDATAPTR)
                        goto decode;
-               used = used_mad_buffer_bytes(&pmd->stream, len);
-               btr_consume(btrn, used);
+               mp3dec_consume(btrn, &pmd->stream, len);
                return;
        }
        fn->min_iqs = 0;