Skip to content

Commit

Permalink
Fix race condition with notification registration before registerServ…
Browse files Browse the repository at this point in the history 
…ice (#8)

Resolves null pointer dereference crashes in notification handlers. Also adds initialising class members at declaration.
  • Loading branch information
@lvs1974
lvs1974 authored Feb 21, 2020
1 parent 628f5c0 commit 8841fd8
Show file tree
Hide file tree
Showing 2 changed files with 50 additions and 42 deletions.
59 changes: 33 additions & 26 deletions VoodooPS2Controller/VoodooPS2Controller.cpp
View file
Original file line number Diff line number Diff line change
Expand Up @@ -293,6 +293,11 @@ bool ApplePS2Controller::init(OSDictionary* dict)
_cmdbyteLock = IOLockAlloc();
if (!_cmdbyteLock)
return false;

_deliverNotification = OSSymbol::withCString(kDeliverNotifications);
if (_deliverNotification == NULL)
return false;


_workLoop = 0;

Expand Down Expand Up @@ -383,6 +388,7 @@ void ApplePS2Controller::free(void)
IOLockFree(_cmdbyteLock);
_cmdbyteLock = 0;
}

#if DEBUGGER_SUPPORT
if (_controllerLock)
{
Expand Down Expand Up @@ -502,38 +508,14 @@ void ApplePS2Controller::resetController(void)
bool ApplePS2Controller::start(IOService * provider)
{
DEBUG_LOG("ApplePS2Controller::start entered...\n");

const OSSymbol * deliverNotification = OSSymbol::withCString(kDeliverNotifications);
if (deliverNotification == NULL)
return false;

OSDictionary * propertyMatch = propertyMatching(deliverNotification, kOSBooleanTrue);
if (propertyMatch != NULL) {
IOServiceMatchingNotificationHandler notificationHandler = OSMemberFunctionCast(IOServiceMatchingNotificationHandler, this, &ApplePS2Controller::notificationHandler);

//
// Register notifications for availability of any IOService objects wanting to consume our message events
//
_publishNotify = addMatchingNotification(gIOFirstPublishNotification,
propertyMatch,
notificationHandler,
this,
0, 10000);

_terminateNotify = addMatchingNotification(gIOTerminatedNotification,
propertyMatch,
notificationHandler,
this,
0, 10000);

propertyMatch->release();
}
deliverNotification->release();
//
// The driver has been instructed to start. Allocate all our resources.
//
if (!super::start(provider))
return false;

OSDictionary * propertyMatch = nullptr;

#if DEBUGGER_SUPPORT
// Enable special key sequence to enter debugger if debug boot-arg was set.
Expand Down Expand Up @@ -676,6 +658,28 @@ bool ApplePS2Controller::start(IOService * provider)

registerService();

propertyMatch = propertyMatching(_deliverNotification, kOSBooleanTrue);
if (propertyMatch != NULL) {
IOServiceMatchingNotificationHandler notificationHandler = OSMemberFunctionCast(IOServiceMatchingNotificationHandler, this, &ApplePS2Controller::notificationHandler);

//
// Register notifications for availability of any IOService objects wanting to consume our message events
//
_publishNotify = addMatchingNotification(gIOFirstPublishNotification,
propertyMatch,
notificationHandler,
this,
0, 10000);

_terminateNotify = addMatchingNotification(gIOTerminatedNotification,
propertyMatch,
notificationHandler,
this,
0, 10000);

propertyMatch->release();
}

DEBUG_LOG("ApplePS2Controller::start leaving.\n");
return true; // success

Expand Down Expand Up @@ -728,6 +732,7 @@ void ApplePS2Controller::stop(IOService * provider)

// Free the RMCF configuration cache
OSSafeReleaseNULL(_rmcfCache);
OSSafeReleaseNULL(_deliverNotification);

// Free the request queue lock and empty out the request queue.
if (_requestQueueLock)
Expand Down Expand Up @@ -2036,6 +2041,7 @@ void ApplePS2Controller::notificationHandlerGated(IOService * newService, IONoti

bool ApplePS2Controller::notificationHandler(void * refCon, IOService * newService, IONotifier * notifier)
{
assert(_cmdGate != nullptr);
_cmdGate->runAction(OSMemberFunctionCast(IOCommandGate::Action, this, &ApplePS2Controller::notificationHandlerGated), newService, notifier);
return true;
}
Expand Down Expand Up @@ -2080,6 +2086,7 @@ void ApplePS2Controller::dispatchMessageGated(int* message, void* data)

void ApplePS2Controller::dispatchMessage(int message, void* data)
{
assert(_cmdGate != nullptr);
_cmdGate->runAction(OSMemberFunctionCast(IOCommandGate::Action, this, &ApplePS2Controller::dispatchMessageGated), &message, data);
}

Expand Down
33 changes: 17 additions & 16 deletions VoodooPS2Controller/VoodooPS2Controller.h
View file
Original file line number Diff line number Diff line change
Expand Up @@ -181,9 +181,9 @@ class EXPORT ApplePS2Controller : public IOService
OSDeclareDefaultStructors(ApplePS2Controller);

public: // interrupt-time variables and functions
IOInterruptEventSource * _interruptSourceKeyboard;
IOInterruptEventSource * _interruptSourceMouse;
IOInterruptEventSource * _interruptSourceQueue;
IOInterruptEventSource * _interruptSourceKeyboard {nullptr};
IOInterruptEventSource * _interruptSourceMouse {nullptr};
IOInterruptEventSource * _interruptSourceQueue {nullptr};

#if DEBUGGER_SUPPORT
bool _debuggingEnabled;
Expand All @@ -197,7 +197,7 @@ class EXPORT ApplePS2Controller : public IOService
#endif //DEBUGGER_SUPPORT

private:
IOWorkLoop * _workLoop;
IOWorkLoop * _workLoop {nullptr};
queue_head_t _requestQueue;
IOLock* _requestQueueLock;
IOLock* _cmdbyteLock;
Expand All @@ -221,20 +221,20 @@ class EXPORT ApplePS2Controller : public IOService
int _ignoreInterrupts;
int _ignoreOutOfOrder;

ApplePS2MouseDevice * _mouseDevice; // mouse nub
ApplePS2KeyboardDevice * _keyboardDevice; // keyboard nub
ApplePS2MouseDevice * _mouseDevice {nullptr}; // mouse nub
ApplePS2KeyboardDevice * _keyboardDevice {nullptr}; // keyboard nub

IONotifier* _publishNotify;
IONotifier* _terminateNotify;
IONotifier* _publishNotify {nullptr};
IONotifier* _terminateNotify {nullptr};

OSSet* _notificationServices;
OSSet* _notificationServices {nullptr};

#if DEBUGGER_SUPPORT
IOSimpleLock * _controllerLock; // mach simple spin lock
IOSimpleLock * _controllerLock {nullptr}; // mach simple spin lock

KeyboardQueueElement * _keyboardQueueAlloc; // queues' allocation space
queue_head_t _keyboardQueue; // queue of available keys
queue_head_t _keyboardQueueUnused; // queue of unused entries
KeyboardQueueElement * _keyboardQueueAlloc {nullptr}; // queues' allocation space
queue_head_t _keyboardQueue; // queue of available keys
queue_head_t _keyboardQueueUnused; // queue of unused entries

bool _extendedState;
UInt16 _modifierState;
Expand All @@ -249,11 +249,12 @@ class EXPORT ApplePS2Controller : public IOService
#endif
int _wakedelay;
bool _mouseWakeFirst;
IOCommandGate* _cmdGate;
IOCommandGate* _cmdGate {nullptr};
#if WATCHDOG_TIMER
IOTimerEventSource* _watchdogTimer;
IOTimerEventSource* _watchdogTimer {nullptr};
#endif
OSDictionary* _rmcfCache;
OSDictionary* _rmcfCache {nullptr};
const OSSymbol* _deliverNotification {nullptr};

virtual PS2InterruptResult _dispatchDriverInterrupt(PS2DeviceType deviceType, UInt8 data);
virtual void dispatchDriverInterrupt(PS2DeviceType deviceType, UInt8 data);
Expand Down

0 comments on commit 8841fd8

Please sign in to comment.