mp4: Improve handling of read errors.

Currently read_data() of mp4.c is an atrocious mess. The ->read()
callback is defined to return uint32_t, but the return value is
stored in a signed 32 bit integer. Moreover, read_data() contains a
dead store, it handles neither short nor interrupted reads correctly,
and it moves the file position backwards on errors.

While this is easy to fix, a more intricate problem is that most
callers of read_data(), including all read_intX() helpers, ignore the
return value of read_data() and return uninitialized stack contents in
the error case. This is kind of dealt with by the ->read_error member
of struct mp4, but this not more than a kludge, which, according to
the comments, was applied after several CVEs had been filed against
the library.

Let's DTRT here, even though it adds a fair amount of new code:
Check the return value of each read operation and fail early on errors.

We have to distinguish three cases: error, EOF, and success, encoded
as return values -1, 0 and 1, respectively. This commit converts most
functions which read from an mp4 file to this convention. More work
is required as return values are not checked everywhere yet. This was
left for subsequent commits to keep the already large patch within
reasonable size.

Since we don't rely on ->read_error of struct mp4 any more, it can
be removed.

diff --git a/aac_afh.c b/aac_afh.c
index 30e7164e726b21956b0734d69369d30227bad95c..2226394aa2f2370d0e85b8bfc648328bc8452245 100644 (file)
--- a/aac_afh.c
+++ b/aac_afh.c
@@ -27,7 +27,7 @@ struct aac_afh_context {
        struct mp4_callback cb;
 };
 
-static uint32_t aac_afh_read_cb(void *user_data, void *dest, uint32_t want)
+static ssize_t aac_afh_read_cb(void *user_data, void *dest, size_t want)
 {
        struct aac_afh_context *c = user_data;
        size_t have, rv;
@@ -35,7 +35,7 @@ static uint32_t aac_afh_read_cb(void *user_data, void *dest, uint32_t want)
        if (want == 0 || c->fpos >= c->mapsize)
                return 0;
        have = c->mapsize - c->fpos;
-       rv = PARA_MIN(have, (size_t)want);
+       rv = PARA_MIN(have, want);
        PARA_DEBUG_LOG("reading %zu bytes @%zu\n", rv, c->fpos);
        memcpy(dest, c->map + c->fpos, rv);
        c->fpos += rv;
@@ -178,7 +178,7 @@ close:
        return ret;
 }
 
