Context Navigation

← Previous Ticket
Next Ticket →

Opened 7 years ago

Closed 7 years ago

Last modified 7 years ago

#13263 closed Bug Report - Crash (fixed)

Crash in ProgramMapTable::Create

Reported by:	Gary Buhrmaster	Owned by:	Peter Bennett
Priority:	minor	Milestone:	29.2
Component:	MythTV - General	Version:	Master Head
Severity:	medium	Keywords:	gcc8
Cc:		Ticket locked:	no

Description

Abort in libmythtv due to accessing beyond the size of the vector.

With at least some recording sources (I am using an OCUR device) it is apparently possible to end up having zero descriptors in the stream when ProgramMapTable::Create is called, but GCC 8 (and libstdc++) now includes AddressSanitizer? integration for std::vector, detecting out-of-range accesses to a vector, which means that referencing the 0th element is now an error (and an abort) even though the called code would not typically copy any data.

Proposed (lightly tested (it no longer crashes!)) fix follows to simply not try to copy from an empty vector to avoid the abort.

It accepted, this will likely need to be backported to fixes/29 (and maybe fixes/0.28 if that is still considered supported).

I suspect there is other places in the code where this will need to be fixed (when it rains, it pours).

Proposed patch:

diff --git a/mythtv/libs/libmythtv/mpeg/mpegtables.cpp b/mythtv/libs/libmythtv/mpeg/mpegtables.cpp
index 3120b821f6..296e3a3416 100644
--- a/mythtv/libs/libmythtv/mpeg/mpegtables.cpp
+++ b/mythtv/libs/libmythtv/mpeg/mpegtables.cpp
@@ -445,7 +445,8 @@ ProgramMapTable* ProgramMapTable::Create(
         uint len = global_desc[i][1] + 2;
         gdesc.insert(gdesc.end(), global_desc[i], global_desc[i] + len);
     }
-    pmt->SetProgramInfo(&gdesc[0], gdesc.size());
+    if (!gdesc.empty())
+        pmt->SetProgramInfo(&gdesc[0], gdesc.size());
 
     for (uint i = 0; i < count; i++)
     {
@@ -457,7 +458,8 @@ ProgramMapTable* ProgramMapTable::Create(
                          prog_desc[i][j], prog_desc[i][j] + len);
         }
 
-        pmt->AppendStream(pids[i], types[i], &pdesc[0], pdesc.size());
+        if (!pdesc.empty())
+            pmt->AppendStream(pids[i], types[i], &pdesc[0], pdesc.size());
     }
     pmt->Finalize();

Attachments (2)

20180524_gary_patch2a_ticket13263.patch (886 bytes) - added by Peter Bennett 7 years ago.: patch 2a
20180524_gary_patch2b_ticket13263.patch (532 bytes) - added by Peter Bennett 7 years ago.: patch 2b

Download all attachments as: .zip

Change History (28)

comment:1 Changed 7 years ago by Gary Buhrmaster

FWIW: related (by gcc8 changes for involving the improved AddressSanitizer?) to #13264

comment:2 Changed 7 years ago by David Hampton <mythtv@…>

Owner:	set to David Hampton <mythtv@…>
Resolution:	→ fixed
Status:	new → closed

In 7f00642ba1/mythtv:

Fix crash in ProgramMapTable::Create

Abort in libmythtv due to accessing beyond the size of the vector.

With at least some recording sources (I am using an OCUR device) it is
apparently possible to end up having zero descriptors in the stream
when ProgramMapTable::Create is called, but GCC 8 (and libstdc++) now
includes AddressSanitizer? integration for std::vector, detecting
out-of-range accesses to a vector, which means that referencing the
0th element is now an error (and an abort) even though the called code
would not typically copy any data.

Patch from Gary Buhrmaster, fixes #13263.

comment:3 Changed 7 years ago by David Hampton <mythtv@…>

In c2fa4ba798/mythtv:

Fix crash in ProgramMapTable::Create

Abort in libmythtv due to accessing beyond the size of the vector.

With at least some recording sources (I am using an OCUR device) it is
apparently possible to end up having zero descriptors in the stream
when ProgramMapTable::Create is called, but GCC 8 (and libstdc++) now
includes AddressSanitizer? integration for std::vector, detecting
out-of-range accesses to a vector, which means that referencing the
0th element is now an error (and an abort) even though the called code
would not typically copy any data.

Patch from Gary Buhrmaster, fixes #13263.

(cherry picked from commit 7f00642ba11eb0d9d633a23ce74e5b695c05153e)

comment:4 Changed 7 years ago by David Hampton <mythtv@…>

In 0635315bc/mythtv:

Fix crash in ProgramMapTable::Create

Abort in libmythtv due to accessing beyond the size of the vector.

With at least some recording sources (I am using an OCUR device) it is
apparently possible to end up having zero descriptors in the stream
when ProgramMapTable::Create is called, but GCC 8 (and libstdc++) now
includes AddressSanitizer? integration for std::vector, detecting
out-of-range accesses to a vector, which means that referencing the
0th element is now an error (and an abort) even though the called code
would not typically copy any data.

Patch from Gary Buhrmaster, fixes #13263.

(cherry picked from commit 7f00642ba11eb0d9d633a23ce74e5b695c05153e)

comment:5 Changed 7 years ago by Stuart Auchterlonie

Milestone:	unknown → 29.2
Version:	Unspecified → Master Head

comment:6 Changed 7 years ago by warpme

this commit causes for me failed recordings/livetv from multiple DVB-S/S2 SAT mplexes with:

DTVRec[17]: Music Choice program detected

it looks this commit is clear regression

i'm on current master

comment:7 Changed 7 years ago by David Hampton

Resolution:	fixed
Status:	closed → new

comment:8 Changed 7 years ago by Peter Bennett

The changes have been reverted in aa8bdcb6927e59375dec20b13ae066b3d171e2a4 , 0849e9959639dd8dfeab0af4ce0dfc166da1f896 , 519a3f550057a252fb82622c7860fb9d239c6a3c

comment:9 Changed 7 years ago by Peter Bennett

New patch from Gary is available at https://lists.gt.net/mythtv/dev/617728#617728

comment:10 Changed 7 years ago by Peter Bennett

Owner:	changed from David Hampton <mythtv@…> to Peter Bennett
Status:	new → assigned

I confirm this patch also caused failure in CETON recordings.

Changed 7 years ago by Peter Bennett

Attachment:	20180524_gary_patch2a_ticket13263.patch added

patch 2a

Changed 7 years ago by Peter Bennett

Attachment:	20180524_gary_patch2b_ticket13263.patch added

patch 2b

comment:11 Changed 7 years ago by Peter Bennett

Email from Gary May 14th 2018

Based on your far greater c++ expertise, the follow is the next (final?) proposed patch:

And while it is not strictly necessary, I would also propose this patch, in order to avoid a potential future static analysis warnings:

I have been running my BE and FE with these patches and they seem to work for me.

comment:12 Changed 7 years ago by Peter Bennett

Gary's new patch looks good, as long as we are on C++ 11, which it seems we are. I will test them personally before committing anything.

comment:13 Changed 7 years ago by Peter Bennett

Resolution:	→ Fixed
Status:	assigned → closed

Fixed in eba1a192dbc5d81c29f0e08300a760038d20e1cd

Author: Gary Buhrmaster <gary.buhrmaster@…> Date: Fri May 25 15:43:51 2018 -0400

Fix crash during recording with newer compilers

Invalid attempt to access subscript 0 of a vector of size 0 causes an abort with newer compilers. Fixed in three places.

Fixes #13263

Signed-off-by: Peter Bennett <pbennett@…>