| Message ID | 20211201083752.66991-1-lq@chinaffmpeg.org |
|---|---|
| State | New |
| Headers | show |
| Series | [FFmpeg-devel] fftools/ffmpeg_filter: fix SEGV in choose_pix_fmts after avio_close_dyn_buf | expand |
| Context | Check | Description |
|---|---|---|
| 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 |
Quoting Steven Liu (2021-12-01 09:37:52) > From: Steven Liu <liuqi05@kuaishou.com> > > ret could be set to s->opaque->buffer in avio_close_dyn_buf, so it can > be set to NULL, check NULL pointer deference after it should be ok. I don't understand this reasoning. avio_close_dyn_buf() should produce a non-NULL output buffer if something was written into it. Since this branch is only taken when (enc->pix_fmts != NULL), something should always be written, so the output buffer should be non-NULL and have a non-zero size. Under what circumstances will it happen that the output is NULL?
Anton Khirnov <anton@khirnov.net> 于2021年12月1日周三 下午6:21写道： > > Quoting Steven Liu (2021-12-01 09:37:52) > > From: Steven Liu <liuqi05@kuaishou.com> > > > > ret could be set to s->opaque->buffer in avio_close_dyn_buf, so it can > > be set to NULL, check NULL pointer deference after it should be ok. > > I don't understand this reasoning. avio_close_dyn_buf() should produce > a non-NULL output buffer if something was written into it. Since this > branch is only taken when (enc->pix_fmts != NULL), something should > always be written, so the output buffer should be non-NULL and have a > non-zero size. > > Under what circumstances will it happen that the output is NULL? Hi Anton, This is the message: ffmpeg version 4.4.1 Copyright (c) 2000-2021 the FFmpeg developers built with clang version 10.0.0-4ubuntu1 configuration: --enable-gpl --enable-nonfree --disable-doc --disable-stripping --disable-asm --disable-optimizations --cc=/home/r1/src_git/mfuzz-project/build/bin/mclangwrapper --cxx=/home/r1/src_git/mfuzz-project/build/bin/mclangwrapper++ libavutil 56. 70.100 / 56. 70.100 libavcodec 58.134.100 / 58.134.100 libavformat 58. 76.100 / 58. 76.100 libavdevice 58. 13.100 / 58. 13.100 libavfilter 7.110.100 / 7.110.100 libswscale 5. 9.100 / 5. 9.100 libswresample 3. 9.100 / 3. 9.100 libpostproc 55. 9.100 / 55. 9.100 Input #0, mov,mp4,m4a,3gp,3g2,mj2, from './file': Metadata: major_brand : isom minor_version : 512 compatible_brands: isomiso2mp41 encoder : Lavf58.76.100 Duration: 00:00:02.00, start: 0.000000, bitrate: 113 kb/s Stream #0:0(und): Video: mpeg4 (Simple Profile) (mp4v / 0x7634706D), yuv420p(tv, bt709), 640x480 [SAR 6:5 DAR 8:5], 109 kb/s, 3 fps, 3 tbr, 12288 tbn, 3 tbc (default) Metadata: handler_name : VideoHandler vendor_id : [0][0][0][0] Stream mapping: Stream #0:0 -> #0:0 (mpeg4 (native) -> mpeg4 (native)) Press [q] to stop, [?] for help faultinject: error actived 1! name: realloc, file: src/libavutil/mem.c, line: 142 ??:? /home/r1/src_git/mfuzz-project/wrapper/mfuzzrt/faultinject.c:44 /home/r1/ffmpeg/ffmpeg-4.4.1/build/src/libavutil/mem.c:142 /home/r1/ffmpeg/ffmpeg-4.4.1/build/src/libavutil/mem.c:171 /home/r1/ffmpeg/ffmpeg-4.4.1/build/src/libavformat/aviobuf.c:1312 /home/r1/ffmpeg/ffmpeg-4.4.1/build/src/libavformat/aviobuf.c:170 /home/r1/ffmpeg/ffmpeg-4.4.1/build/src/libavformat/aviobuf.c:191 /home/r1/ffmpeg/ffmpeg-4.4.1/build/src/libavformat/aviobuf.c:248 /home/r1/ffmpeg/ffmpeg-4.4.1/build/src/libavformat/aviobuf.c:1442 /home/r1/ffmpeg/ffmpeg-4.4.1/build/src/fftools/ffmpeg_filter.c:129 /home/r1/ffmpeg/ffmpeg-4.4.1/build/src/fftools/ffmpeg_filter.c:476 /home/r1/ffmpeg/ffmpeg-4.4.1/build/src/fftools/ffmpeg_filter.c:657 /home/r1/ffmpeg/ffmpeg-4.4.1/build/src/fftools/ffmpeg_filter.c:1076 /home/r1/ffmpeg/ffmpeg-4.4.1/build/src/fftools/ffmpeg.c:2237 /home/r1/ffmpeg/ffmpeg-4.4.1/build/src/fftools/ffmpeg.c:2318 /home/r1/ffmpeg/ffmpeg-4.4.1/build/src/fftools/ffmpeg.c:2515 /home/r1/ffmpeg/ffmpeg-4.4.1/build/src/fftools/ffmpeg.c:2675 /home/r1/ffmpeg/ffmpeg-4.4.1/build/src/fftools/ffmpeg.c:4396 /home/r1/ffmpeg/ffmpeg-4.4.1/build/src/fftools/ffmpeg.c:4751 /home/r1/ffmpeg/ffmpeg-4.4.1/build/src/fftools/ffmpeg.c:4805 /home/r1/ffmpeg/ffmpeg-4.4.1/build/src/fftools/ffmpeg.c:5010 /lib/x86_64-linux-gnu/libc.so.6(__libc_start_main+0xf3)[0x7ff6ec5900b3] ??:? AddressSanitizer:DEADLYSIGNAL ================================================================= ==2784535==ERROR: AddressSanitizer: SEGV on unknown address (pc 0x00000055552e bp 0x7fffaec75a90 sp 0x7fffaec75780 T0) ==2784535==The signal is caused by a READ memory access. ==2784535==Hint: this fault was caused by a dereference of a high value address (see register values below). Dissassemble the provided pc to learn which register was used. #0 0x55552e in choose_pix_fmts /home/r1/ffmpeg/ffmpeg-4.4.1/build/src/fftools/ffmpeg_filter.c:130:22 #1 0x54f58a in configure_output_video_filter /home/r1/ffmpeg/ffmpeg-4.4.1/build/src/fftools/ffmpeg_filter.c:476:21 #2 0x5442b0 in configure_output_filter /home/r1/ffmpeg/ffmpeg-4.4.1/build/src/fftools/ffmpeg_filter.c:657:37 #3 0x541012 in configure_filtergraph /home/r1/ffmpeg/ffmpeg-4.4.1/build/src/fftools/ffmpeg_filter.c:1076:9 #4 0x6033de in ifilter_send_frame /home/r1/ffmpeg/ffmpeg-4.4.1/build/src/fftools/ffmpeg.c:2237:15 #5 0x6018e3 in send_frame_to_filters /home/r1/ffmpeg/ffmpeg-4.4.1/build/src/fftools/ffmpeg.c:2318:15 #6 0x5fa778 in decode_video /home/r1/ffmpeg/ffmpeg-4.4.1/build/src/fftools/ffmpeg.c:2515:11 #7 0x5a5c8b in process_input_packet /home/r1/ffmpeg/ffmpeg-4.4.1/build/src/fftools/ffmpeg.c:2675:19 #8 0x5d281e in process_input /home/r1/ffmpeg/ffmpeg-4.4.1/build/src/fftools/ffmpeg.c:4396:23 #9 0x59f1cf in transcode_step /home/r1/ffmpeg/ffmpeg-4.4.1/build/src/fftools/ffmpeg.c:4751:11 #10 0x593970 in transcode /home/r1/ffmpeg/ffmpeg-4.4.1/build/src/fftools/ffmpeg.c:4805:15 #11 0x58f7a4 in main /home/r1/ffmpeg/ffmpeg-4.4.1/build/src/fftools/ffmpeg.c:5010:9 #12 0x7ff6ec5900b2 in __libc_start_main /build/glibc-eX1tMB/glibc-2.31/csu/../csu/libc-start.c:308:16 #13 0x42033d in _start (/home/r1/ffmpeg/ffmpeg_4.4.1+0x42033d) AddressSanitizer can not provide additional info. SUMMARY: AddressSanitizer: SEGV /home/r1/ffmpeg/ffmpeg-4.4.1/build/src/fftools/ffmpeg_filter.c:130:22 in choose_pix_fmts ```c // in libavformat/aviobuf.c:1294 1294 static int dyn_buf_write(void *opaque, uint8_t *buf, int buf_size) 1295 { ... // av_reallocp() could set `opaque->buffer = NULL`; see below; 1312 if ((err = av_reallocp(&d->buffer, new_allocated_size)) < 0) { ... 1316 } ... 1324 } ``` ```c // in libavutil/mem.c:161 161 int av_reallocp(void *ptr, size_t size) 162 { ... 171 val = av_realloc(val, size); 172 173 if (!val) { // av_freep() could set `ptr = NULL`; 174 av_freep(ptr); 175 return AVERROR(ENOMEM); 176 } ... 180 } ``` ```c // in fftools/ffmpeg_filter.c:94 94 static char *choose_pix_fmts(OutputFilter *ofilter) 95 { ... 114 uint8_t *ret; ... // `avio_close_dyn_buf()` set `ret = s->opaque->buffer`, so ret may be NULL; 129 len = avio_close_dyn_buf(s, &ret); // NULL pointer deference. 130 ret[len - 1] = 0; ... 134 } ``` Thanks Steven
Quoting Steven Liu (2021-12-01 11:27:41) > Anton Khirnov <anton@khirnov.net> 于2021年12月1日周三 下午6:21写道： > > > > Quoting Steven Liu (2021-12-01 09:37:52) > > > From: Steven Liu <liuqi05@kuaishou.com> > > > > > > ret could be set to s->opaque->buffer in avio_close_dyn_buf, so it can > > > be set to NULL, check NULL pointer deference after it should be ok. > > > > I don't understand this reasoning. avio_close_dyn_buf() should produce > > a non-NULL output buffer if something was written into it. Since this > > branch is only taken when (enc->pix_fmts != NULL), something should > > always be written, so the output buffer should be non-NULL and have a > > non-zero size. > > > > Under what circumstances will it happen that the output is NULL? > > ```c > // in libavformat/aviobuf.c:1294 > 1294 static int dyn_buf_write(void *opaque, uint8_t *buf, int buf_size) > 1295 { > ... > // av_reallocp() could set `opaque->buffer = NULL`; see below; > 1312 if ((err = av_reallocp(&d->buffer, new_allocated_size)) < 0) { So the problem is that memory allocation fails and nothing gets written in the buffer? Then it seems more correct to check the return value of avio_printf().
diff --git a/fftools/ffmpeg_filter.c b/fftools/ffmpeg_filter.c index 452b689d62..f2f77ff86f 100644 --- a/fftools/ffmpeg_filter.c +++ b/fftools/ffmpeg_filter.c @@ -119,6 +119,8 @@ static char *choose_pix_fmts(OutputFilter *ofilter) avio_printf(s, "%s|", name); } len = avio_close_dyn_buf(s, &ret); + if (len <= 0 || !ret) + return NULL; ret[len - 1] = 0; return ret; } else