-static uint32_t aac_afh_meta_read_cb(void *user_data, void *dest, uint32_t want)
+static ssize_t aac_afh_meta_read_cb(void *user_data, void *dest, size_t want)
 {
        int fd = *(int *)user_data;
        return read(fd, dest, want);

diff --git a/mp4.c b/mp4.c
index aa9435927c3efbf5a1956d6e0ce86a27f5fbe192..3e7a74a88d4c3c2c401fc77947c77296045c8d1f 100644 (file)
--- a/mp4.c
+++ b/mp4.c
@@ -52,7 +52,6 @@ struct mp4 {
        uint8_t last_atom;
        uint64_t file_size;
 
-       uint32_t read_error;
        uint32_t error;
 
        /* incremental track index while reading the file */
@@ -70,57 +69,73 @@ int32_t mp4_total_tracks(const struct mp4 *f)
        return f->total_tracks;
 }
 
-static int32_t read_data(struct mp4 *f, void *data, uint32_t size)
+/*
+ * Returns -1, 0, or 1 on errors/EOF/success. Partial reads followed by EOF or
+ * read errors are treated as errors.
+ */
+static int read_data(struct mp4 *f, void *data, size_t size)
 {
-       int32_t result = 1;
-
-       result = f->cb->read(f->cb->user_data, data, size);
-
-       if (result < size)
-               f->read_error++;
-
-       f->current_position += size;
-
-       return result;
+       while (size > 0) {
+               ssize_t ret = f->cb->read(f->cb->user_data, data, size);
+               if (ret < 0 && errno == EINTR)
+                       continue;
+               /* regard EAGAIN as an error as reads should be blocking. */
+               if (ret <= 0)
+                       return ret < 0? -1 : 0;
+               f->current_position += ret;
+               size -= ret;
+       }
+       return 1;
 }
 
-static uint64_t read_int64(struct mp4 *f)
+static int read_int64(struct mp4 *f, uint64_t *result)
 {
        uint8_t data[8];
+       int ret = read_data(f, data, 8);
 
-       read_data(f, data, 8);
-       return read_u64_be(data);
+       if (ret > 0 && result)
+               *result = read_u64_be(data);
+       return ret;
 }
 
-static uint32_t read_int32(struct mp4 *f)
+static int read_int32(struct mp4 *f, uint32_t *result)
 {
-       int8_t data[4];
+       uint8_t data[4];
+       int ret = read_data(f, data, 4);
 
-       read_data(f, data, 4);
-       return read_u32_be(data);
+       if (ret > 0 && result)
+               *result = read_u32_be(data);
+       return ret;
 }
 
-static uint32_t read_int24(struct mp4 *f)
+static int read_int24(struct mp4 *f, uint32_t *result)
 {
-       int8_t data[4];
+       uint8_t data[3];
+       int ret = read_data(f, data, 3);
 
-       read_data(f, data, 3);
-       return read_u24_be(data);
+       if (ret > 0 && result)
+               *result = read_u24_be(data);
+       return ret;
 }
 
-static uint16_t read_int16(struct mp4 *f)
+static int read_int16(struct mp4 *f, uint16_t *result)
 {
-       int8_t data[2];
+       uint8_t data[2];
+       int ret = read_data(f, data, 2);
 
-       read_data(f, data, 2);
-       return read_u16_be(data);
+       if (ret > 0 && result)
+               *result = read_u16_be(data);
+       return ret;
 }
 
-static uint8_t read_int8(struct mp4 *f)
+static uint8_t read_int8(struct mp4 *f, uint8_t *result)
 {
-       uint8_t output;
-       read_data(f, &output, 1);
-       return output;
+       uint8_t data[1];
+       int ret = read_data(f, data, 1);
+
+       if (ret > 0 && result)
+               *result = data[0];
+       return ret;
 }
 
 static bool atom_compare(int8_t a1, int8_t b1, int8_t c1, int8_t d1,
@@ -128,6 +143,7 @@ static bool atom_compare(int8_t a1, int8_t b1, int8_t c1, int8_t d1,
 {
        return a1 == a2 && b1 == b2 && c1 == c2 && d1 == d2;
 }
+
 enum atoms {
        /* atoms with subatoms */
        ATOM_MOOV = 1,
@@ -366,29 +382,33 @@ static uint8_t atom_name_to_type(int8_t a, int8_t b, int8_t c, int8_t d)
                return ATOM_UNKNOWN;
 }
 
-/* read atom header, return atom size, atom size is with header included */
-static uint64_t atom_read_header(struct mp4 *f, uint8_t * atom_type,
-                               uint8_t * header_size)
+/* read atom header, atom size is returned with header included. */
+static int atom_read_header(struct mp4 *f, uint8_t *atom_type,
+               uint8_t *header_size, uint64_t *atom_size)
 {
-       uint64_t size;
-       int32_t ret;
+       uint32_t size;
+       int ret;
        int8_t atom_header[8];
 
        ret = read_data(f, atom_header, 8);
-       if (ret != 8)
-               return 0;
-
+       if (ret <= 0)
+               return ret;
        size = read_u32_be(atom_header);
-       *header_size = 8;
-
-       /* check for 64 bit atom size */
-       if (size == 1) {
-               *header_size = 16;
-               size = read_int64(f);
+       if (size == 1) { /* 64 bit atom size */
+               if (header_size)
+                       *header_size = 16;
+               ret = read_int64(f, atom_size);
+               if (ret <= 0)
+                       return ret;
+       } else {
+               if (header_size)
+                       *header_size = 8;
+               if (atom_size)
+                       *atom_size = size;
        }
        *atom_type = atom_name_to_type(atom_header[4], atom_header[5],
                atom_header[6], atom_header[7]);
-       return size;
+       return 1;
 }
 
 static int64_t get_position(const struct mp4 *f)
@@ -437,162 +457,221 @@ static void track_add(struct mp4 *f)
        f->track[f->total_tracks - 1] = para_calloc(sizeof(struct mp4_track));
 }
 
-static int32_t read_stsz(struct mp4 *f)
+static int read_stsz(struct mp4 *f)
 {
+       int ret;
        int32_t i;
        struct mp4_track *t;
 
        if (f->total_tracks == 0)
-               return f->error++;
+               return -1;
        t = f->track[f->total_tracks - 1];
-       read_int8(f); /* version */
-       read_int24(f);  /* flags */
-       t->stsz_sample_size = read_int32(f);
-       t->stsz_sample_count = read_int32(f);
+       ret = read_int8(f, NULL); /* version */
+       if (ret <= 0)
+               return ret;
+       ret = read_int24(f, NULL); /* flags */
+       if (ret <= 0)
+               return ret;
+       ret = read_int32(f, &t->stsz_sample_size);
+       if (ret <= 0)
+               return ret;
+       ret = read_int32(f, &t->stsz_sample_count);
+       if (ret <= 0)
+               return ret;
        if (t->stsz_sample_size != 0)
-               return 0;
+               return 1;
        t->stsz_table = para_malloc(t->stsz_sample_count * sizeof(int32_t));
-       for (i = 0; i < t->stsz_sample_count && !f->read_error; i++)
-               t->stsz_table[i] = read_int32(f);
-       return 0;
+       for (i = 0; i < t->stsz_sample_count; i++) {
+               ret = read_int32(f, &t->stsz_table[i]);
+               if (ret <= 0)
+                       return ret;
+       }
+       return 1;
 }
 
-static int32_t read_stts(struct mp4 *f)
+static int read_stts(struct mp4 *f)
 {
+       int ret;
        int32_t i;
        struct mp4_track *t;
 
-       /* CVE-2017-9223 */
        if (f->total_tracks == 0)
-               return f->error++;
+               return -1;
        t = f->track[f->total_tracks - 1];
        if (t->stts_entry_count)
                return 0;
-       read_int8(f); /* version */
-       read_int24(f);  /* flags */
-       t->stts_entry_count = read_int32(f);
-
+       ret = read_int8(f, NULL); /* version */
+       if (ret <= 0)
+               return ret;
+       ret = read_int24(f, NULL); /* flags */
+       if (ret <= 0)
+               return ret;
+       ret = read_int32(f, &t->stts_entry_count);
+       if (ret <= 0)
+               return ret;
        t->stts_sample_count = para_malloc(t->stts_entry_count
                * sizeof(int32_t));
        t->stts_sample_delta = para_malloc(t->stts_entry_count
                * sizeof (int32_t));
-       /* CVE-2017-9254 */
-       for (i = 0; i < t->stts_entry_count && !f->read_error; i++) {
-               t->stts_sample_count[i] = read_int32(f);
-               t->stts_sample_delta[i] = read_int32(f);
+       for (i = 0; i < t->stts_entry_count; i++) {
+               ret = read_int32(f, &t->stts_sample_count[i]);
+               if (ret <= 0)
+                       return ret;
+               ret = read_int32(f, &t->stts_sample_delta[i]);
+               if (ret <= 0)
+                       return ret;
        }
        return 1;
 }
 
-static int32_t read_stsc(struct mp4 *f)
+static int read_stsc(struct mp4 *f)
 {
+       int ret;
        int32_t i;
        struct mp4_track *t;
 
        if (f->total_tracks == 0)
-               return f->error++;
+               return -1;
        t = f->track[f->total_tracks - 1];
 
-       read_int8(f); /* version */
-       read_int24(f);  /* flags */
-       t->stsc_entry_count = read_int32(f);
+       ret = read_int8(f, NULL); /* version */
+       if (ret <= 0)
+               return ret;
+       ret = read_int24(f, NULL); /* flags */
+       if (ret <= 0)
+               return ret;
+       ret = read_int32(f, &t->stsc_entry_count);
+       if (ret <= 0)
+               return ret;
        t->stsc_first_chunk = para_malloc(t->stsc_entry_count * sizeof(int32_t));
        t->stsc_samples_per_chunk = para_malloc(t->stsc_entry_count
                * sizeof (int32_t));
        t->stsc_sample_desc_index = para_malloc(t->stsc_entry_count *
                sizeof (int32_t));
 
-       /* CVE-2017-9255 */
-       for (i = 0; i < t->stsc_entry_count && !f->read_error; i++) {
-               t->stsc_first_chunk[i] = read_int32(f);
-               t->stsc_samples_per_chunk[i] = read_int32(f);
-               t->stsc_sample_desc_index[i] = read_int32(f);
+       for (i = 0; i < t->stsc_entry_count; i++) {
+               ret = read_int32(f, &t->stsc_first_chunk[i]);
+               if (ret <= 0)
+                       return ret;
+               ret = read_int32(f, &t->stsc_samples_per_chunk[i]);
+               if (ret <= 0)
+                       return ret;
+               ret = read_int32(f, &t->stsc_sample_desc_index[i]);
+               if (ret <= 0)
+                       return ret;
        }
-       return 0;
+       return 1;
 }
 
-static int32_t read_stco(struct mp4 *f)
+static int read_stco(struct mp4 *f)
 {
+       int ret;
        int32_t i;
        struct mp4_track *t;
 
        if (f->total_tracks == 0)
-               return f->error++;
+               return -1;
        t = f->track[f->total_tracks - 1];
 
-       read_int8(f); /* version */
-       read_int24(f);  /* flags */
-       t->stco_entry_count = read_int32(f);
+       ret = read_int8(f, NULL); /* version */
+       if (ret <= 0)
+               return ret;
+       ret = read_int24(f, NULL); /* flags */
+       if (ret <= 0)
+               return ret;
+       ret = read_int32(f, &t->stco_entry_count);
+       if (ret <= 0)
+               return ret;
        t->stco_chunk_offset = para_malloc(t->stco_entry_count
                * sizeof(int32_t));
-       /* CVE-2017-9256 */
-       for (i = 0; i < t->stco_entry_count && !f->read_error; i++)
-               t->stco_chunk_offset[i] = read_int32(f);
-       return 0;
+       for (i = 0; i < t->stco_entry_count; i++) {
+               ret = read_int32(f, &t->stco_chunk_offset[i]);
+               if (ret <= 0)
+                       return ret;
+       }
+       return 1;
 }
 
-static int32_t read_mp4a(struct mp4 *f)
+static int read_mp4a(struct mp4 *f)
 {
+       int ret;
        int32_t i;
        uint8_t atom_type = 0;
        uint8_t header_size = 0;
        struct mp4_track *t;
 
        if (f->total_tracks == 0)
-               return f->error++;
+               return -1;
        t = f->track[f->total_tracks - 1];
 
-       for (i = 0; i < 6; i++)
-               read_int8(f); /* reserved */
-       /* data_reference_index */ read_int16(f);
-
-       read_int32(f);  /* reserved */
-       read_int32(f);  /* reserved */
-
-       t->channelCount = read_int16(f);
-       read_int16(f);
-
-       read_int16(f);
-       read_int16(f);
-
-       t->sampleRate = read_int16(f);
-
-       read_int16(f);
-
-       atom_read_header(f, &atom_type, &header_size);
-       return 0;
-}
-
-static int32_t read_stsd(struct mp4 *f)
-{
-       int32_t i, entry_count;
+       for (i = 0; i < 6; i++) {
+               ret = read_int8(f, NULL); /* reserved */
+               if (ret <= 0)
+                       return ret;
+       }
+       ret = read_int16(f, NULL); /* data_reference_index */
+       if (ret <= 0)
+               return ret;
+       ret = read_int32(f, NULL); /* reserved */
+       if (ret <= 0)
+               return ret;
+       ret = read_int32(f, NULL); /* reserved */
+       if (ret <= 0)
+               return ret;
+       ret = read_int16(f, &t->channelCount);
+       if (ret <= 0)
+               return ret;
+       ret = read_int16(f, NULL);
+       if (ret <= 0)
+               return ret;
+       ret = read_int16(f, NULL);
+       if (ret <= 0)
+               return ret;
+       ret = read_int16(f, NULL);
+       if (ret <= 0)
+               return ret;
+       ret = read_int16(f, &t->sampleRate);
+       if (ret <= 0)
+               return ret;
+       ret = read_int16(f, NULL);
+       if (ret <= 0)
+               return ret;
+       return atom_read_header(f, &atom_type, &header_size, NULL);
+}
+
+static int read_stsd(struct mp4 *f)
+{
+       int ret;
+       uint32_t i, entry_count;
        uint8_t header_size = 0;
        struct mp4_track *t;
 
-       /* CVE-2017-9218 */
        if (f->total_tracks == 0)
-               return f->error++;
+               return -1;
        t = f->track[f->total_tracks - 1];
-
-       read_int8(f); /* version */
-       read_int24(f);  /* flags */
-
-       entry_count = read_int32(f);
-
-       /* CVE-2017-9253 */
-       for (i = 0; i < entry_count && !f->read_error; i++) {
+       ret = read_int8(f, NULL); /* version */
+       if (ret <= 0)
+               return ret;
+       ret = read_int24(f, NULL); /* flags */
+       if (ret <= 0)
+               return ret;
+       ret = read_int32(f, &entry_count);
+       if (ret <= 0)
+               return ret;
+       for (i = 0; i < entry_count; i++) {
                uint64_t skip = get_position(f);
                uint64_t size;
                uint8_t atom_type = 0;
-               size = atom_read_header(f, &atom_type, &header_size);
+               ret = atom_read_header(f, &atom_type, &header_size, &size);
+               if (ret <= 0)
+                       return ret;
                skip += size;
                t->is_audio = atom_type == ATOM_MP4A;
                if (t->is_audio)
                        read_mp4a(f);
                set_position(f, skip);
        }
-
-       return 0;
+       return 1;
 }
 
 static int32_t tag_add_field(struct mp4_metadata *meta, const char *item,
@@ -613,15 +692,19 @@ static int32_t tag_add_field(struct mp4_metadata *meta, const char *item,
        return 1;
 }
 
-static char *read_string(struct mp4 *f, uint32_t length)
+static int read_string(struct mp4 *f, uint32_t length, char **result)
 {
        char *str = para_malloc(length + 1);
-       if ((uint32_t)read_data(f, str, length) != length) {
+       int ret = read_data(f, str, length);
+
+       if (ret <= 0) {
                free(str);
-               str = NULL;
-       } else
-               str[length] = 0;
-       return str;
+               *result = NULL;
+       } else {
+               str[length] = '\0';
+               *result = str;
+       }
+       return ret;
 }
 
 static const char *get_metadata_name(uint8_t atom_type)
@@ -636,8 +719,9 @@ static const char *get_metadata_name(uint8_t atom_type)
        }
 }
 
-static void parse_tag(struct mp4 *f, uint8_t parent, int32_t size)
+static int parse_tag(struct mp4 *f, uint8_t parent, int32_t size)
 {
+       int ret;
        uint64_t subsize, sumsize;
        char *data = NULL;
        uint32_t len = 0;
@@ -645,70 +729,104 @@ static void parse_tag(struct mp4 *f, uint8_t parent, int32_t size)
 
        for (
                sumsize = 0;
-               sumsize < size && !f->read_error; /* CVE-2017-9222 */
+               sumsize < size;
                set_position(f, destpos), sumsize += subsize
        ) {
                uint8_t atom_type;
                uint8_t header_size = 0;
-               subsize = atom_read_header(f, &atom_type, &header_size);
+               ret = atom_read_header(f, &atom_type, &header_size, &subsize);
+               if (ret <= 0)
+                       return ret;
                destpos = get_position(f) + subsize - header_size;
                if (atom_type != ATOM_DATA)
                        continue;
-               read_int8(f); /* version */
-               read_int24(f);  /* flags */
-               read_int32(f);  /* reserved */
+               ret = read_int8(f, NULL); /* version */
+               if (ret <= 0)
+                       return ret;
+               ret = read_int24(f, NULL); /* flags */
+               if (ret <= 0)
+                       return ret;
+               ret = read_int32(f, NULL); /* reserved */
+               if (ret <= 0)
+                       return ret;
                free(data);
-               data = read_string(f, subsize - (header_size + 8));
+               ret = read_string(f, subsize - (header_size + 8), &data);
+               if (ret <= 0)
+                       return ret;
                len = subsize - (header_size + 8);
        }
        if (!data)
-               return;
+               return -1;
        tag_add_field(&f->meta, get_metadata_name(parent), data, len);
        free(data);
+       return 1;
 }
 
-static int32_t read_mdhd(struct mp4 *f)
+static int read_mdhd(struct mp4 *f)
 {
+       int ret;
        uint32_t version;
        struct mp4_track *t;
 
-       /* CVE-2017-9221 */
        if (f->total_tracks == 0)
-               return f->error++;
+               return -1;
        t = f->track[f->total_tracks - 1];
 
-       version = read_int32(f);
+       ret = read_int32(f, &version);
+       if (ret <= 0)
+               return ret;
        if (version == 1) {
-               read_int64(f); //creation-time
-               read_int64(f); //modification-time
-               t->timeScale = read_int32(f); //timescale
-               t->duration = read_int64(f); //duration
+               ret = read_int64(f, NULL); /* creation-time */
+               if (ret <= 0)
+                       return ret;
+               ret = read_int64(f, NULL); /* modification-time */
+               if (ret <= 0)
+                       return ret;
+               ret = read_int32(f, &t->timeScale);
+               if (ret <= 0)
+                       return ret;
+               ret = read_int64(f, &t->duration);
+               if (ret <= 0)
+                       return ret;
        } else { //version == 0
                uint32_t temp;
 
-               read_int32(f);  //creation-time
-               read_int32(f);  //modification-time
-               t->timeScale = read_int32(f); //timescale
-               temp = read_int32(f);
+               ret = read_int32(f, NULL); /* creation-time */
+               if (ret <= 0)
+                       return ret;
+               ret = read_int32(f, NULL); /* modification-time */
+               if (ret <= 0)
+                       return ret;
+               ret = read_int32(f, &t->timeScale);
+               if (ret <= 0)
+                       return ret;
+               ret = read_int32(f, &temp);
+               if (ret <= 0)
+                       return ret;
                t->duration = (temp == (uint32_t) (-1))?
                        (uint64_t) (-1) : (uint64_t) (temp);
        }
-       read_int16(f);
-       read_int16(f);
+       ret = read_int16(f, NULL);
+       if (ret <= 0)
+               return ret;
+       ret = read_int16(f, NULL);
+       if (ret <= 0)
+               return ret;
        return 1;
 }
 
 static int32_t read_ilst(struct mp4 *f, int32_t size)
 {
+       int ret;
        uint64_t sumsize = 0;
 
        while (sumsize < size) {
                uint8_t atom_type;
                uint64_t subsize, destpos;
                uint8_t header_size = 0;
-               subsize = atom_read_header(f, &atom_type, &header_size);
-               if (subsize == 0)
-                       break;
+               ret = atom_read_header(f, &atom_type, &header_size, &subsize);
+               if (ret <= 0)
+                       return ret;
                destpos = get_position(f) + subsize - header_size;
                switch (atom_type) {
                case ATOM_ARTIST:
@@ -721,21 +839,26 @@ static int32_t read_ilst(struct mp4 *f, int32_t size)
                set_position(f, destpos);
                sumsize += subsize;
        }
-
-       return 0;
+       return 1;
 }
 
 static int32_t read_meta(struct mp4 *f, uint64_t size)
 {
+       int ret;
        uint64_t subsize, sumsize = 0;
        uint8_t atom_type;
        uint8_t header_size = 0;
 
-       read_int8(f); /* version */
-       read_int24(f);  /* flags */
-
+       ret = read_int8(f, NULL); /* version */
+       if (ret <= 0)
+               return ret;
+       ret = read_int24(f, NULL); /* flags */
+       if (ret <= 0)
+               return ret;
        while (sumsize < (size - (header_size + 4))) {
-               subsize = atom_read_header(f, &atom_type, &header_size);
+               ret = atom_read_header(f, &atom_type, &header_size, &subsize);
+               if (ret <= 0)
+                       return ret;
                if (subsize <= header_size + 4)
                        return 1;
                if (atom_type == ATOM_ILST)
@@ -744,8 +867,7 @@ static int32_t read_meta(struct mp4 *f, uint64_t size)
                        set_position(f, get_position(f) + subsize - header_size);
                sumsize += subsize;
        }
-
-       return 0;
+       return 1;
 }
 
 static int32_t atom_read(struct mp4 *f, int32_t size, uint8_t atom_type)
@@ -779,21 +901,21 @@ static int32_t atom_read(struct mp4 *f, int32_t size, uint8_t atom_type)
 }
 
 /* parse atoms that are sub atoms of other atoms */
-static int32_t parse_sub_atoms(struct mp4 *f, uint64_t total_size, int meta_only)
+static int parse_sub_atoms(struct mp4 *f, uint64_t total_size, int meta_only)
 {
+       int ret;
        uint64_t size;
        uint8_t atom_type = 0;
        uint64_t counted_size = 0;
        uint8_t header_size = 0;
 
        while (counted_size < total_size) {
-               size = atom_read_header(f, &atom_type, &header_size);
-               counted_size += size;
-
-               /* check for end of file */
+               ret = atom_read_header(f, &atom_type, &header_size, &size);
+               if (ret <= 0)
+                       return ret;
                if (size == 0)
-                       break;
-
+                       return -1;
+               counted_size += size;
                /* we're starting to read a new track, update index,
                 * so that all data and tables get written in the right place
                 */
@@ -808,22 +930,20 @@ static int32_t parse_sub_atoms(struct mp4 *f, uint64_t total_size, int meta_only
                        atom_read(f, (uint32_t) size, atom_type);
                }
        }
-
-       return 0;
+       return 1;
 }
 
 /* parse root atoms */
 static int32_t parse_atoms(struct mp4 *f, int meta_only)
 {
+       int ret;
        uint64_t size;
        uint8_t atom_type = 0;
        uint8_t header_size = 0;
 
        f->file_size = 0;
-       f->read_error = 0;
 
-       while ((size =
-               atom_read_header(f, &atom_type, &header_size)) != 0) {
+       while ((ret = atom_read_header(f, &atom_type, &header_size, &size)) > 0) {
                f->file_size += size;
                f->last_atom = atom_type;
 
@@ -842,8 +962,7 @@ static int32_t parse_atoms(struct mp4 *f, int meta_only)
                        set_position(f, get_position(f) + size - header_size);
                }
        }
-
-       return 0;
+       return ret;
 }
 
 struct mp4 *mp4_open_read(const struct mp4_callback *cb)
@@ -1073,44 +1192,59 @@ struct mp4_metadata *mp4_get_meta(struct mp4 *f)
        return &f->meta;
 }
 
-static uint32_t find_atom(struct mp4 *f, uint64_t base, uint32_t size,
-                         const char *name)
+static int find_atom(struct mp4 *f, uint64_t base, uint32_t size,
+       const char *name)
 {
        uint32_t remaining = size;
        uint64_t atom_offset = base;
+
        for (;;) {
+               int ret;
                char atom_name[4];
                uint32_t atom_size;
 
                set_position(f, atom_offset);
 
                if (remaining < 8)
-                       break;
-               atom_size = read_int32(f);
+                       return -1;
+               ret = read_int32(f, &atom_size);
+               if (ret <= 0)
+                       return ret;
                if (atom_size > remaining || atom_size < 8)
-                       break;
-               read_data(f, atom_name, 4);
-
+                       return -1;
+               ret = read_data(f, atom_name, 4);
+               if (ret <= 0)
+                       return ret;
                if (!memcmp(atom_name, name, 4)) {
                        set_position(f, atom_offset);
                        return 1;
                }
-
                remaining -= atom_size;
                atom_offset += atom_size;
        }
-       return 0;
 }
 
-static uint32_t find_atom_v2(struct mp4 *f, uint64_t base, uint32_t size,
+/*
+ * Try to find atom <name> with atom <name_inside> in it. Besides -1/0/1 for
+ * error, EOF and success, this function may return 2 to indicate that the
+ * desired atoms were not found.
+ */
+static int find_atom_v2(struct mp4 *f, uint64_t base, uint32_t size,
                const char *name, uint32_t extraheaders, const char *name_inside)
 {
        uint64_t first_base = (uint64_t) (-1);
-       while (find_atom(f, base, size, name))  //try to find atom <name> with atom <name_inside> in it
-       {
-               uint64_t mybase = get_position(f);
-               uint32_t mysize = read_int32(f);
 
+       for (;;) {
+               uint64_t mybase;
+               uint32_t mysize;
+               int ret = find_atom(f, base, size, name);
+
+               if (ret <= 0)
+                       return ret;
+               mybase = get_position(f);
+               ret = read_int32(f, &mysize);
+               if (ret <= 0)
+                       return ret;
                if (first_base == (uint64_t) (-1))
                        first_base = mybase;
 
@@ -1120,20 +1254,19 @@ static uint32_t find_atom_v2(struct mp4 *f, uint64_t base, uint32_t size,
                if (find_atom (f, mybase + (8 + extraheaders),
                                mysize - (8 + extraheaders), name_inside)) {
                        set_position(f, mybase);
-                       return 2;
+                       return 1;
                }
                base += mysize;
                if (size <= mysize)
                        break;
                size -= mysize;
        }
-
-       if (first_base != (uint64_t) (-1))      //wanted atom inside not found
-       {
+       if (first_base != (uint64_t)(-1)) {
                set_position(f, first_base);
                return 1;
-       } else
-               return 0;
+       }
+       /* wanted atom inside not found */
+       return 2;
 }
 
 struct membuffer {
@@ -1271,7 +1404,7 @@ static bool membuffer_transfer_from_file(struct membuffer *buf, struct mp4 *src,
 
        membuffer_write(buf, 0, bytes);
        bufptr = membuffer_get_ptr(buf);
-       if (read_data(src, bufptr + oldsize, bytes) != bytes) {
+       if (read_data(src, bufptr + oldsize, bytes) != 1) {
                free(buf->data);
                free(buf);
                return false;
@@ -1328,19 +1461,21 @@ static uint32_t fix_byte_order_32(uint32_t src)
 
 static void *modify_moov(struct mp4 *f, uint32_t *out_size)
 {
+       int ret;
        uint64_t total_base = f->moov_offset + 8;
        uint32_t total_size = (uint32_t) (f->moov_size - 8);
-
        uint64_t udta_offset, meta_offset, ilst_offset;
        uint32_t udta_size, meta_size, ilst_size;
-
        uint32_t new_ilst_size;
        void *new_ilst_buffer, *out_buffer;
-
        uint8_t *p_out;
        int32_t size_delta;
+       uint32_t tmp;
 
-       if (!find_atom_v2(f, total_base, total_size, "udta", 0, "meta")) {
+       ret = find_atom_v2(f, total_base, total_size, "udta", 0, "meta");
+       if (ret <= 0)
+               return NULL;
+       if (ret == 2) {
                struct membuffer *buf;
                void *new_udta_buffer;
                uint32_t new_udta_size;
@@ -1362,8 +1497,13 @@ static void *modify_moov(struct mp4 *f, uint32_t *out_size)
                return membuffer_detach(buf);
        }
        udta_offset = get_position(f);
-       udta_size = read_int32(f);
-       if (!find_atom_v2 (f, udta_offset + 8, udta_size - 8, "meta", 4, "ilst")) {
+       ret = read_int32(f, &udta_size);
+       if (ret <= 0)
+               return NULL;
+       ret = find_atom_v2(f, udta_offset + 8, udta_size - 8, "meta", 4, "ilst");
+       if (ret <= 0)
+               return NULL;
+       if (ret == 2) {
                struct membuffer *buf;
                void *new_meta_buffer;
                uint32_t new_meta_size;
@@ -1385,7 +1525,6 @@ static void *modify_moov(struct mp4 *f, uint32_t *out_size)
                        free(new_meta_buffer);
                        return NULL;
                }
-
                membuffer_write_atom(buf, "meta", new_meta_size,
                        new_meta_buffer);
                free(new_meta_buffer);
@@ -1394,12 +1533,16 @@ static void *modify_moov(struct mp4 *f, uint32_t *out_size)
                return membuffer_detach(buf);
        }
        meta_offset = get_position(f);
-       meta_size = read_int32(f);
+       ret = read_int32(f, &meta_size);
+       if (ret <= 0)
+               return NULL;
        /* shouldn't happen, find_atom_v2 above takes care of it */
        if (!find_atom(f, meta_offset + 12, meta_size - 12, "ilst"))
                return NULL;
        ilst_offset = get_position(f);
-       ilst_size = read_int32(f);
+       ret = read_int32(f, &ilst_size);
+       if (ret <= 0)
+               return NULL;
        if (!create_ilst(&f->meta, &new_ilst_buffer, &new_ilst_size))
                return NULL;
        size_delta = new_ilst_size - (ilst_size - 8);
@@ -1407,29 +1550,52 @@ static void *modify_moov(struct mp4 *f, uint32_t *out_size)
        out_buffer = para_malloc(*out_size);
        p_out = out_buffer;
        set_position(f, total_base);
-       read_data(f, p_out, (uint32_t) (udta_offset - total_base));
+       ret = read_data(f, p_out, udta_offset - total_base);
+       if (ret <= 0)
+               return NULL;
        p_out += (uint32_t) (udta_offset - total_base);
-       *(uint32_t *)p_out = fix_byte_order_32(read_int32(f) + size_delta);
+       ret = read_int32(f, &tmp);
+       if (ret <= 0)
+               return NULL;
+       *(uint32_t *)p_out = fix_byte_order_32(tmp + size_delta);
        p_out += 4;
-       read_data(f, p_out, 4);
+       ret = read_data(f, p_out, 4);
+       if (ret <= 0)
+               return NULL;
        p_out += 4;
-       read_data(f, p_out, (uint32_t) (meta_offset - udta_offset - 8));
+       ret = read_data(f, p_out, meta_offset - udta_offset - 8);
+       if (ret <= 0)
+               return NULL;
        p_out += (uint32_t) (meta_offset - udta_offset - 8);
-       *(uint32_t *)p_out = fix_byte_order_32(read_int32(f) + size_delta);
+       ret = read_int32(f, &tmp);
+       if (ret <= 0)
+               return NULL;
+       *(uint32_t *)p_out = fix_byte_order_32(tmp + size_delta);
        p_out += 4;
-       read_data(f, p_out, 4);
+       ret = read_data(f, p_out, 4);
+       if (ret <= 0)
+               return NULL;
        p_out += 4;
-       read_data(f, p_out, (uint32_t) (ilst_offset - meta_offset - 8));
+       ret = read_data(f, p_out, ilst_offset - meta_offset - 8);
+       if (ret <= 0)
+               return NULL;
        p_out += (uint32_t) (ilst_offset - meta_offset - 8);
-       *(uint32_t *)p_out = fix_byte_order_32(read_int32(f) + size_delta);
+       ret = read_int32(f, &tmp);
+       if (ret <= 0)
+               return NULL;
+       *(uint32_t *)p_out = fix_byte_order_32(tmp + size_delta);
        p_out += 4;
-       read_data(f, p_out, 4);
+       ret = read_data(f, p_out, 4);
+       if (ret <= 0)
+               return NULL;
        p_out += 4;
        memcpy(p_out, new_ilst_buffer, new_ilst_size);
        p_out += new_ilst_size;
        set_position(f, ilst_offset + ilst_size);
-       read_data(f, p_out, (uint32_t) (total_size
-               - (ilst_offset - total_base) - ilst_size));
+       ret = read_data(f, p_out, total_size
+               - (ilst_offset - total_base) - ilst_size);
+       if (ret <= 0)
+               return NULL;
        free(new_ilst_buffer);
        return out_buffer;
 }

diff --git a/mp4.h b/mp4.h
index 1d48174af6cd30635a31522f393285068a580de9..9ff6f95ece00dd2ddd6e278f2c8a6302d8a5b9a6 100644 (file)
--- a/mp4.h
+++ b/mp4.h
@@ -1,5 +1,5 @@
 struct mp4_callback {
-    uint32_t (*read)(void *user_data, void *buffer, uint32_t length);
+    ssize_t (*read)(void *user_data, void *buffer, size_t length);
     uint32_t (*write)(void *udata, void *buffer, uint32_t length);
     uint32_t (*seek)(void *user_data, uint64_t position);
     uint32_t (*truncate)(void *user_data);