vss: Avoid read-overflowing the header buffer for ogg streams.

valgrind complains because of invalid reads/writes in vss.c:

	==998== Invalid write of size 1
	==998==    at 0x8050B09: vss_post_select (vss.c:574)
	==998==    by 0x806106C: schedule (sched.c:71)
	==998==    by 0x804EE04: main (server.c:579)
	==998==  Address 0x46d99bc is 0 bytes after a block of size 548 alloc'd
	==998==    at 0x4028A3B: realloc (vg_replace_malloc.c:632)
	==998==    by 0x805356B: para_realloc (string.c:40)
	==998==    by 0x80506EC: vss_post_select (vss.c:331)
	==998==    by 0x806106C: schedule (sched.c:71)
	==998==    by 0x804EE04: main (server.c:579)
	==998==

	...

	==5543== Invalid read of size 1
	==5543==    at 0x8050EBD: vss_post_select (vss.c:1099)
	==5543==    by 0x806108E: schedule (sched.c:71)
	==5543==    by 0x804EE04: main (server.c:579)
	==5543==  Address 0x47c70ac is 0 bytes after a block of size 3,956 alloc'd
	==5543==    at 0x4028A3B: realloc (vg_replace_malloc.c:632)
	==5543==    by 0x805358D: para_realloc (string.c:40)
	==5543==    by 0x80642AA: add_ogg_page (ogg_afh.c:78)
	==5543==    by 0x8064458: vorbis_get_header_callback (ogg_afh.c:132)
	==5543==    by 0x8063EF1: process_ogg_packets (ogg_afh_common.c:48)
	==5543==    by 0x8063F9A: ogg_get_file_info (ogg_afh_common.c:144)
	==5543==    by 0x8064200: vorbis_get_header (ogg_afh.c:149)
	==5543==    by 0x804FDD9: recv_afs_result (vss.c:1006)
	==5543==    by 0x80503F4: vss_post_select (vss.c:1124)
	==5543==    by 0x806108E: schedule (sched.c:71)
	==5543==    by 0x804EE04: main (server.c:579)

The problem is that for ogg streams chunk 0 points to a buffer on
the heap rather than to the mapped audio file, but we are checking
the buffer bounds against the memory map.

The fix consists of two parts. (a) We now treat a FEC group special
if it starts at chunk zero: Such a group now contains only this single
chunk. (b) When setting up the FEC group we always compare the buffer
bounds against the start of the first buffer in the group rather than
the memory map.

diff --git a/vss.c b/vss.c
index db1beeba17d080e08f306830bc0ce9dd0b7bf5e0..4a8aafa8d6e0c2ae239d09342ac10f3b08263828 100644 (file)
--- a/vss.c
+++ b/vss.c
@@ -361,8 +361,17 @@ static void vss_get_chunk(int chunk_num, struct vss_task *vsst,
 static void compute_group_size(struct vss_task *vsst, struct fec_group *g,
                int max_bytes)
 {
 static void compute_group_size(struct vss_task *vsst, struct fec_group *g,
                int max_bytes)
 {

+       char *buf;
+       size_t len;

        int i, max_chunks = PARA_MAX(1LU, 150 / tv2ms(vss_chunk_time()));
 
        int i, max_chunks = PARA_MAX(1LU, 150 / tv2ms(vss_chunk_time()));
 

+       if (g->first_chunk == 0) {
+               g->num_chunks = 1;
+               vss_get_chunk(0, vsst, &buf, &len);
+               g->bytes = len;
+               return;
+       }
+

        g->num_chunks = 0;
        g->bytes = 0;
        /*
        g->num_chunks = 0;
        g->bytes = 0;
        /*


@@ -372,8 +381,6 @@ static void compute_group_size(struct vss_task *vsst, struct fec_group *g,
         * of exactly one chunk for these audio formats.
         */
        for (i = 0;; i++) {
         * of exactly one chunk for these audio formats.
         */
        for (i = 0;; i++) {

-               char *buf;
-               size_t len;

                int chunk_num = g->first_chunk + i;
 
                if (g->bytes > 0 && i >= max_chunks) /* duration limit */
                int chunk_num = g->first_chunk + i;
 
                if (g->bytes > 0 && i >= max_chunks) /* duration limit */


@@ -502,7 +509,7 @@ static int setup_next_fec_group(struct fec_client *fc, struct vss_task *vsst)
 {
        int ret, i, k, n, data_slices;
        size_t len;
 {
        int ret, i, k, n, data_slices;
        size_t len;

-       char *buf;
+       char *buf, *p;

        struct fec_group *g = &fc->group;
 
        if (fc->state == FEC_STATE_NONE) {
        struct fec_group *g = &fc->group;
 
        if (fc->state == FEC_STATE_NONE) {


@@ -562,16 +569,20 @@ static int setup_next_fec_group(struct fec_client *fc, struct vss_task *vsst)
                assert(i == g->num_header_slices - 1);
        }
 
                assert(i == g->num_header_slices - 1);
        }
 

-       /* setup data slices */
+       /*
+        * Setup data slices. Note that for ogg streams chunk 0 points to a
+        * buffer on the heap rather than to the mapped audio file.
+        */

        vss_get_chunk(g->first_chunk, vsst, &buf, &len);
        vss_get_chunk(g->first_chunk, vsst, &buf, &len);

-       for (; i < g->num_header_slices + data_slices; i++) {
-               if (buf + g->slice_bytes > vsst->map + mmd->size) {
+       for (p = buf; i < g->num_header_slices + data_slices; i++) {
+               if (p + g->slice_bytes > buf + g->bytes) {

                        /*
                        /*

-                        * Can not use the memory mapped audio file for this
-                        * slice as it goes beyond the map.
+                        * We must make a copy for this slice since using p
+                        * directly would exceed the buffer.

                         */
                         */

-                       uint32_t payload_size = vsst->map + mmd->size - buf;
-                       memcpy(fc->extra_src_buf, buf, payload_size);
+                       uint32_t payload_size = buf + g->bytes - p;
+                       assert(payload_size + FEC_HEADER_SIZE <= fc->mps);
+                       memcpy(fc->extra_src_buf, p, payload_size);

                        if (payload_size < g->slice_bytes)
                                memset(fc->extra_src_buf + payload_size, 0,
                                        g->slice_bytes - payload_size);
                        if (payload_size < g->slice_bytes)
                                memset(fc->extra_src_buf + payload_size, 0,
                                        g->slice_bytes - payload_size);


@@ -579,8 +590,8 @@ static int setup_next_fec_group(struct fec_client *fc, struct vss_task *vsst)
                        i++;
                        break;
                }
                        i++;
                        break;
                }

-               fc->src_data[i] = (const unsigned char *)buf;
-               buf += g->slice_bytes;
+               fc->src_data[i] = (const unsigned char *)p;
+               p += g->slice_bytes;

        }
        if (i < k) {
                /* use arbitrary data for all remaining slices */
        }
        if (i < k) {
                /* use arbitrary data for all remaining slices */