Re: [FFmpeg-devel] [PATCH] avformat/nutdec: Add check for avformat_new_stream

Re: [FFmpeg-devel] [PATCH] avformat/nutdec: Add check for avformat_new_stream

[Jiasheng Jiang]

Andreas Rheinhardt:
>> As the potential failure of the memory allocation,
>> the avformat_new_stream() could return NULL pointer.
>> Therefore, it should be better to check it and return
>> error if fails.
>> 
>> Fixes: 84ad31ff18 ("lavf: replace av_new_stream->avformat_new_stream part II.")
>
> This commit did not introduce this bug; it merely replaced the unchecked
> function.
>
> If you look at nut_read_header() you will see that it just retries even
> on allocation failure. So this is not a complete fix. And if it retries
> and finds a different packet header, it adds ever more streams, because
> the already created streams have not been deleted. A proper fix would
> need to check the return value of decode_main_header for ENOMEM, but if
> time_base_count were invalid and huge, one could get an allocation error
> even though there might be a valid header somewhere else. So one would
> need an equivalent of NUT_MAX_STREAMS for timebases or some other
> criterion to rule this out.

Fine, I have submit a v2 to fix the problems above.

Jiang

_______________________________________________
ffmpeg-devel mailing list
ffmpeg-devel@ffmpeg.org
https://ffmpeg.org/mailman/listinfo/ffmpeg-devel

To unsubscribe, visit link above, or email
ffmpeg-devel-request@ffmpeg.org with subject "unsubscribe".

Re: [FFmpeg-devel] [PATCH] avformat/nutdec: Add check for avformat_new_stream

[Andreas Rheinhardt]

Jiasheng Jiang:
> As the potential failure of the memory allocation,
> the avformat_new_stream() could return NULL pointer.
> Therefore, it should be better to check it and return
> error if fails.
> 
> Fixes: 84ad31ff18 ("lavf: replace av_new_stream->avformat_new_stream part II.")

This commit did not introduce this bug; it merely replaced the unchecked
function.

> Signed-off-by: Jiasheng Jiang <jiasheng@iscas.ac.cn>
> ---
>  libavformat/nutdec.c | 6 +++++-
>  1 file changed, 5 insertions(+), 1 deletion(-)
> 
> diff --git a/libavformat/nutdec.c b/libavformat/nutdec.c
> index 0a8a700acf..eb2ba4840a 100644
> --- a/libavformat/nutdec.c
> +++ b/libavformat/nutdec.c
> @@ -352,7 +352,11 @@ static int decode_main_header(NUTContext *nut)
>          goto fail;
>      }
>      for (i = 0; i < stream_count; i++)
> -        avformat_new_stream(s, NULL);
> +        if (!avformat_new_stream(s, NULL)) {
> +            av_free(nut->stream);
> +            ret = AVERROR(ENOMEM);
> +            goto fail;
> +        }
>  
>      return 0;
>  fail:

If you look at nut_read_header() you will see that it just retries even
on allocation failure. So this is not a complete fix. And if it retries
and finds a different packet header, it adds ever more streams, because
the already created streams have not been deleted. A proper fix would
need to check the return value of decode_main_header for ENOMEM, but if
time_base_count were invalid and huge, one could get an allocation error
even though there might be a valid header somewhere else. So one would
need an equivalent of NUT_MAX_STREAMS for timebases or some other
criterion to rule this out.

- Andreas
_______________________________________________
ffmpeg-devel mailing list
ffmpeg-devel@ffmpeg.org
https://ffmpeg.org/mailman/listinfo/ffmpeg-devel

To unsubscribe, visit link above, or email
ffmpeg-devel-request@ffmpeg.org with subject "unsubscribe".

[FFmpeg-devel] [PATCH] avformat/nutdec: Add check for avformat_new_stream

[Jiasheng Jiang]

As the potential failure of the memory allocation,
the avformat_new_stream() could return NULL pointer.
Therefore, it should be better to check it and return
error if fails.

Fixes: 84ad31ff18 ("lavf: replace av_new_stream->avformat_new_stream part II.")
Signed-off-by: Jiasheng Jiang <jiasheng@iscas.ac.cn>
---
 libavformat/nutdec.c | 6 +++++-
 1 file changed, 5 insertions(+), 1 deletion(-)

diff --git a/libavformat/nutdec.c b/libavformat/nutdec.c
index 0a8a700acf..eb2ba4840a 100644
--- a/libavformat/nutdec.c
+++ b/libavformat/nutdec.c
@@ -352,7 +352,11 @@ static int decode_main_header(NUTContext *nut)
         goto fail;
     }
     for (i = 0; i < stream_count; i++)
-        avformat_new_stream(s, NULL);
+        if (!avformat_new_stream(s, NULL)) {
+            av_free(nut->stream);
+            ret = AVERROR(ENOMEM);
+            goto fail;
+        }
 
     return 0;
 fail:
-- 
2.25.1

_______________________________________________
ffmpeg-devel mailing list
ffmpeg-devel@ffmpeg.org
https://ffmpeg.org/mailman/listinfo/ffmpeg-devel

To unsubscribe, visit link above, or email
ffmpeg-devel-request@ffmpeg.org with subject "unsubscribe".