| Message ID | 20211203093911.65914-1-young_chelsea@163.com |
|---|---|
| State | New |
| Headers | show |
| Series | [FFmpeg-devel] libavcodec/avpacketc packet release exception | expand |
| Context | Check | Description |
|---|---|---|
| andriy/commit_msg_x86 | warning | The first line of the commit message must start with a context terminated by a colon and a space, for example "lavu/opt: " or "doc: ". |
| andriy/make_x86 | success | Make finished |
| andriy/make_fate_x86 | success | Make fate finished |
| andriy/commit_msg_ppc | warning | The first line of the commit message must start with a context terminated by a colon and a space, for example "lavu/opt: " or "doc: ". |
| andriy/make_ppc | success | Make finished |
| andriy/make_fate_ppc | success | Make fate finished |
> 2021年12月3日 下午5:45，Andreas Rheinhardt <andreas.rheinhardt@outlook.com> 写道： > > Yu Yang: >> 'pkt' and '*pkt' should be judged separately for release. >> SEGV by a READ memory access (address points to the zero page) >> >> ```c >> // in fftools/ffmpeg.c:515 >> 515 static void ffmpeg_cleanup(int ret) >> 516 { >> ... >> >> 626 for (i = 0; i < nb_input_files; i++) { >> 627 avformat_close_input(&input_files[i]->ctx); >> // `input_files[0] == NULL` but `&input_files[0]->pkt == 0x68`, see below; >> 628 av_packet_free(&input_files[i]->pkt); >> 629 av_freep(&input_files[i]); >> 630 } >> >> ... >> 674 } >> ``` >> ```c >> // in libavcodec/avpacket.c:75 >> 75 void av_packet_free(AVPacket **pkt) >> 76 { >> // pkt == 0x68, `*pkt` cause `SEGV`. >> 77 if (!pkt || !*pkt) >> 78 return; >> 79 >> 80 av_packet_unref(*pkt); >> 81 av_freep(pkt); >> 82 } >> ``` >> >> coredump backtrace info: >> ==4536==ERROR: AddressSanitizer: SEGV on unknown address 0x000000000068 (pc 0x000002a794d0 bp 0x7ffdf587a910 sp 0x7ffdf587a8e0 T0) >> ==4536==The signal is caused by a READ memory access. >> ==4536==Hint: address points to the zero page. >> #0 0x2a794d0 in av_packet_free /home/r1/ffmpeg/ffmpeg-4.4.1/build/src/libavcodec/avpacket.c:77:18 >> #1 0x592107 in ffmpeg_cleanup /home/r1/ffmpeg/ffmpeg-4.4.1/build/src/fftools/ffmpeg.c:628:9 >> #2 0x55fe0e in exit_program /home/r1/ffmpeg/ffmpeg-4.4.1/build/src/fftools/cmdutils.c:136:9 >> #3 0x4cfcd4 in open_input_file /home/r1/ffmpeg/ffmpeg-4.4.1/build/src/fftools/ffmpeg_opt.c:1268:9 exit_program >> #4 0x4c9dc0 in open_files /home/r1/ffmpeg/ffmpeg-4.4.1/build/src/fftools/ffmpeg_opt.c:3338:15 >> #5 0x4c9295 in ffmpeg_parse_options /home/r1/ffmpeg/ffmpeg-4.4.1/build/src/fftools/ffmpeg_opt.c:3378:11 open_file >> #6 0x58f241 in main /home/r1/ffmpeg/ffmpeg-4.4.1/build/src/fftools/ffmpeg.c:4988:11 >> #7 0x7f122a83d0b2 in __libc_start_main /build/glibc-eX1tMB/glibc-2.31/csu/../csu/libc-start.c:308:16 >> #8 0x42033d in _start (/home/r1/ffmpeg/ffmpeg_4.4.1+0x42033d) >> >> >> Reported-by: TOTE Robot <oslab@tsinghua.edu.cn> >> Signed-off-by: Yu Yang <young_chelsea@163.com> >> --- >> libavcodec/avpacket.c | 9 ++++----- >> 1 file changed, 4 insertions(+), 5 deletions(-) >> >> diff --git a/libavcodec/avpacket.c b/libavcodec/avpacket.c >> index d8d8fef3b9..8348bec581 100644 >> --- a/libavcodec/avpacket.c >> +++ b/libavcodec/avpacket.c >> @@ -74,11 +74,10 @@ AVPacket *av_packet_alloc(void) >> >> void av_packet_free(AVPacket **pkt) >> { >> - if (!pkt || !*pkt) >> - return; >> - >> - av_packet_unref(*pkt); >> - av_freep(pkt); >> + if (*pkt) >> + av_packet_unref(*pkt); >> + if (pkt) >> + av_freep(pkt); >> } >> >> static int packet_alloc(AVBufferRef **buf, int size) >> > > This change makes no sense at all: If pkt == NULL (this should actually > never happen and indicates a bug somewhere else; we should actually not > check for this, but legacy), then the first check dereferences a NULL > pointer and crashes. Thx, the code I wrote is indeed too stupid. > > - 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".
diff --git a/libavcodec/avpacket.c b/libavcodec/avpacket.c index d8d8fef3b9..8348bec581 100644 --- a/libavcodec/avpacket.c +++ b/libavcodec/avpacket.c @@ -74,11 +74,10 @@ AVPacket *av_packet_alloc(void) void av_packet_free(AVPacket **pkt) { - if (!pkt || !*pkt) - return; - - av_packet_unref(*pkt); - av_freep(pkt); + if (*pkt) + av_packet_unref(*pkt); + if (pkt) + av_freep(pkt); } static int packet_alloc(AVBufferRef **buf, int size)
'pkt' and '*pkt' should be judged separately for release. SEGV by a READ memory access (address points to the zero page) ```c // in fftools/ffmpeg.c:515 515 static void ffmpeg_cleanup(int ret) 516 { ... 626 for (i = 0; i < nb_input_files; i++) { 627 avformat_close_input(&input_files[i]->ctx); // `input_files[0] == NULL` but `&input_files[0]->pkt == 0x68`, see below; 628 av_packet_free(&input_files[i]->pkt); 629 av_freep(&input_files[i]); 630 } ... 674 } ``` ```c // in libavcodec/avpacket.c:75 75 void av_packet_free(AVPacket **pkt) 76 { // pkt == 0x68, `*pkt` cause `SEGV`. 77 if (!pkt || !*pkt) 78 return; 79 80 av_packet_unref(*pkt); 81 av_freep(pkt); 82 } ``` coredump backtrace info: ==4536==ERROR: AddressSanitizer: SEGV on unknown address 0x000000000068 (pc 0x000002a794d0 bp 0x7ffdf587a910 sp 0x7ffdf587a8e0 T0) ==4536==The signal is caused by a READ memory access. ==4536==Hint: address points to the zero page. #0 0x2a794d0 in av_packet_free /home/r1/ffmpeg/ffmpeg-4.4.1/build/src/libavcodec/avpacket.c:77:18 #1 0x592107 in ffmpeg_cleanup /home/r1/ffmpeg/ffmpeg-4.4.1/build/src/fftools/ffmpeg.c:628:9 #2 0x55fe0e in exit_program /home/r1/ffmpeg/ffmpeg-4.4.1/build/src/fftools/cmdutils.c:136:9 #3 0x4cfcd4 in open_input_file /home/r1/ffmpeg/ffmpeg-4.4.1/build/src/fftools/ffmpeg_opt.c:1268:9 exit_program #4 0x4c9dc0 in open_files /home/r1/ffmpeg/ffmpeg-4.4.1/build/src/fftools/ffmpeg_opt.c:3338:15 #5 0x4c9295 in ffmpeg_parse_options /home/r1/ffmpeg/ffmpeg-4.4.1/build/src/fftools/ffmpeg_opt.c:3378:11 open_file #6 0x58f241 in main /home/r1/ffmpeg/ffmpeg-4.4.1/build/src/fftools/ffmpeg.c:4988:11 #7 0x7f122a83d0b2 in __libc_start_main /build/glibc-eX1tMB/glibc-2.31/csu/../csu/libc-start.c:308:16 #8 0x42033d in _start (/home/r1/ffmpeg/ffmpeg_4.4.1+0x42033d) Reported-by: TOTE Robot <oslab@tsinghua.edu.cn> Signed-off-by: Yu Yang <young_chelsea@163.com> --- libavcodec/avpacket.c | 9 ++++----- 1 file changed, 4 insertions(+), 5 deletions(-)