Segfault #144

Closed
opened 2026-01-29 16:36:20 +00:00 by claunia · 5 comments
Owner

Originally created by @Liontooth on GitHub (May 19, 2016).

Originally assigned to: @rkuchumov, @abhishek-vinjamoori on GitHub.

On on fresh ccextractor from this github repository,

$ ccextractor -debug -608 -datets -ttxt -autoprogram -tpage 888 -UCLA -noru -utf8 --nofontcolor -parsepat -parsepmt -unixts $BTIM -o $FIL.ccx.out $FIL.mpg

I get a bunch of warnings, but overall excellent results until the process is interrupted by a segfault:

Warning: Out of order packets detected for PID:.
       ctx->PID_buffers[pid]->prev_ccounter:1100, ctx->ctx->PID_buffers[pid]->ccounter:10
Please Report: Unknown table id in PMT expected 0x02 found 0x8C
Packet (pid 1101) skipped - transport error.
Packet (pid 1101) skipped - transport error.
Packet (pid 1101) skipped - transport error.
Packet (pid 1101) skipped - no payload.
Packet (pid 1101) skipped - no payload.
TS continuity counter not incremented prev/curr 10/14
Warning: Out of order packets detected for PID:.
       ctx->PID_buffers[pid]->prev_ccounter:1100, ctx->ctx->PID_buffers[pid]->ccounter:10
Please Report: Unknown table id in PMT expected 0x02 found 0x8C
Segmentation fault

Sample file (11GB) -- this seems to happen more often with large files:

http://vrnewsscape.ucla.edu/dropbox/2016-02-19_1021_RU_TVC_News.mpg

The text is mostly extracted; the segfault may be at the very end.

Cheers,
David

Originally created by @Liontooth on GitHub (May 19, 2016). Originally assigned to: @rkuchumov, @abhishek-vinjamoori on GitHub. On on fresh ccextractor from this github repository, ``` $ ccextractor -debug -608 -datets -ttxt -autoprogram -tpage 888 -UCLA -noru -utf8 --nofontcolor -parsepat -parsepmt -unixts $BTIM -o $FIL.ccx.out $FIL.mpg ``` I get a bunch of warnings, but overall excellent results until the process is interrupted by a segfault: ``` Warning: Out of order packets detected for PID:. ctx->PID_buffers[pid]->prev_ccounter:1100, ctx->ctx->PID_buffers[pid]->ccounter:10 Please Report: Unknown table id in PMT expected 0x02 found 0x8C Packet (pid 1101) skipped - transport error. Packet (pid 1101) skipped - transport error. Packet (pid 1101) skipped - transport error. Packet (pid 1101) skipped - no payload. Packet (pid 1101) skipped - no payload. TS continuity counter not incremented prev/curr 10/14 Warning: Out of order packets detected for PID:. ctx->PID_buffers[pid]->prev_ccounter:1100, ctx->ctx->PID_buffers[pid]->ccounter:10 Please Report: Unknown table id in PMT expected 0x02 found 0x8C Segmentation fault ``` Sample file (11GB) -- this seems to happen more often with large files: http://vrnewsscape.ucla.edu/dropbox/2016-02-19_1021_RU_TVC_News.mpg The text is mostly extracted; the segfault may be at the very end. Cheers, David
Author
Owner

@cfsmp3 commented on GitHub (May 31, 2016):

This sample is actually quite cool, it shows a couple things we're not dealing with:

Please Report: Unknown table id in PMT expected 0x02 found 0x8C
(that case at least we detect, but I'm not sure we're handling it probably).

The actual crash happen here:
ts_buffer_psi_packet ()
memcpy(ctx->PID_buffers[pid]->buffer+ctx->PID_buffers[pid]->buffer_length, payload_start, payload_length);

Here payload_length goes out of bounds, caused by this:
adaptation_field_length = tspacket[4];
payload_start = payload_start + adaptation_field_length + 1;
payload_length = tspacket+188-payload_start;

For which adaptation_field_length is 230 which is impossible since packets are 188 bytes.
However, the Transport Error Indicator is not set, so this isn't a transmit error (unless it bypassed checks).
Anyway, it's obvious that some sanity checks here are in order if we're going to be doing pointer arithmetic with subtract :-)
I'm assigning this to Ruslan (but still leaving Abhishek) since he's quite good with bits :-)

