Bug 1688685 - land NSS NSS_3_62_BETA1 UPGRADE_NSS_RELEASE, r=mt
authorBenjamin Beurdouche <bbeurdouche@mozilla.com>
Tue, 16 Feb 2021 10:39:36 +0000
changeset 567640 6c3c5675e3df69927e18f0a5901417a92ba85bca
parent 567639 6c46b71bb9fc90f65c03e7b7b27bf52d19893f71
child 567641 57fd0943cb502cc7da83889c8b7a1c8e26f4e52d
push id136525
push userbbeurdouche@mozilla.com
push dateTue, 16 Feb 2021 10:41:58 +0000
treeherderautoland@6c3c5675e3df [default view] [failures only]
perfherder[talos] [build metrics] [platform microbench] (compared to previous push)
reviewersmt
bugs1688685, 1688374, 1682044
milestone87.0a1
first release with
nightly linux32
nightly linux64
nightly mac
nightly win32
nightly win64
last release without
nightly linux32
nightly linux64
nightly mac
nightly win32
nightly win64
Bug 1688685 - land NSS NSS_3_62_BETA1 UPGRADE_NSS_RELEASE, r=mt ``` 2021-02-05 Danh <congdanhqx@gmail.com> * gtests/manifest.mn: Bug 1688374 - Fix parallel build NSS-3.61 with make. r=kjacobs [a5c857139b37] [NSS_3_62_BETA1] 2021-02-05 Robert Relyea <rrelyea@redhat.com> * lib/libpkix/pkix/util/pkix_tools.c: Bug 1682044 pkix_Build_GatherCerts() + pkix_CacheCert_Add() can corrupt "cachedCertTable" Patch by Andrew Cagney Preliminary Review by Ryan Sleevie Tested against all.sh rrelyea. r=kjacobs (this bug is old) pkix_Build_GatherCerts() has two code paths for creating the list "certsFound": pkix_CacheCert_Lookup() this sets "certsFound" to a new list "certsFound" and "cachedCertTable" share items but not the list pkix_CacheCert_Add(pkix_pl_Pk11CertStore_CertQuery()) this sets "certsFound" to a new list; and then adds the list to "cachedCertTable" "certsFound" and "cachedCertTable" share a linked list Because the latter doesn't create a separate list, deleting list elements from "certsFound" can also delete list elements from within "cacheCertTable". And if this happens while pkix_CacheCert_Lookup() is trying to update the same element's reference, a core dump can result. In detail (note that reference counts may occasionally seem off by 1, its because data is being captured before function local variables release their reference): pkix_Build_GatherCerts() calls pkix_pl_Pk11CertStore_CertQuery() (via a pointer) to sets "certsFound": PKIX_CHECK(getCerts (certStore, state->certSel, state->verifyNode, &nbioContext, &certsFound, plContext), PKIX_GETCERTSFAILED); it then calls: PKIX_CHECK(pkix_CacheCert_Add (certStore, certSelParams, certsFound, plContext), PKIX_CACHECERTADDFAILED); [dafda4eee75c] ``` Differential Revision: https://phabricator.services.mozilla.com/D105209
security/nss/TAG-INFO
security/nss/coreconf/coreconf.dep
security/nss/gtests/manifest.mn
security/nss/lib/libpkix/pkix/util/pkix_tools.c
--- a/security/nss/TAG-INFO
+++ b/security/nss/TAG-INFO
@@ -1,1 +1,1 @@
-fc3a4c142c16
\ No newline at end of file
+NSS_3_62_BETA1
\ No newline at end of file
--- a/security/nss/coreconf/coreconf.dep
+++ b/security/nss/coreconf/coreconf.dep
@@ -5,8 +5,9 @@
 
 /*
  * A dummy header file that is a dependency for all the object files.
  * Used to force a full recompilation of NSS in Mozilla's Tinderbox
  * depend builds.  See comments in rules.mk.
  */
 
 #error "Do not include this header file."
+
--- a/security/nss/gtests/manifest.mn
+++ b/security/nss/gtests/manifest.mn
@@ -36,16 +36,17 @@ NSS_SRCDIRS = \
 	nss_bogo_shim \
 	pkcs11testmodule \
 	$(NULL)
 
 certdb_gtest: common
 certhigh_gtest: common
 cryptohi_gtest: common
 der_gtest: common
+freebl_gtest: common
 pk11_gtest: common pkcs11testmodule
 smime_gtest: common
 softoken_gtest: common
 ssl_gtest: google_test
 sysinit_gtest: google_test
 util_gtest: common
 endif
 endif
--- a/security/nss/lib/libpkix/pkix/util/pkix_tools.c
+++ b/security/nss/lib/libpkix/pkix/util/pkix_tools.c
@@ -1158,16 +1158,17 @@ PKIX_Error *
 pkix_CacheCert_Add(
         PKIX_CertStore *store,
         PKIX_ComCertSelParams *certSelParams,
         PKIX_List* certs,
         void *plContext)
 {
         PKIX_List *cachedKeys = NULL;
         PKIX_List *cachedValues = NULL;
+        PKIX_List *cachedCerts = NULL;
         PKIX_PL_Date *cacheValidUntilDate = NULL;
         PKIX_PL_X500Name *subject = NULL;
         PKIX_Error *cachedCertError = NULL;
         PKIX_CertStore_CheckTrustCallback trustCallback = NULL;
         PKIX_UInt32 cachePeriod = CACHE_ITEM_PERIOD_SECONDS;
         PKIX_UInt32 numCerts = 0;
 
         PKIX_ENTER(BUILD, "pkix_CacheCert_Add");
@@ -1214,19 +1215,22 @@ pkix_CacheCert_Add(
                PKIX_DATECREATECURRENTOFFBYSECONDSFAILED);
 
         PKIX_CHECK(PKIX_List_AppendItem
                 (cachedValues,
                 (PKIX_PL_Object *)cacheValidUntilDate,
                 plContext),
                 PKIX_LISTAPPENDITEMFAILED);
 
+        PKIX_DUPLICATE(certs, &cachedCerts, plContext,
+                PKIX_OBJECTDUPLICATELISTFAILED);
+
         PKIX_CHECK(PKIX_List_AppendItem
                 (cachedValues,
-                (PKIX_PL_Object *)certs,
+                (PKIX_PL_Object *)cachedCerts,
                 plContext),
                 PKIX_LISTAPPENDITEMFAILED);
 
         cachedCertError = PKIX_PL_HashTable_Add
                     (cachedCertTable,
                     (PKIX_PL_Object *) cachedKeys,
                     (PKIX_PL_Object *) cachedValues,
                     plContext);
@@ -1238,16 +1242,17 @@ pkix_CacheCert_Add(
                         "entry existed\n");
         }
 
 cleanup:
 
         PKIX_DECREF(subject);
         PKIX_DECREF(cachedKeys);
         PKIX_DECREF(cachedValues);
+        PKIX_DECREF(cachedCerts);
         PKIX_DECREF(cacheValidUntilDate);
         PKIX_DECREF(cachedCertError);
 
         PKIX_RETURN(BUILD);
 }
 
 /*
  * FUNCTION: pkix_CacheCrlEntry_Lookup