Bug 1754294 - take the appropriate lock when accessing SECMODModule slot information r=jschanck

When accessing a SECMODModule's slots or slotCount members, the read lock of
the module list must be acquired.

Differential Revision: https://phabricator.services.mozilla.com/D138215

security/manager/ssl/RootCertificateTelemetryUtils.cpp

@@ -110,6 +110,7 @@ int32_t RootCABinNumber(Span<const uint8_t> cert) {
   // here as a backstop to catch situations where a built-in root was added but
   // the built-in telemetry information was not updated.
   UniqueSECMODModule rootsModule(SECMOD_FindModule(kRootModuleName));
+  AutoSECMODListReadLock secmodLock;
   if (!rootsModule || rootsModule->slotCount != 1) {
     return ROOT_CERTIFICATE_UNKNOWN;
   }

security/manager/ssl/nsNSSComponent.cpp

@@ -938,10 +938,8 @@ nsresult nsNSSComponent::CheckForSmartCardChanges() {
     sLastCheckedForSmartCardChanges = now;
   }
 
-  // SECMOD_UpdateSlotList attempts to acquire the list lock as well,
-  // so we have to do this in two steps. The lock protects the list itself, so
-  // if we get our own owned references to the modules we're interested in,
-  // there's no thread safety concern here.
+  // SECMOD_UpdateSlotList attempts to acquire the list lock as well, so we
+  // have to do this in three steps.
   Vector<UniqueSECMODModule> modulesWithRemovableSlots;
   {
     AutoSECMODListReadLock secmodLock;
@@ -959,6 +957,9 @@ nsresult nsNSSComponent::CheckForSmartCardChanges() {
   for (auto& module : modulesWithRemovableSlots) {
     // Best-effort.
     Unused << SECMOD_UpdateSlotList(module.get());
+  }
+  AutoSECMODListReadLock secmodLock;
+  for (auto& module : modulesWithRemovableSlots) {
     for (int i = 0; i < module->slotCount; i++) {
       // We actually don't care about the return value here - we just need to
       // call this to get NSS to update its view of this slot.