| Message ID | 4b90cd303402a9f87964aef883271c53c4ee8c78.camel@ammirata.net |
|---|---|
| State | New |
| Headers | show |
| Series | [FFmpeg-devel] Fix for PES packets with too much padding | expand |
| Context | Check | Description |
|---|---|---|
| andriy/x86_make | success | Make finished |
| andriy/x86_make_fate | success | Make fate finished |
| andriy/PPC64_make | success | Make finished |
| andriy/PPC64_make_fate | success | Make fate finished |
August 3, 2021 10:07 AM, "Sergio M. Ammirata, Ph.D." <sergio@ammirata.net> wrote: > PES packet with too much padding trigger unlimited error > messages "PES packet size mismatch" because the code that > corrects the length is wrong. > Here is a sample file: http://99.93.62.129/smpte2038.ts > PID 300 is the one triggering the errors. > I am attaching a patch that fixes the problem. > On the if statement right above your change, are you sure that the condition should still be buf_size > pes->total_size? Especially if the old change was just buf_size = pes->total_size (so clipping according to condition); and the new one is setting it to something different. thx for patch. > _______________________________________________ > 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".
On Tue, Aug 03, 2021 at 10:07:34AM -0400, Sergio M. Ammirata, Ph.D. wrote: > PES packet with too much padding trigger unlimited error > messages "PES packet size mismatch" because the code that > corrects the length is wrong. > Here is a sample file: http://99.93.62.129/smpte2038.ts > PID 300 is the one triggering the errors. > I am attaching a patch that fixes the problem. > > mpegts.c | 2 +- > 1 file changed, 1 insertion(+), 1 deletion(-) > 8f8c73a5e78d7c740958054764defa3571979fc4 0001-Fix-setting-the-correct-size-for-PES-packets-with-to.patch > From 3a2760d42b38023c73f9a3ab18de01a44526dbc9 Mon Sep 17 00:00:00 2001 > From: Sergio Ammirata <sergio@ammirata.net> > Date: Tue, 3 Aug 2021 13:43:44 +0000 > Subject: [PATCH] Fix setting the correct size for PES packets with too much > padding > > --- > libavformat/mpegts.c | 2 +- > 1 file changed, 1 insertion(+), 1 deletion(-) > > diff --git a/libavformat/mpegts.c b/libavformat/mpegts.c > index 640c9afa5d..40439c26c0 100644 > --- a/libavformat/mpegts.c > +++ b/libavformat/mpegts.c > @@ -1355,7 +1355,7 @@ skip: > buf_size > pes->total_size) { > // pes packet size is < ts size packet and pes data is padded with 0xff > // not sure if this is legal in ts but see issue #2392 > - buf_size = pes->total_size; > + buf_size = PES_START_SIZE + pes->total_size - pes->pes_header_size; > } > memcpy(pes->buffer->data + pes->data_index, p, buf_size); > pes->data_index += buf_size; This causes segfaults [...]
On Thu, Aug 05, 2021 at 11:08:30PM +0200, Michael Niedermayer wrote: > On Tue, Aug 03, 2021 at 10:07:34AM -0400, Sergio M. Ammirata, Ph.D. wrote: > > PES packet with too much padding trigger unlimited error > > messages "PES packet size mismatch" because the code that > > corrects the length is wrong. > > Here is a sample file: http://99.93.62.129/smpte2038.ts > > PID 300 is the one triggering the errors. > > I am attaching a patch that fixes the problem. > > > > > mpegts.c | 2 +- > > 1 file changed, 1 insertion(+), 1 deletion(-) > > 8f8c73a5e78d7c740958054764defa3571979fc4 0001-Fix-setting-the-correct-size-for-PES-packets-with-to.patch > > From 3a2760d42b38023c73f9a3ab18de01a44526dbc9 Mon Sep 17 00:00:00 2001 > > From: Sergio Ammirata <sergio@ammirata.net> > > Date: Tue, 3 Aug 2021 13:43:44 +0000 > > Subject: [PATCH] Fix setting the correct size for PES packets with too much > > padding > > > > --- > > libavformat/mpegts.c | 2 +- > > 1 file changed, 1 insertion(+), 1 deletion(-) > > > > diff --git a/libavformat/mpegts.c b/libavformat/mpegts.c > > index 640c9afa5d..40439c26c0 100644 > > --- a/libavformat/mpegts.c > > +++ b/libavformat/mpegts.c > > @@ -1355,7 +1355,7 @@ skip: > > buf_size > pes->total_size) { > > // pes packet size is < ts size packet and pes data is padded with 0xff > > // not sure if this is legal in ts but see issue #2392 > > - buf_size = pes->total_size; > > + buf_size = PES_START_SIZE + pes->total_size - pes->pes_header_size; > > } > > memcpy(pes->buffer->data + pes->data_index, p, buf_size); > > pes->data_index += buf_size; > > This causes segfaults ==5552== Invalid write of size 2 ==5552== at 0x4C38753: memmove (in /usr/lib/valgrind/vgpreload_memcheck-amd64-linux.so) ==5552== by 0x62DD54: mpegts_push_data (in ffmpeg_g) ==5552== by 0x62B832: handle_packet (in ffmpeg_g) ==5552== by 0x62BCD4: handle_packets (in ffmpeg_g) ==5552== by 0x62BE11: mpegts_read_packet (in ffmpeg_g) ==5552== by 0x6BADB9: ff_read_packet (in ffmpeg_g) ==5552== by 0x6BBAFA: read_frame_internal (in ffmpeg_g) ==5552== by 0x6C05EC: avformat_find_stream_info (in ffmpeg_g) ==5552== by 0x2DB175: open_input_file (in ffmpeg_g) ==5552== by 0x2DEA5B: ffmpeg_parse_options (in ffmpeg_g) ==5552== by 0x2D3151: main (in ffmpeg_g) ==5552== Address 0x2d25cb80 is 0 bytes after a block of size 128 alloc'd ==5552== at 0x4C33E76: memalign (in /usr/lib/valgrind/vgpreload_memcheck-amd64-linux.so) ==5552== by 0x4C33F91: posix_memalign (in /usr/lib/valgrind/vgpreload_memcheck-amd64-linux.so) ==5552== by 0x11166C2: av_malloc (in ffmpeg_g) ==5552== by 0x1100F65: av_buffer_alloc (in ffmpeg_g) ==5552== by 0x11017A4: av_buffer_pool_get (in ffmpeg_g) ==5552== by 0x62E16F: mpegts_push_data (in ffmpeg_g) ==5552== by 0x62B832: handle_packet (in ffmpeg_g) ==5552== by 0x62BCD4: handle_packets (in ffmpeg_g) ==5552== by 0x62BE11: mpegts_read_packet (in ffmpeg_g) ==5552== by 0x6BADB9: ff_read_packet (in ffmpeg_g) ==5552== by 0x6BBAFA: read_frame_internal (in ffmpeg_g) ==5552== by 0x6C05EC: avformat_find_stream_info (in ffmpeg_g) ==5552== by 0x2DB175: open_input_file (in ffmpeg_g) ==5552== by 0x2DEA5B: ffmpeg_parse_options (in ffmpeg_g) ==5552== by 0x2D3151: main (in ffmpeg_g) [...]
You are right ... the if statement would have to change as well. On Tue, 2021-08-03 at 14:20 +0000, ffmpegandmahanstreamer@e.email wrote: > August 3, 2021 10:07 AM, "Sergio M. Ammirata, Ph.D." < > sergio@ammirata.net> wrote: > PES packet with too much padding trigger unlimited > errormessages "PES packet size mismatch" because the code > thatcorrects the length is wrong.Here is a sample file: > http://99.93.62.129/smpte2038.tsPID 300 is the one > triggering the errors.I am attaching a patch that fixes > the problem. > On the if statement right above your change, are you > sure that the condition should still be buf_size > pes- > >total_size? Especially if the old change was just > buf_size = pes->total_size (so clipping according to > condition); and the new one is setting it to something > different. > thx for patch. > _______________________________________________ffmpeg- > devel mailing listffmpeg-devel@ffmpeg.org > https://ffmpeg.org/mailman/listinfo/ffmpeg-devel > To unsubscribe, visit link above, or emailffmpeg-devel- > request@ffmpeg.org with subject > "unsubscribe".___________________________________________ > ____ffmpeg-devel mailing listffmpeg-devel@ffmpeg.org > https://ffmpeg.org/mailman/listinfo/ffmpeg-devel > To unsubscribe, visit link above, or > emailffmpeg-devel-request@ffmpeg.org with subject > "unsubscribe".
Can you share the video file you are using to get the segfault? On Thu, 2021-08-05 at 23:10 +0200, Michael Niedermayer wrote: > On Thu, Aug 05, 2021 at 11:08:30PM +0200, Michael > Niedermayer wrote: > On Tue, Aug 03, 2021 at 10:07:34AM -0400, Sergio M. > Ammirata, Ph.D. wrote: > PES packet with too much padding trigger unlimited > errormessages "PES packet size mismatch" because the code > thatcorrects the length is wrong.Here is a sample file: > http://99.93.62.129/smpte2038.tsPID 300 is the one > triggering the errors.I am attaching a patch that fixes > the problem. > > mpegts.c | 2 +- 1 file changed, 1 insertion(+), 1 > deletion(- > )8f8c73a5e78d7c740958054764defa3571979fc4 0001-Fix- > setting-the-correct-size-for-PES-packets-with- > to.patchFrom 3a2760d42b38023c73f9a3ab18de01a44526dbc9 Mon > Sep 17 00:00:00 2001From: Sergio Ammirata < > sergio@ammirata.net>Date: Tue, 3 Aug 2021 13:43:44 > +0000Subject: [PATCH] Fix setting the correct size for > PES packets with too much padding > --- libavformat/mpegts.c | 2 +- 1 file changed, 1 > insertion(+), 1 deletion(-) > diff --git a/libavformat/mpegts.c > b/libavformat/mpegts.cindex 640c9afa5d..40439c26c0 > 100644--- a/libavformat/mpegts.c+++ > b/libavformat/mpegts.c@@ -1355,7 +1355,7 @@ > skip: buf_size > pes- > >total_size) { // pes packet size is > < ts size packet and pes data is padded with > 0xff // not sure if this is legal in > ts but see issue #2392- buf_size = > pes->total_size;+ buf_size = > PES_START_SIZE + pes->total_size - pes- > >pes_header_size; } memcp > y(pes->buffer->data + pes->data_index, p, > buf_size); pes->data_index += buf_size; > This causes segfaults > ==5552== Invalid write of size 2==5552== at 0x4C38753: > memmove (in /usr/lib/valgrind/vgpreload_memcheck-amd64- > linux.so)==5552== by 0x62DD54: mpegts_push_data (in > ffmpeg_g)==5552== by 0x62B832: handle_packet (in > ffmpeg_g)==5552== by 0x62BCD4: handle_packets (in > ffmpeg_g)==5552== by 0x62BE11: mpegts_read_packet (in > ffmpeg_g)==5552== by 0x6BADB9: ff_read_packet (in > ffmpeg_g)==5552== by 0x6BBAFA: read_frame_internal (in > ffmpeg_g)==5552== by 0x6C05EC: > avformat_find_stream_info (in ffmpeg_g)==5552== by > 0x2DB175: open_input_file (in ffmpeg_g)==5552== by > 0x2DEA5B: ffmpeg_parse_options (in > ffmpeg_g)==5552== by 0x2D3151: main (in > ffmpeg_g)==5552== Address 0x2d25cb80 is 0 bytes after a > block of size 128 alloc'd==5552== at 0x4C33E76: > memalign (in /usr/lib/valgrind/vgpreload_memcheck-amd64- > linux.so)==5552== by 0x4C33F91: posix_memalign (in > /usr/lib/valgrind/vgpreload_memcheck-amd64- > linux.so)==5552== by 0x11166C2: av_malloc (in > ffmpeg_g)==5552== by 0x1100F65: av_buffer_alloc (in > ffmpeg_g)==5552== by 0x11017A4: av_buffer_pool_get (in > ffmpeg_g)==5552== by 0x62E16F: mpegts_push_data (in > ffmpeg_g)==5552== by 0x62B832: handle_packet (in > ffmpeg_g)==5552== by 0x62BCD4: handle_packets (in > ffmpeg_g)==5552== by 0x62BE11: mpegts_read_packet (in > ffmpeg_g)==5552== by 0x6BADB9: ff_read_packet (in > ffmpeg_g)==5552== by 0x6BBAFA: read_frame_internal (in > ffmpeg_g)==5552== by 0x6C05EC: > avformat_find_stream_info (in ffmpeg_g)==5552== by > 0x2DB175: open_input_file (in ffmpeg_g)==5552== by > 0x2DEA5B: ffmpeg_parse_options (in > ffmpeg_g)==5552== by 0x2D3151: main (in ffmpeg_g) > > [...] > > _______________________________________________ffmpeg- > devel mailing listffmpeg-devel@ffmpeg.org > https://ffmpeg.org/mailman/listinfo/ffmpeg-devel > To unsubscribe, visit link above, or > emailffmpeg-devel-request@ffmpeg.org with subject > "unsubscribe".
diff --git a/libavformat/mpegts.c b/libavformat/mpegts.c index 640c9afa5d..40439c26c0 100644 --- a/libavformat/mpegts.c +++ b/libavformat/mpegts.c @@ -1355,7 +1355,7 @@ skip: buf_size > pes->total_size) { // pes packet size is < ts size packet and pes data is padded with 0xff // not sure if this is legal in ts but see issue #2392 - buf_size = pes->total_size; + buf_size = PES_START_SIZE + pes->total_size - pes->pes_header_size; } memcpy(pes->buffer->data + pes->data_index, p, buf_size); pes->data_index += buf_size;