SecItemCopyMatching memory leaking

Question

I have a memory leak in the next code. I inspired from here and this is a part of RSA algorithm.

- (SecKeyRef)getPublicKeyRef {
OSStatus resultCode = noErr;
SecKeyRef publicKeyReference = NULL;

if(publicKey == NULL) {
    NSMutableDictionary * queryPublicKey = [[NSMutableDictionary alloc] init];

    NSData *publicTag = [NSData dataWithBytes:publicKeyIdentifier

                                       length:strlen((const char *)publicKeyIdentifier)]; 

    // Set the public key query dictionary.
    [queryPublicKey setObject:(id)kSecClassKey forKey:(id)kSecClass];
    [queryPublicKey setObject:publicTag forKey:(id)kSecAttrApplicationTag];

    [queryPublicKey setObject:(id)kSecAttrKeyTypeRSA forKey:(id)kSecAttrKeyType];

    [queryPublicKey setObject:[NSNumber numberWithBool:YES] forKey:(id)kSecReturnRef];

    // Get the key.     
    resultCode = SecItemCopyMatching((CFDictionaryRef)queryPublicKey, (CFTypeRef *)&publicKeyReference);
   // NSLog(@"getPublicKey: result code: %d", resultCode);

    if(resultCode != noErr)
    {
        publicKeyReference = NULL;
    }

    // [publicTag release];
    [queryPublicKey release];
} else {
    publicKeyReference = publicKey;
}

return publicKeyReference;

}

The Leak instrument says it is leaking in this line:

resultCode = SecItemCopyMatching((CFDictionaryRef)queryPublicKey, (CFTypeRef *)&publicKeyReference);

Please tell me how could I solve this.

Answer 1

Your method is sometimes returning an instance with retain count +1 and you are most likely not releasing it in the rest of your code. You are returning with retain count +1 if SecItemCopyMatching is called, but if publicKey is set then your function is returning a value with retain count +-0 which is bad.

You need to make sure that you always return with the same retain count. In this case, I'd do:

} else {
    publicKeyReference = publicKey;
    CFRetain(publicKeyReference);
}

Then every caller to your method must make sure to CFRelease the value... but that'd violate the get rule (it should return retain count +-0), so maybe it would be a good idea to then rename the method.