Hi Klaus,
I've isolated an null pointer dereferencing bug in cSkins::Message():
In the very rare case that a message is displayed using a cSkinDisplayMessage object, and the message is not removed before another cSkinDisplay object takes control, the next attempt to display a message while no cSkinDisplay::Current() object is available causes a segfault.
The reason is that the cSkins::displayMessage pointer is not cleared if another cSkinDisplay object takes over. cSkins::Message() sees an existing displayMessage object, but accesses the non-existing cSkinDisplay::Current(), assuming they're both the same.
The only way I know how to trigger this is to use mtStatus messages and allow user activity while the message is shown. I don't think that plain VDR can trigger this on its own.
The attached patch changes cSkins::Message() by only checking the existence of cSkinDisplay::Current(). If no current cSkinDisplay exists, any old displayMessage object is deleted and a new one is created.
Cheers,
Udo
--- skins.c.orig 2006-11-30 02:19:41.705523304 +0100 +++ skins.c 2006-11-30 02:33:28.269230960 +0100 @@ -226,8 +226,11 @@ } if (!Current()) return kNone; - if (!cSkinDisplay::Current() && !displayMessage) + if (!cSkinDisplay::Current()) { + if (displayMessage) + delete displayMessage; displayMessage = Current()->DisplayMessage(); + } cSkinDisplay::Current()->SetMessage(Type, s); cSkinDisplay::Current()->Flush(); cStatus::MsgOsdStatusMessage(s);
Udo Richter wrote:
I've found another segfault related to displayMessage. If VDR gets terminated while a message is displayed, the delete displayMessage in cSkins::~cSkins() takes action. But since Skins is a global variable, ~cSkins() will be called very late within the shutdown process, at a time when skins and OSD are already free'd. In case of skin plugins, the plugin program code is even already unloaded at that point, causing a segfault.
The attached patch cleans up any remaining displayMessage before the cSkin objects are freed.
Cheers,
Udo
--- skins.h.orig 2006-12-05 17:35:51.724023920 +0100 +++ skins.h 2006-12-05 17:37:27.589486448 +0100 @@ -360,6 +360,8 @@ ///< Processes the first queued message, if any. void Flush(void); ///< Flushes the currently active cSkinDisplay, if any. + virtual void Clear(void); + ///< Free up all registered skins };
extern cSkins Skins; --- skins.c.orig 2006-12-05 17:47:10.949575792 +0100 +++ skins.c 2006-12-05 18:05:27.597505048 +0100 @@ -358,3 +358,12 @@ if (cSkinDisplay::Current()) cSkinDisplay::Current()->Flush(); } + +void cSkins::Clear(void) +{ + if (displayMessage) { + delete displayMessage; + displayMessage = NULL; + } + cList<cSkin>::Clear(); +}
-
Udo Richter