]> git.ipfire.org Git - thirdparty/bind9.git/commitdiff
Fix race condition crash
authorMatthijs Mekking <matthijs@isc.org>
Mon, 19 Feb 2024 11:05:34 +0000 (12:05 +0100)
committerMatthijs Mekking <matthijs@isc.org>
Wed, 6 Mar 2024 09:49:02 +0000 (10:49 +0100)
When running resolver benchmark pipeline, a crash occurred:

https://gitlab.isc.org/isc-projects/bind9-shotgun-ci/-/pipelines/163946

In the code we are doing a lookup, it fails (meaning there is no node
with lookup name), we create the node and insert it and it fails.
But dns_qp_insert can only return ISC_R_SUCCESS or ISC_R_EXISTS.
So it must have been inserted in between. This is a race condition bug.

The first lookup only requires a write lock and if the lookup failed
the lock gets upgraded to a write lock and we insert the missing data.

To fix the race condition bug, we need to do a lookup again after we
have upgraded the lock to make sure it wasn't inserted in the mean
time.

lib/dns/qpdb.c

index eaba8055afec99ef631f568a47b37e7bc9db9cc6..8ed465b1821130cea74923ef86035f248b33ed3a 100644 (file)
@@ -1905,10 +1905,14 @@ dns__qpdb_findnodeintree(dns_qpdb_t *qpdb, dns_qp_t *tree,
                 * Try to upgrade the lock and if that fails unlock then relock.
                 */
                TREE_FORCEUPGRADE(&qpdb->tree_lock, &tlocktype);
-               node = dns_qpdata_create(qpdb, name);
-               result = dns_qp_insert(tree, node, 0);
-               INSIST(result == ISC_R_SUCCESS);
-               dns_qpdata_unref(node);
+               result = dns_qp_lookup(tree, name, NULL, NULL, NULL,
+                                      (void **)&node, NULL);
+               if (result != ISC_R_SUCCESS) {
+                       node = dns_qpdata_create(qpdb, name);
+                       result = dns_qp_insert(tree, node, 0);
+                       INSIST(result == ISC_R_SUCCESS);
+                       dns_qpdata_unref(node);
+               }
 
                if (tree == qpdb->tree) {
                        dns__qpzone_addwildcards(qpdb, name, true);