the-darkvoid / BrcmPatchRAM

Broadcom PatchRAM driver for OS X
GNU General Public License v2.0
88 stars 62 forks source link

The event passed to IOLockSleep/IOLockWakeup can not be NULL #17

Closed hjelmn closed 9 years ago

hjelmn commented 9 years ago

A NULL event causes a kernel panic with 10.10.3 and probably other versions.

Signed-off-by: Nathan Hjelm hjelmn@me.com

hjelmn commented 9 years ago

For reference. The IOLockSleep call eventually calls assert_wait (see https://opensource.apple.com/source/xnu/xnu-2782.1.97/osfmk/kern/sched_prim.c). This is where the failure occurs:

    if(event == NO_EVENT)
        panic("assert_wait() called with NO_EVENT");

NO_EVENT is defined as:

#define     NO_EVENT            ((event_t) 0)

in osfmk/kern/kern_types.h

From what I can tell the event can be just about anything. Using a pointer to a void * is done in other projects.

RehabMan commented 9 years ago

Are you sure? It is working fine on my machine running 10.10.3.

hjelmn commented 9 years ago

Pretty sure. Digging through xnu there is no way this won't panic. My hackintosh panics on every boot without this change.

I have this card: http://www.amazon.com/gp/product/B00JY6X9HM?psc=1&redirect=true&ref_=oh_aui_detailpage_o07_s00

RehabMan commented 9 years ago

Like I said, no panic here. From where did you download Yosemite?

RehabMan commented 9 years ago

Note if the "event" just has to be non-zero, we can simply use 'this'.

hjelmn commented 9 years ago

Created 10.10.2 image from Install OS X Yosemite.app and patched with Unibeast. Updated to 10.10.3 using the App Store.

As far as I can tell using this should be safe. I didn't dig deep enough in xnu to be sure that the kernel doesn't write something to the provided address.

RehabMan commented 9 years ago

Strange that you're the only person having this issue...

hjelmn commented 9 years ago

Yeah. Not sure why. I can send you a crash reporter log on Friday showing the panic.

RehabMan commented 9 years ago

It probably has something to do with the headers/SDK you're building against...

The project, as checked in in my repo, uses SDK 10.6.

RehabMan commented 9 years ago

For compatibility with newer SDK, this patch:

Speedy-OSX:brcmpatch.git RehabMan$ git diff
diff --git a/BrcmPatchRAM/BrcmPatchRAM.cpp b/BrcmPatchRAM/BrcmPatchRAM.cpp
index 40b251a..44b0dec 100644
--- a/BrcmPatchRAM/BrcmPatchRAM.cpp
+++ b/BrcmPatchRAM/BrcmPatchRAM.cpp
@@ -764,7 +764,7 @@ void BrcmPatchRAM::readCompletion(void* target, void* parameter, IOReturn status
     IOLockUnlock(me->mCompletionLock);

     // wake waiting task in performUpgrade (in IOLockSleep)...
-    IOLockWakeup(me->mCompletionLock, NULL, true);
+    IOLockWakeup(me->mCompletionLock, me, true);
 }

 IOReturn BrcmPatchRAM::hciCommand(void * command, UInt16 length)
@@ -1039,7 +1039,7 @@ bool BrcmPatchRAM::performUpgrade()
             continue;
         }
         // wait for completion of the async read
-        IOLockSleep(mCompletionLock, NULL, 0);
+        IOLockSleep(mCompletionLock, this, 0);
     }

     IOLockUnlock(mCompletionLock);

That's what I'll check in after testing.

RehabMan commented 9 years ago

The change is checked into my fork of this repo.

the-darkvoid commented 9 years ago

Changes merged as part of Rehabman's other pull.