Security-59306.61.1.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..ad7039cf3844cd30cd3a7058ebc68d2ef604b074 100644 (file)
--- a/OSX/libsecurity_keychain/lib/KCCursor.cpp
+++ b/OSX/libsecurity_keychain/lib/KCCursor.cpp
@@ -35,8 +35,9 @@
 #include "Globals.h"
 #include "StorageManager.h"
 #include <Security/SecKeychainItemPriv.h>
 #include "Globals.h"
 #include "StorageManager.h"
 #include <Security/SecKeychainItemPriv.h>

-#include <SecBase.h>
+#include <Security/SecBase.h>

 #include <Security/SecBasePriv.h>
 #include <Security/SecBasePriv.h>

+#include <utilities/array_size.h>

 
 using namespace KeychainCore;
 using namespace CssmClient;
 
 using namespace KeychainCore;
 using namespace CssmClient;


@@ -60,8 +61,8 @@ KCCursorImpl::KCCursorImpl(const StorageManager::KeychainList &searchList, SecIt
        mCurrent(mSearchList.begin()),
        mAllFailed(true),
     mDeleteInvalidRecords(false),
        mCurrent(mSearchList.begin()),
        mAllFailed(true),
     mDeleteInvalidRecords(false),

-       mMutex(Mutex::recursive),
-    mKeychainReadLock(NULL)
+    mIsNewKeychain(true),
+       mMutex(Mutex::recursive)

 {
     recordType(Schema::recordTypeFor(itemClass));
 
 {
     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;
        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];
                }
                {
                        temp = gKeyAttributeLookupTable[attr->tag];
                }


@@ -117,8 +119,8 @@ KCCursorImpl::KCCursorImpl(const StorageManager::KeychainList &searchList, const
        mCurrent(mSearchList.begin()),
        mAllFailed(true),
     mDeleteInvalidRecords(false),
        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;
 {
        if (!attrList) // No additional selectionPredicates: we are done
                return;


@@ -172,9 +174,6 @@ KCCursorImpl::KCCursorImpl(const StorageManager::KeychainList &searchList, const
 
 KCCursorImpl::~KCCursorImpl() throw()
 {
 
 KCCursorImpl::~KCCursorImpl() throw()
 {

-    if(mKeychainReadLock) {
-        delete mKeychainReadLock;
-    }

 }
 
 //static ModuleNexus<Mutex> gActivationMutex;
 }
 
 //static ModuleNexus<Mutex> gActivationMutex;


@@ -187,20 +186,10 @@ KCCursorImpl::next(Item &item)
        DbUniqueRecord uniqueId;
        OSStatus status = 0;
 
        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;
         {
        for (;;)
        {
         Item tempItem = NULL;
         {

-            if(isNewKeychain) {
-                newKeychain(mCurrent);
-                isNewKeychain = false;
-            }
-

             while (!mDbCursor)
             {
                 // Do the newKeychain dance before we check our done status
             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;
                 catch(const CommonError &err)
                 {
                     ++mCurrent;

+                    mIsNewKeychain = true;

                 }
             }
 
             Keychain &kc = *mCurrent;
                 }
             }
 
             Keychain &kc = *mCurrent;

+

             Mutex* mutex = kc->getKeychainMutex();
             StLock<Mutex> _(*mutex);
 
             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.
                 // 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;
             }
 
                 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);
                 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
 
                     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;
 
 
                item = tempItem;
 


@@ -366,7 +336,7 @@ void KCCursorImpl::deleteInvalidRecord(DbUniqueRecord& uniqueId) {
         uniqueId->deleteRecord();
     } catch(CssmError delcssme) {
         if (delcssme.osStatus() == CSSMERR_DL_RECORD_NOT_FOUND) {
         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;
         }
         } else {
             throw;
         }


@@ -392,16 +362,16 @@ void KCCursorImpl::setDeleteInvalidRecords(bool deleteRecord) {
 }
 
 void KCCursorImpl::newKeychain(StorageManager::KeychainList::iterator kcIter) {
 }
 
 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();
     }
 
     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;

 }
 }