Search code examples
c++iosiokitipadosdriverkit

Possible leak of an IOBufferMemoryDescriptor in DriverKit driver

I have a DriverKit driver for iPadOS. It works fine, but the static analyzer is flagging a possible leak that I don't understand. The code flagged is in the driver class itself:

void IMPL(CDCDriver, ReadComplete)
{
    // Simply trampolines back to USBCDCInterface where the read is actually handled
    IOBufferMemoryDescriptor *readDataBuffer = ivars->cdcInterface->readData(status, actualByteCount, completionTimestamp);
    if (actualByteCount > 0) {
        os_log(OS_LOG_DEFAULT, "[Driver] Got %u bytes of data!", actualByteCount);
        if (ivars->client != nullptr) {
            ivars->client->dataWasReceived(readDataBuffer, actualByteCount);
        }
    }
}  // [!] Potential leak of an object stored into 'readDataBuffer'

Screenshot of static analyzer results

According to the analyzer, the call to readData() on the first line of the method returns an OSObject of type IOBufferMemoryDescriptor with a +1 retain count. However, that method is implemented like so:

IOBufferMemoryDescriptor *USBCDCInterface::readData(IOReturn status,
                                                    uint32_t actualByteCount,
                                                    uint64_t completionTimestamp)
{
    pollForData();
    return readDataBuffer;
}

Here, readDataBuffer is a member variable of USBCDCInterface, created when the driver is started with a call to IOUSBHostInterface::CreateIOBuffer():

IOBufferMemoryDescriptor *readDataBuffer = nullptr;
result = dataInterface->CreateIOBuffer(kIOMemoryDirectionIn, 256, &readDataBuffer);
if (result != kIOReturnSuccess) {
    return result;
}
this->readDataBuffer = readDataBuffer;

My understanding of OSObject (and subclasses there of) is that it uses a reference counting mechanic very similar to NSObject. Creating it, e.g. with CreateIOBuffer() should result in an object with a +1 retain count. The USBCDCInterface instance that created it owns it, and therefore should release() it when it's done with it (in USBCDCInterface::free()). Simply returning it from readData() shouldn't transfer ownership, and it shouldn't be the responsibility of the caller of that method to release it. Notably, calling release() on the object at the end of CDCDriver::ReadComplete() causes the driver to crash, just as I'd expect, which makes me think it's a false positive from the analyzer.

Either my understanding is flawed, and there's something about returning that object from USBCDCInterface::readData() that (by convention or otherwise) obligates the caller to release it (again though, doing so crashes the driver), or else this is a false positive on the part of the analyzer.

If this were ObjC/Cocoa, I'd guess that there's something about the naming of my methods causing the analyzer to assume the readData() method effectively transfers ownership (like a -copy or +alloc method), but I can't find any documentation of naming conventions like that, nor have I been able to work out if/how OSObject methods are annotated with their memory management semantics.

Can anyone provide any insight?

Solution

  • You should have continued the “if this were Objective-C/Cocoa” thought to its logical conclusion: OSObject refcounting follows the same a similar convention as Objective-C and CoreFoundation's. You have 2 options: