Make NSSInitSingleton::tpm_slot_ a ScopedPK11Slot.

crypto/nss_util.cc

 // Copyright (c) 2012 The Chromium Authors. All rights reserved.
 // Use of this source code is governed by a BSD-style license that can be
 // found in the LICENSE file.
 
 #include "crypto/nss_util.h"
 #include "crypto/nss_util_internal.h"
 
 #include <nss.h>
 #include <pk11pub.h>
 #include <plarena.h>
@@ skipping 368 matching lines @@
   }
 
   void OnInitializedTPMTokenAndSystemSlot(
       const base::Callback<void(bool)>& callback,
       scoped_ptr<TPMModuleAndSlot> tpm_args) {
     DCHECK(thread_checker_.CalledOnValidThread());
     DVLOG(2) << "Loaded chaps: " << !!tpm_args->chaps_module
              << ", got tpm slot: " << !!tpm_args->tpm_slot;
 
     chaps_module_ = tpm_args->chaps_module;
-    tpm_slot_ = tpm_args->tpm_slot;
+    tpm_slot_.reset(tpm_args->tpm_slot);

[Ryan Sleevi, 2014/07/29 23:43:40]

Would be great to either make this a scoped_ptr<> as well, or
PK11_ReferenceSlot. This one surprised me that it's "safe"

[pneubeck (no reviews), 2014/07/30 08:28:49]

Done.

     if (!chaps_module_ && test_system_slot_) {
       // chromeos_unittests try to test the TPM initialization process. If we
       // have a test DB open, pretend that it is the TPM slot.
-      tpm_slot_ = PK11_ReferenceSlot(test_system_slot_.get());
+      tpm_slot_.reset(PK11_ReferenceSlot(test_system_slot_.get()));
     }
     initializing_tpm_token_ = false;
 
     if (tpm_slot_) {
       TPMReadyCallbackList callback_list;
       callback_list.swap(tpm_ready_callback_list_);
       for (TPMReadyCallbackList::iterator i = callback_list.begin();
            i != callback_list.end();
            ++i) {
         (*i).Run();
       }
     }
 
     callback.Run(!!tpm_slot_);
   }
 
   bool IsTPMTokenReady(const base::Closure& callback) {
     if (!callback.is_null()) {
       // Cannot DCHECK in the general case yet, but since the callback is
       // a new addition to the API, DCHECK to make sure at least the new uses
       // don't regress.
       DCHECK(thread_checker_.CalledOnValidThread());
     } else if (!thread_checker_.CalledOnValidThread()) {
       // TODO(mattm): Change to DCHECK when callers have been fixed.
       DVLOG(1) << "Called on wrong thread.\n"
                << base::debug::StackTrace().ToString();
     }
 
-    if (tpm_slot_ != NULL)
+    if (tpm_slot_)
       return true;
 
     if (!callback.is_null())
       tpm_ready_callback_list_.push_back(callback);
 
     return false;
   }
 
   // Note that CK_SLOT_ID is an unsigned long, but cryptohome gives us the slot
   // id as an int. This should be safe since this is only used with chaps, which
