Kext OSDynamicCast error during OSObject :: free

I have an IOKit driver, which is obtained from the IOService base class, and its raw pointer is delivered to some callback function from the kauth structure, which can be called very often.

To extract this instance from the pointer, I use the safe OSDynamicCast method, and I make sure that during the driver disconnection, I will disconnect the kauth calls and clear all existing calls BEFORE releasing the driver. However, sometimes I still get a kernel panic on OSDynamicCast :

 frame #0: [inlined] OSMetaClass::checkMetaCast(check=0xffffff802b28d3f0) frame #1: [inlined] OSMetaClassBase::metaCast(OSMetaClass const*) const frame #2: kernel`OSMetaClassBase::safeMetaCast 

When I disconnected and dropped kauth calls to OSObject::free in the IOService::stop , the problem did not recur (at least after several tens of attempts).

Perhaps someone has an idea if during the period between ::stop and ::free memory that causes this panic is freed up?

Here is a little code that emphasizes my design when it causes a panic.

 kauth_callback(kauth_cred_t credential, void *idata, /* This is the RAW pointer for my IOService based instance */ kauth_action_t action, uintptr_t arg0, uintptr_t arg1, uintptr_t arg2, uintptr_t arg3) { ... // taking shared_lock with mutex auto my_inst = OSDynamicCast(com_my_driver, reinterpret_cast<OSObject *>(idata)); ... } void com_my_driver::free(IOService *provider) { kauth_unlisten_scope(my_listener_); // taking unique lock with mutex to make sure no outstanding kauth calls. super::free(provider); //calling OSObject free } 

And if I translate the logic from ::free to ::stop , it works:

 void com_my_driver::stop(IOService *provider) { kauth_unlisten_scope(my_listener_); // taking unique lock with mutex to make sure no outstanding kauth calls. super::stop(provider); // Calling IOService::stop() } 
+5
source share
1 answer

In kauth_unlisten_scope there is an inherent race condition. Solving your mutex will almost certainly not completely solve the problem, because your code in the callback can actually run after kauth_unlisten_scope() returns - in other words, the kauth callback has not yet blocked the mutexes.

All you can do is sleep some time after returning kauth_unlisten_scope() . Hopefully after a second or so, all kauth callbacks have completed successfully.

If you want to be more careful, you can also add a global boolean flag, which is usually true, but set to false before you unregister your kauth listener. You can check the flag when entering a callback, before locking the mutex, etc. If this flag is false, return immediately from the callback. This at least prevents access to dynamically allocated memory; however, it still does not 100% solve the problem in principle, because the global variable will, of course, disappear when kext is unloaded.

Apple has been aware of the problem for 7 years when I used the kauth API; they didn’t fix it at the time, and as they plan to phase out the kexts over the next few years, I don’t think it will change.

Side notes:

Do not use reinterpret_cast<> to convert from opaque void* to a specific pointer type. static_cast<> is for this purpose.

Alternatively, you can use a static action instead of a dynamic one, assuming you always pass objects like com_my_driver to kauth_listen_scope() . In fact, you have to do static_cast<> for a static type that was originally degraded to void* , which I suspect is not an OSObject in your case. If this is different from the dynamic type that you expect, cast the result of this to a derived type.

eg. BADLY:

 //startup com_my_driver* myobj = …; // com_my_driver* implicitly degrading to void* kauth_listen_scope("com_my_driver", kauth_callback, myobj); // … int kauth_callback(kauth_cred_t credential, void *idata, kauth_action_t action, uintptr_t arg0, uintptr_t arg1, uintptr_t arg2, uintptr_t arg3) { // the void* came from a com_my_driver*, not an OSObject*! auto my_inst = static_cast<com_my_driver*>(static_cast<OSObject *>(idata)); } 

IT IS BETTER:

 int kauth_callback(kauth_cred_t credential, void *idata, kauth_action_t action, uintptr_t arg0, uintptr_t arg1, uintptr_t arg2, uintptr_t arg3) { auto my_inst = static_cast<com_my_driver*>(idata); } 

This is a pretty minor point, and assuming you are not doing anything with multiple inheritance, which you should not do in IOKit, in any case it will be compiled with the same code, but it will bypass undefined behavior. In addition, incorrect code is more confusing to read, and if you use OSDynamicCast , less efficient - and efficiency is extremely important in kauth callbacks.

In fact, so much so that I am afraid to even block the mutex on the hot path that you propose to do. This means that you literally single-threaded download all the I / O files for all user processes throughout the system. Do not send this code to customers. Consider using RW-Lock instead, and only a read lock in general with write locks reserved for when they are really needed.

+1
source

Source: https://habr.com/ru/post/1276101/


All Articles