eden-emu/FFmpeg
20
0
Fork 0
Code Issues Pull requests Projects Releases Packages Wiki Activity Actions

avformat/matroskadec: Improve read error/EOF checks II

Browse source 
This commit fixes a number of bugs:

1. There was no check that no read error/EOF occured during
ebml_read_uint, ebml_read_sint and ebml_read_float.
2. ebml_read_ascii and ebml_read_binary did sometimes not forward
error codes; instead they simply returned AVERROR(EIO).
3. In particular, AVERROR_EOF hasn't been used and no dedicated error
message for it existed. This has been changed.

In order to reduce code duplication, the new error code NEEDS_CHECKING
has been introduced which makes ebml_parse check the AVIOContext's
status for errors.

Signed-off-by: Andreas Rheinhardt <andreas.rheinhardt@gmail.com>
This commit is contained in:
Andreas Rheinhardt 2019-06-24 01:42:30 +02:00 • committed by James Almer
parent 239c7369e0
commit a569a7b3bb
1 changed files with 41 additions and 18 deletions
Download patch file Download diff file Expand all files Collapse all files

59
libavformat/matroskadec.c
View file

@ -69,6 +69,8 @@
#include "qtpalette.h" #include "qtpalette.h"
#define EBML_UNKNOWN_LENGTH UINT64_MAX /* EBML unknown length, in uint64_t */ #define EBML_UNKNOWN_LENGTH UINT64_MAX /* EBML unknown length, in uint64_t */
#define NEEDS_CHECKING 2 /* Indicates that some error checks
* still need to be performed */
typedef enum { typedef enum {
EBML_NONE, EBML_NONE,
@ -871,7 +873,7 @@ static int ebml_read_length(MatroskaDemuxContext *matroska, AVIOContext *pb,
/* /*
* Read the next element as an unsigned int. * Read the next element as an unsigned int.
* 0 is success, < 0 is failure. * Returns NEEDS_CHECKING.
*/ */
static int ebml_read_uint(AVIOContext *pb, int size, uint64_t *num) static int ebml_read_uint(AVIOContext *pb, int size, uint64_t *num)
{ {
@ -882,12 +884,12 @@ static int ebml_read_uint(AVIOContext *pb, int size, uint64_t *num)
while (n++ < size) while (n++ < size)
*num = (*num << 8) | avio_r8(pb); *num = (*num << 8) | avio_r8(pb);
return 0; return NEEDS_CHECKING;
} }
/* /*
* Read the next element as a signed int. * Read the next element as a signed int.
* 0 is success, < 0 is failure. * Returns NEEDS_CHECKING.
*/ */
static int ebml_read_sint(AVIOContext *pb, int size, int64_t *num) static int ebml_read_sint(AVIOContext *pb, int size, int64_t *num)
{ {
@ -903,12 +905,12 @@ static int ebml_read_sint(AVIOContext *pb, int size, int64_t *num)
*num = ((uint64_t)*num << 8) | avio_r8(pb); *num = ((uint64_t)*num << 8) | avio_r8(pb);
} }
return 0; return NEEDS_CHECKING;
} }
/* /*
* Read the next element as a float. * Read the next element as a float.
* 0 is success, < 0 is failure. * Returns NEEDS_CHECKING or < 0 on obvious failure.
*/ */
static int ebml_read_float(AVIOContext *pb, int size, double *num) static int ebml_read_float(AVIOContext *pb, int size, double *num)
{ {
@ -921,24 +923,25 @@ static int ebml_read_float(AVIOContext *pb, int size, double *num)
else else
return AVERROR_INVALIDDATA; return AVERROR_INVALIDDATA;
return 0; return NEEDS_CHECKING;
} }
/* /*
* Read the next element as an ASCII string. * Read the next element as an ASCII string.
* 0 is success, < 0 is failure. * 0 is success, < 0 or NEEDS_CHECKING is failure.
*/ */
static int ebml_read_ascii(AVIOContext *pb, int size, char **str) static int ebml_read_ascii(AVIOContext *pb, int size, char **str)
{ {
char *res; char *res;
int ret;
/* EBML strings are usually not 0-terminated, so we allocate one /* EBML strings are usually not 0-terminated, so we allocate one
* byte more, read the string and NULL-terminate it ourselves. */ * byte more, read the string and NULL-terminate it ourselves. */
if (!(res = av_malloc(size + 1))) if (!(res = av_malloc(size + 1)))
return AVERROR(ENOMEM); return AVERROR(ENOMEM);
if (avio_read(pb, (uint8_t *) res, size) != size) { if ((ret = avio_read(pb, (uint8_t *) res, size)) != size) {
av_free(res); av_free(res);
return AVERROR(EIO); return ret < 0 ? ret : NEEDS_CHECKING;
} }
(res)[size] = '\0'; (res)[size] = '\0';
av_free(*str); av_free(*str);
@ -949,7 +952,7 @@ static int ebml_read_ascii(AVIOContext *pb, int size, char **str)
/* /*
* Read the next element as binary data. * Read the next element as binary data.
* 0 is success, < 0 is failure. * 0 is success, < 0 or NEEDS_CHECKING is failure.
*/ */
static int ebml_read_binary(AVIOContext *pb, int length, EbmlBin *bin) static int ebml_read_binary(AVIOContext *pb, int length, EbmlBin *bin)
{ {
@ -963,11 +966,11 @@ static int ebml_read_binary(AVIOContext *pb, int length, EbmlBin *bin)
bin->data = bin->buf->data; bin->data = bin->buf->data;
bin->size = length; bin->size = length;
bin->pos = avio_tell(pb); bin->pos = avio_tell(pb);
if (avio_read(pb, bin->data, length) != length) { if ((ret = avio_read(pb, bin->data, length)) != length) {
av_buffer_unref(&bin->buf); av_buffer_unref(&bin->buf);
bin->data = NULL; bin->data = NULL;
bin->size = 0; bin->size = 0;
return AVERROR(EIO); return ret < 0 ? ret : NEEDS_CHECKING;
} }
return 0; return 0;
@ -1255,14 +1258,34 @@ static int ebml_parse_elem(MatroskaDemuxContext *matroska,
case EBML_STOP: case EBML_STOP:
return 1; return 1;
default: default:
if (ffio_limit(pb, length) != length) if (ffio_limit(pb, length) != length) {
// ffio_limit emits its own error message,
// so we don't have to.
return AVERROR(EIO); return AVERROR(EIO);
return avio_skip(pb, length) < 0 ? AVERROR(EIO) : 0; }
res = avio_skip(pb, length);
res = res < 0 ? res : 0;
}
if (res) {
if (res == NEEDS_CHECKING) {
if (pb->eof_reached) {
if (pb->error)
res = pb->error;
else
res = AVERROR_EOF;
} else
res = 0;
}
if (res == AVERROR_INVALIDDATA)
av_log(matroska->ctx, AV_LOG_ERROR, "Invalid element\n");
else if (res == AVERROR(EIO))
av_log(matroska->ctx, AV_LOG_ERROR, "Read error\n");
else if (res == AVERROR_EOF) {
av_log(matroska->ctx, AV_LOG_ERROR, "File ended prematurely\n");
res = AVERROR(EIO);
}
} }
if (res == AVERROR_INVALIDDATA)
av_log(matroska->ctx, AV_LOG_ERROR, "Invalid element\n");
else if (res == AVERROR(EIO))
av_log(matroska->ctx, AV_LOG_ERROR, "Read error\n");
return res; return res;
} }