diff mbox

[FFmpeg-devel,06/10] segment.c: Add allocation failure checks.

Message ID 20180528182711.3221-7-klaxa1337@googlemail.com
State Superseded
Headers show

Commit Message

Stephan Holljes May 28, 2018, 6:27 p.m. UTC 
Signed-off-by: Stephan Holljes <klaxa1337@googlemail.com>
---
 segment.c | 26 ++++++++++++++++++++++++++
 1 file changed, 26 insertions(+)

Comments

Michael Niedermayer May 28, 2018, 9:45 p.m. UTC | #1 
On Mon, May 28, 2018 at 08:27:07PM +0200, Stephan Holljes wrote:
> Signed-off-by: Stephan Holljes <klaxa1337@googlemail.com>
> ---
>  segment.c | 26 ++++++++++++++++++++++++++
>  1 file changed, 26 insertions(+)
> 
> diff --git a/segment.c b/segment.c
> index c40d1ad..986aeb5 100644
> --- a/segment.c
> +++ b/segment.c
> @@ -82,6 +82,10 @@ int segment_write(void *opaque, unsigned char *buf, int buf_size)
>      struct Segment *seg = (struct Segment*) opaque;
>      seg->size += buf_size;
>      seg->buf = (unsigned char*) av_realloc(seg->buf, seg->size);
> +    if (!seg->buf) {
> +        av_log(NULL, AV_LOG_ERROR, "Could not grow segment.\n");
> +        return AVERROR(ENOMEM);
> +    }
>      memcpy(seg->buf + seg->size - buf_size, buf, buf_size);
>      return buf_size;
>  }
> @@ -110,6 +114,10 @@ void segment_init(struct Segment **seg_p, AVFormatContext *fmt)
>      int i;
>      AVStream *in_stream, *out_stream;
>      struct Segment *seg = (struct Segment*) av_malloc(sizeof(struct Segment));
> +    if (!seg) {
> +        av_log(fmt, AV_LOG_ERROR, "Could not allocate segment.\n");
> +        return;
> +    }

i didnt look at the surrounding code but just from the patch 
segment_init fails and its void, not returning an error, that seems odd
does this work ?


[...]
Stephan Holljes May 28, 2018, 11:37 p.m. UTC | #2 
On Mon, May 28, 2018 at 11:45 PM, Michael Niedermayer
<michael@niedermayer.cc> wrote:
> On Mon, May 28, 2018 at 08:27:07PM +0200, Stephan Holljes wrote:
>> Signed-off-by: Stephan Holljes <klaxa1337@googlemail.com>
>> ---
>>  segment.c | 26 ++++++++++++++++++++++++++
>>  1 file changed, 26 insertions(+)
>>
>> diff --git a/segment.c b/segment.c
>> index c40d1ad..986aeb5 100644
>> --- a/segment.c
>> +++ b/segment.c
>> @@ -82,6 +82,10 @@ int segment_write(void *opaque, unsigned char *buf, int buf_size)
>>      struct Segment *seg = (struct Segment*) opaque;
>>      seg->size += buf_size;
>>      seg->buf = (unsigned char*) av_realloc(seg->buf, seg->size);
>> +    if (!seg->buf) {
>> +        av_log(NULL, AV_LOG_ERROR, "Could not grow segment.\n");
>> +        return AVERROR(ENOMEM);
>> +    }
>>      memcpy(seg->buf + seg->size - buf_size, buf, buf_size);
>>      return buf_size;
>>  }
>> @@ -110,6 +114,10 @@ void segment_init(struct Segment **seg_p, AVFormatContext *fmt)
>>      int i;
>>      AVStream *in_stream, *out_stream;
>>      struct Segment *seg = (struct Segment*) av_malloc(sizeof(struct Segment));
>> +    if (!seg) {
>> +        av_log(fmt, AV_LOG_ERROR, "Could not allocate segment.\n");
>> +        return;
>> +    }
>
> i didnt look at the surrounding code but just from the patch
> segment_init fails and its void, not returning an error, that seems odd
> does this work ?

Indeed, this would break as soon as ffserver.c calls seg->id = id++;
I think not being able to allocate a segment is fatal enough to
gracefully shut down the server-thread that depends on it. Checking
the allocated segment against NULL should be sufficient (if *seg_p is
set to NULL in the if() checking if seg is NULL).

Thanks, I will read up more on setrlimit() and friends to try to test
for these allocation failures.

>
>
> [...]
> --
> Michael     GnuPG fingerprint: 9FF2128B147EF6730BADF133611EC787040B0FAB
>
> No human being will ever know the Truth, for even if they happen to say it
> by chance, they would not even known they had done so. -- Xenophanes
>
> _______________________________________________
> ffmpeg-devel mailing list
> ffmpeg-devel@ffmpeg.org
> http://ffmpeg.org/mailman/listinfo/ffmpeg-devel
>
diff mbox

Patch

diff --git a/segment.c b/segment.c
index c40d1ad..986aeb5 100644
--- a/segment.c
+++ b/segment.c
@@ -82,6 +82,10 @@  int segment_write(void *opaque, unsigned char *buf, int buf_size)
     struct Segment *seg = (struct Segment*) opaque;
     seg->size += buf_size;
     seg->buf = (unsigned char*) av_realloc(seg->buf, seg->size);
+    if (!seg->buf) {
+        av_log(NULL, AV_LOG_ERROR, "Could not grow segment.\n");
+        return AVERROR(ENOMEM);
+    }
     memcpy(seg->buf + seg->size - buf_size, buf, buf_size);
     return buf_size;
 }
@@ -110,6 +114,10 @@  void segment_init(struct Segment **seg_p, AVFormatContext *fmt)
     int i;
     AVStream *in_stream, *out_stream;
     struct Segment *seg = (struct Segment*) av_malloc(sizeof(struct Segment));
+    if (!seg) {
+        av_log(fmt, AV_LOG_ERROR, "Could not allocate segment.\n");
+        return;
+    }
 
     seg->ifmt = av_find_input_format("matroska");
     seg->fmt_ctx = NULL;
@@ -119,10 +127,28 @@  void segment_init(struct Segment **seg_p, AVFormatContext *fmt)
     seg->ts_len = 0;
     seg->buf = NULL;
     seg->avio_buffer = (unsigned char*) av_malloc(AV_BUFSIZE);
+    if (!seg->avio_buffer) {
+        av_log(NULL, AV_LOG_ERROR, "Could not allocate segment avio_buffer.\n");
+        av_free(seg);
+        return;
+    }
     pthread_mutex_init(&seg->nb_read_lock, NULL);
     seg->io_ctx = avio_alloc_context(seg->avio_buffer, AV_BUFSIZE, 1, seg, NULL, &segment_write, NULL);
+    if (!seg->io_ctx) {
+        av_log(NULL, AV_LOG_ERROR, "Could not allocate segment io context.\n");
+        av_free(seg->avio_buffer);
+        av_free(seg);
+        return;
+    }
     seg->io_ctx->seekable = 0;
     avformat_alloc_output_context2(&seg->fmt_ctx, NULL, "matroska", NULL);
+    if (!seg->fmt_ctx) {
+        av_log(seg->io_ctx, AV_LOG_ERROR, "Could not allocate segment output context.\n");
+        av_free(seg->avio_buffer);
+        av_free(seg->io_ctx);
+        av_free(seg);
+        return;
+    }
     if ((ret = av_opt_set_int(seg->fmt_ctx, "flush_packets", 1, AV_OPT_SEARCH_CHILDREN)) < 0) {
         av_log(seg->fmt_ctx, AV_LOG_WARNING, "Could not set flush_packets!\n");
     }