Opened 12 years ago

Closed 12 years ago

#4746 closed defect (fixed)

make sure the audio_codec pointer is valid before dereferencing in AudioOutputBase::Reconfigure

Reported by: Erik Hovland <erik@…> Owned by: Isaac Richards
Priority: minor Milestone: 0.21
Component: mythtv Version: head
Severity: low Keywords:
Cc: Ticket locked: no

Description

AudioOutputBase::Reconfigure in libs/libmyth/audiooutputbase.cpp may have a null for the audio_codec pointer. So when it builds the soundtouch object, it could dereference that pointer (causing a segfault). It should check before doing this.

Attachments (3)

libs_libmyth_audiooutputbase.cpp-fix-forgotten-null-check.patch (1.3 KB) - added by Erik Hovland <erik@…> 12 years ago.
Adds check of audio_codec pointer before it is dereferenced
libs_libmyth_audiooutputbase.cpp-fix-forgotten-null-check.2.patch (1.5 KB) - added by Erik Hovland <erik@…> 12 years ago.
This is the same check for null, but it leverages the conditional right before
libs_libmyth_audiooutputbase.cpp-fix-forgotten-null-check.3.patch (1.8 KB) - added by Erik Hovland <erik@…> 12 years ago.
Last try. Adds new check in another method that does the same work

Download all attachments as: .zip

Change History (9)

Changed 12 years ago by Erik Hovland <erik@…>

Adds check of audio_codec pointer before it is dereferenced

comment:1 Changed 12 years ago by Nigel

Thanks for your recent code reviews and patches, Erik.
I think those two lines you are guarding should just be moved up 3 lines:

% svn diff libs/libmyth/audiooutputbase.cpp
Index: libs/libmyth/audiooutputbase.cpp
===================================================================
--- libs/libmyth/audiooutputbase.cpp    (revision 16194)
+++ libs/libmyth/audiooutputbase.cpp    (working copy)
@@ -396,9 +396,7 @@
                         VERBOSE(VB_AUDIO, LOC + "Failed to Create Encoder");
                     }
                 }
-            }
-            if (encoder)
-            {
+
                 pSoundStretch->setSampleRate(audio_codec->sample_rate);
                 pSoundStretch->setChannels(audio_codec->channels);
             }

comment:2 Changed 12 years ago by Erik Hovland <erik@…>

Good point. I am reworking my patch and testing. At the least, the encoder null pointer check is still valid since the code above it might set it to null if the init didn't work out.

Changed 12 years ago by Erik Hovland <erik@…>

This is the same check for null, but it leverages the conditional right before

comment:3 Changed 12 years ago by Erik Hovland <erik@…>

Nigel is right, the previous conditional is the same as the check I added. So I reworked the code to keep using the null pointer check instead of making my own.

Changed 12 years ago by Erik Hovland <erik@…>

Last try. Adds new check in another method that does the same work

comment:4 Changed 12 years ago by Nigel

(In [16251]) Check for invalid audio_codec in Reconfigure(). See #4746.

comment:5 Changed 12 years ago by Nigel

(In [16252]) Check for invalid audio_codec in Reconfigure(). See #4746.

comment:6 Changed 12 years ago by Nigel

Resolution: fixed
Status: newclosed

Thinking further about this, I was wrong. If audio_codec was set, but the encoder failed to be created, I think we want the sound stretching to be reset to the default sample rate. Reopen this if my checkins cause problems.

Note: See TracTickets for help on using tickets.