@@ skipping 156 matching lines @@
                << base::debug::StackTrace().ToString();
     }
 
     return PK11_GetInternalKeySlot();
   }
 #endif
 
 #if defined(OS_CHROMEOS)
   void GetSystemNSSKeySlotCallback(
       const base::Callback<void(ScopedPK11Slot)>& callback) {
-    callback.Run(ScopedPK11Slot(PK11_ReferenceSlot(tpm_slot_)));
+    callback.Run(ScopedPK11Slot(PK11_ReferenceSlot(tpm_slot_.get())));
   }
 
   ScopedPK11Slot GetSystemNSSKeySlot(
       const base::Callback<void(ScopedPK11Slot)>& callback) {
     DCHECK(thread_checker_.CalledOnValidThread());
     // TODO(mattm): chromeos::TPMTokenloader always calls
     // InitializeTPMTokenAndSystemSlot with slot 0.  If the system slot is
     // disabled, tpm_slot_ will be the first user's slot instead. Can that be
     // detected and return NULL instead?
 
     base::Closure wrapped_callback;
     if (!callback.is_null()) {
       wrapped_callback =
           base::Bind(&NSSInitSingleton::GetSystemNSSKeySlotCallback,
                      base::Unretained(this) /* singleton is leaky */,
                      callback);
     }
     if (IsTPMTokenReady(wrapped_callback))
-      return ScopedPK11Slot(PK11_ReferenceSlot(tpm_slot_));
+      return ScopedPK11Slot(PK11_ReferenceSlot(tpm_slot_.get()));
     return ScopedPK11Slot();
   }
 #endif
 
 #if defined(USE_NSS)
   base::Lock* write_lock() {
     return &write_lock_;
   }
 #endif  // defined(USE_NSS)
 
   // This method is used to force NSS to be initialized without a DB.
   // Call this method before NSSInitSingleton() is constructed.
   static void ForceNoDBInit() {
     force_nodb_init_ = true;
   }
 
  private:
   friend struct base::DefaultLazyInstanceTraits<NSSInitSingleton>;
 
   NSSInitSingleton()
       : tpm_token_enabled_for_nss_(false),
         initializing_tpm_token_(false),
         chaps_module_(NULL),
-        tpm_slot_(NULL),
         root_(NULL) {
     base::TimeTicks start_time = base::TimeTicks::Now();
 
     // It's safe to construct on any thread, since LazyInstance will prevent any
     // other threads from accessing until the constructor is done.
     thread_checker_.DetachFromThread();
 
     DisableAESNIIfNeeded();
 
     EnsureNSPRInit();
@@ skipping 96 matching lines @@
                            50);
   }
 
   // NOTE(willchan): We don't actually execute this code since we leak NSS to
   // prevent non-joinable threads from using NSS after it's already been shut
   // down.
   ~NSSInitSingleton() {
 #if defined(OS_CHROMEOS)
     STLDeleteValues(&chromeos_user_map_);
 #endif
-    if (tpm_slot_) {
-      PK11_FreeSlot(tpm_slot_);
-      tpm_slot_ = NULL;
-    }
+    tpm_slot_.reset();
     if (root_) {
       SECMOD_UnloadUserModule(root_);
       SECMOD_DestroyModule(root_);
       root_ = NULL;
     }
     if (chaps_module_) {
       SECMOD_UnloadUserModule(chaps_module_);
       SECMOD_DestroyModule(chaps_module_);
       chaps_module_ = NULL;
     }
@@ skipping 64 matching lines @@
   }
 
   // If this is set to true NSS is forced to be initialized without a DB.
   static bool force_nodb_init_;
 
   bool tpm_token_enabled_for_nss_;
   bool initializing_tpm_token_;
   typedef std::vector<base::Closure> TPMReadyCallbackList;
   TPMReadyCallbackList tpm_ready_callback_list_;
   SECMODModule* chaps_module_;
-  PK11SlotInfo* tpm_slot_;
+  crypto::ScopedPK11Slot tpm_slot_;
   SECMODModule* root_;
 #if defined(OS_CHROMEOS)
   typedef std::map<std::string, ChromeOSUserData*> ChromeOSUserMap;
   ChromeOSUserMap chromeos_user_map_;
   ScopedPK11Slot test_system_slot_;
 #endif
 #if defined(USE_NSS)
   // TODO(davidben): When https://bugzilla.mozilla.org/show_bug.cgi?id=564011
   // is fixed, we will no longer need the lock.
   base::Lock write_lock_;
@@ skipping 235 matching lines @@
   return time.ToInternalValue() - base::Time::UnixEpoch().ToInternalValue();
 }
 
 #if !defined(OS_CHROMEOS)
 PK11SlotInfo* GetPersistentNSSKeySlot() {
   return g_nss_singleton.Get().GetPersistentNSSKeySlot();
 }
 #endif
 
 }  // namespace crypto