Opened 10 years ago

Closed 10 years ago

Last modified 10 years ago

#7614 closed defect (fixed)

Segfault can occur in TVRec::StartRecording()

Reported by: david.madsen@… Owned by: danielk
Priority: minor Milestone: unknown
Component: MythTV - Recording Version: head
Severity: medium Keywords:
Cc: Ticket locked: no

Description

A potential segfault exists in the TVRec::StartRecording?() function. This function creates an iterator to the pendingRecordings QMap. It then yields to the EventThread? to process any outstanding events before proceeding.

In the event thread it is possible that the call to TVRec::HandlePendingRecordings?() will find a stale recording pending and will proceed to free the memory used by the info pointer and then delete the entry out of the pendingRecordings map.

Eventually we will return to execution in the TVRec::StartRecording?() function and we will continue to use the existing iterator. The iterator itself will still be valid since we are looking at a previously made copy of the QMap but since the TVRec::HandlePendingRecordings?() call deleted the data stored at the info pointer this code will cause a segfault.

            if (is_busy && !sourceid)
            {
                mplexid  = (*it).info->GetMplexID();
                sourceid = (*it).info->sourceid;
            }

I've attached a patch that simply re-executes the find after being awaken by the EventThread?. Since the stale entry has now been deleted the following loop will not try to dereference the invalid pointer.

Attachments (1)

StartingRecording_segfault.patch (694 bytes) - added by david.madsen@… 10 years ago.
patch for TVRec::StartRecording?() segfault

Download all attachments as: .zip

Change History (4)

Changed 10 years ago by david.madsen@…

patch for TVRec::StartRecording?() segfault

comment:1 Changed 10 years ago by danielk

Resolution: fixed
Status: newclosed

(In [22861]) Fixes #7614. Fixes a segfault with patch by David Madsen, see ticket for details.

comment:2 Changed 10 years ago by danielk

(In [22862]) Refs #7614. Backport [22861]. Simple fix for a segfault.

comment:3 Changed 10 years ago by anonymous

Do we need to update the value of cancelNext after updating the iterator? If not it may not match the value from the new pending recording. That seems like it would be bad:

     // Rescan pending recordings since the event loop may have deleted
     // a stale entry.  If this happens the info pointer will not be valid
     // since the HandlePendingRecordings loop will have deleted it.
     it = pendingRecordings.find(cardid);
+    if (it != pendingRecordings.end())
+        cancelNext = (*it).canceled;
 

Note: See TracTickets for help on using tickets.