Security-58286.70.7.tar.gz

[apple/security.git] / OSX / libsecurity_keychain / lib / KCCursor.cpp

diff --git a/OSX/libsecurity_keychain/lib/KCCursor.cpp b/OSX/libsecurity_keychain/lib/KCCursor.cpp
index 1c0cdb9d15766bb45b9794402254dfa142252fd4..a5d25d961b372af17e217bb9f53dd1cb633c4938 100644 (file)
--- a/OSX/libsecurity_keychain/lib/KCCursor.cpp
+++ b/OSX/libsecurity_keychain/lib/KCCursor.cpp
@@ -37,6 +37,7 @@
 #include <Security/SecKeychainItemPriv.h>
 #include <SecBase.h>
 #include <Security/SecBasePriv.h>
+#include <utilities/array_size.h>
 
 using namespace KeychainCore;
 using namespace CssmClient;
@@ -60,8 +61,8 @@ KCCursorImpl::KCCursorImpl(const StorageManager::KeychainList &searchList, SecIt
        mCurrent(mSearchList.begin()),
        mAllFailed(true),
     mDeleteInvalidRecords(false),
-       mMutex(Mutex::recursive),
-    mKeychainReadLock(NULL)
+    mIsNewKeychain(true),
+       mMutex(Mutex::recursive)
 {
     recordType(Schema::recordTypeFor(itemClass));
 
@@ -74,8 +75,9 @@ KCCursorImpl::KCCursorImpl(const StorageManager::KeychainList &searchList, SecIt
        for (const SecKeychainAttribute *attr=attrList->attr; attr != end; ++attr)
        {
                const CSSM_DB_ATTRIBUTE_INFO *temp;
-               
-               if (attr->tag <'    ') // ok, is this a key schema?  Handle differently, just because we can...
+
+               // ok, is this a key schema?  Handle differently, just because we can...
+               if (attr->tag <'    ' && attr->tag < array_size(gKeyAttributeLookupTable))
                {
                        temp = gKeyAttributeLookupTable[attr->tag];
                }
@@ -117,8 +119,8 @@ KCCursorImpl::KCCursorImpl(const StorageManager::KeychainList &searchList, const
        mCurrent(mSearchList.begin()),
        mAllFailed(true),
     mDeleteInvalidRecords(false),
-       mMutex(Mutex::recursive),
-    mKeychainReadLock(NULL)
+    mIsNewKeychain(true),
+       mMutex(Mutex::recursive)
 {
        if (!attrList) // No additional selectionPredicates: we are done
                return;
@@ -172,9 +174,6 @@ KCCursorImpl::KCCursorImpl(const StorageManager::KeychainList &searchList, const
 
 KCCursorImpl::~KCCursorImpl() throw()
 {
-    if(mKeychainReadLock) {
-        delete mKeychainReadLock;
-    }
 }
 
 //static ModuleNexus<Mutex> gActivationMutex;
@@ -187,20 +186,10 @@ KCCursorImpl::next(Item &item)
        DbUniqueRecord uniqueId;
        OSStatus status = 0;
 
-    // if this is true, we should perform newKeychain() on mCurrent before
-    // taking any locks. Starts false because mDbCursor isn't anything yet, and
-    // so the while loop will run newKeychain for us.
-    bool isNewKeychain = false;
-
        for (;;)
        {
         Item tempItem = NULL;
         {
-            if(isNewKeychain) {
-                newKeychain(mCurrent);
-                isNewKeychain = false;
-            }
-
             while (!mDbCursor)
             {
                 // Do the newKeychain dance before we check our done status
@@ -229,10 +218,12 @@ KCCursorImpl::next(Item &item)
                 catch(const CommonError &err)
                 {
                     ++mCurrent;
+                    mIsNewKeychain = true;
                 }
             }
 
             Keychain &kc = *mCurrent;
+
             Mutex* mutex = kc->getKeychainMutex();
             StLock<Mutex> _(*mutex);
 
@@ -271,7 +262,7 @@ KCCursorImpl::next(Item &item)
                 // we'd like to call newKeychain(mCurrent) here, but to avoid deadlock
                 // we need to drop the current keychain's mutex first. Use this silly
                 // hack to void the stack Mutex object.
-                isNewKeychain = true;
+                mIsNewKeychain = true;
                 continue;
             }
 
@@ -319,7 +310,7 @@ KCCursorImpl::next(Item &item)
                 if (mDeleteInvalidRecords) {
                     // This is an invalid record for some reason; delete it and restart the loop
                     const char* errStr = cssmErrorString(cssme.error);
-                    secdebugfunc("integrity", "deleting corrupt record because: %d %s", (int) cssme.error, errStr);
+                    secnotice("integrity", "deleting corrupt record because: %d %s", (int) cssme.error, errStr);
 
                     deleteInvalidRecord(uniqueId);
                     // if deleteInvalidRecord doesn't throw, we want to restart the loop
@@ -329,27 +320,6 @@ KCCursorImpl::next(Item &item)
                 }
             }
         }
-        // release the Keychain lock before checking item integrity to avoid deadlock
-
-        try {
-            // If the Item's attribute hash does not match, skip the item
-            if(!tempItem->checkIntegrity()) {
-                secdebugfunc("integrity", "item has no integrity, skipping");
-                continue;
-            }
-        } catch(CssmError cssme) {
-            if (mDeleteInvalidRecords) {
-                // This is an invalid record for some reason; delete it and restart the loop
-                const char* errStr = cssmErrorString(cssme.error);
-                secdebugfunc("integrity", "deleting corrupt record because: %d %s", (int) cssme.error, errStr);
-
-                deleteInvalidRecord(uniqueId);
-                // if deleteInvalidRecord doesn't throw, we want to restart the loop
-                continue;
-            } else {
-                throw;
-            }
-        }
 
                item = tempItem;
 
@@ -366,7 +336,7 @@ void KCCursorImpl::deleteInvalidRecord(DbUniqueRecord& uniqueId) {
         uniqueId->deleteRecord();
     } catch(CssmError delcssme) {
         if (delcssme.osStatus() == CSSMERR_DL_RECORD_NOT_FOUND) {
-            secdebugfunc("integrity", "couldn't delete nonexistent record (this is okay)");
+            secnotice("integrity", "couldn't delete nonexistent record (this is okay)");
         } else {
             throw;
         }
@@ -392,16 +362,16 @@ void KCCursorImpl::setDeleteInvalidRecords(bool deleteRecord) {
 }
 
 void KCCursorImpl::newKeychain(StorageManager::KeychainList::iterator kcIter) {
-    // Always lose the last keychain's lock
-    if(mKeychainReadLock) {
-        delete mKeychainReadLock;
-        mKeychainReadLock = NULL;
+    if(!mIsNewKeychain) {
+        // We've already been called on this keychain, don't bother.
+        return;
     }
 
     if(kcIter != mSearchList.end()) {
         (*kcIter)->performKeychainUpgradeIfNeeded();
-
-        // Grab a read lock on the keychain
-        mKeychainReadLock = new StReadWriteLock(*((*kcIter)->getKeychainReadWriteLock()), StReadWriteLock::Read);
+        (*kcIter)->tickle();
     }
+
+    // Mark down that this function has been called
+    mIsNewKeychain = false;
 }