| Message ID | 20220216070506.1834664-1-jiasheng@iscas.ac.cn |
|---|---|
| State | New |
| Headers | show |
| Series | [FFmpeg-devel] avformat/nutdec: Add check for avformat_new_stream | expand |
| Context | Check | Description |
|---|---|---|
| yinshiyou/make_loongarch64 | success | Make finished |
| yinshiyou/make_fate_loongarch64 | success | Make fate finished |
| andriy/make_aarch64_jetson | success | Make finished |
| andriy/make_fate_aarch64_jetson | success | Make fate finished |
| andriy/make_armv7_RPi4 | success | Make finished |
| andriy/make_fate_armv7_RPi4 | success | Make fate finished |
| andriy/make_x86 | success | Make finished |
| andriy/make_fate_x86 | success | Make fate finished |
| andriy/make_ppc | success | Make finished |
| andriy/make_fate_ppc | success | Make fate finished |
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
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:
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(-)