Ideal behavior of course is to recover and continue processing.
Just terminating is not good enough - we do it in many places around the code and that's fine to process perfect files, but when dealing with live it strange things happen now and then and we can't just refuse to deal with those :-)

@cfsmp3 commented on GitHub (May 31, 2016): This sample is actually quite cool, it shows a couple things we're not dealing with: Please Report: Unknown table id in PMT expected 0x02 found 0x8C (that case at least we detect, but I'm not sure we're handling it probably). The actual crash happen here: ts_buffer_psi_packet () memcpy(ctx->PID_buffers[pid]->buffer+ctx->PID_buffers[pid]->buffer_length, payload_start, payload_length); Here payload_length goes out of bounds, caused by this: adaptation_field_length = tspacket[4]; payload_start = payload_start + adaptation_field_length + 1; payload_length = tspacket+188-payload_start; For which adaptation_field_length is 230 which is impossible since packets are 188 bytes. However, the Transport Error Indicator is not set, so this isn't a transmit error (unless it bypassed checks). Anyway, it's obvious that some sanity checks here are in order if we're going to be doing pointer arithmetic with subtract :-) I'm assigning this to Ruslan (but still leaving Abhishek) since he's quite good with bits :-) Ideal behavior of course is to recover and continue processing. Just terminating is not good enough - we do it in many places around the code and that's fine to process perfect files, but when dealing with live it strange things happen now and then and we can't just refuse to deal with those :-)
Author
Owner

@ghost commented on GitHub (Jan 1, 2017):

Still happening on 0.84. Going to try and fix it.

@ghost commented on GitHub (Jan 1, 2017): Still happening on 0.84. Going to try and fix it.
Author
Owner

@ghost commented on GitHub (Jan 2, 2017):

Looks like we already have a sanity check if adaptation_field_length is less than payload_length, but wait, what? It's supposed to be catching packages where the adaptation field length is more than 184, but it's a < operator in the actual if? I don't understaaaaaannndddd

@ghost commented on GitHub (Jan 2, 2017): Looks like we already have a sanity check if adaptation_field_length is less than payload_length, but wait, what? It's supposed to be catching packages where the adaptation field length is more than 184, but it's a < operator in the actual if? I don't understaaaaaannndddd
Author
Owner

@ghost commented on GitHub (Jan 2, 2017):

Crash is really simple to fix though. Just a quick check to see if the payload length is too much (would be negative if it were an int, but it's unsigned,), and skip it if it is.

@ghost commented on GitHub (Jan 2, 2017): Crash is really simple to fix though. Just a quick check to see if the payload length is too much (would be negative if it were an int, but it's unsigned,), and skip it if it is.
Author
Owner

@Liontooth commented on GitHub (Jan 20, 2017):

This fixes the oops, great work.

In the sample file, at the point of the previous segfault, we now get a skip of 380,803 seconds:

20160219123436.520|20160219123439.520|CC1|-Ну что, патрончики-то кончились, а? 20160219123439.680|20160219123442.000|CC1|НАПРЯЖЕННАЯ МУЗЫКА 20160219123512.520|20160219123513.360|CC1|МУЗЫКА
20160223222156.044|20160223222158.404|CC1|-Кто сообщит родственникам? 20160223222158.524|20160223222201.484|CC1|-Я. -Я. -Я сама сообщу. 20160223222207.044|20160223222210.604|CC1|-Тебя подвезти?

Could you confirm this is a problem in the original file and not a bug in CCExtractor?

@Liontooth commented on GitHub (Jan 20, 2017): This fixes the oops, great work. In the sample file, at the point of the previous segfault, we now get a skip of 380,803 seconds: `20160219123436.520|20160219123439.520|CC1|-Ну что, патрончики-то кончились, а? 20160219123439.680|20160219123442.000|CC1|НАПРЯЖЕННАЯ МУЗЫКА 20160219123512.520|20160219123513.360|CC1|МУЗЫКА` `20160223222156.044|20160223222158.404|CC1|-Кто сообщит родственникам? 20160223222158.524|20160223222201.484|CC1|-Я. -Я. -Я сама сообщу. 20160223222207.044|20160223222210.604|CC1|-Тебя подвезти?` Could you confirm this is a problem in the original file and not a bug in CCExtractor?
Sign in to join this conversation.
1 Participants
Notifications
Due Date
No due date set.
Dependencies

No dependencies set.

Reference: starred/ccextractor#144