4366. [bug] Address race condition when updating rbtnode bit

                        fields. [RT #42379]

diff --git a/CHANGES b/CHANGES
index adb3c7f22f31180e091177f85928bd0fec2b760e..5c33ea67b10bfcb3e44b4cd66c7b2defbaa3a422 100644 (file)
--- a/CHANGES
+++ b/CHANGES
@@ -1,3 +1,6 @@
+4366.  [bug]           Address race condition when updating rbtnode bit
+                       fields. [RT #42379]
+
 4365.  [bug]           Address zone reference counting errors involving
                        nxdomain-redirect. [RT #42258]
 

diff --git a/lib/dns/include/dns/rbt.h b/lib/dns/include/dns/rbt.h
index 3df17890fd843b67b381b8ca3d26efd6ae025cb2..ef45840d12dbcc6ece1e4ae52f725ecb58fe8a56 100644 (file)
--- a/lib/dns/include/dns/rbt.h
+++ b/lib/dns/include/dns/rbt.h
@@ -92,7 +92,18 @@ struct dns_rbtnode {
         *
         * In each case below the "range" indicated is what's _necessary_ for
         * the bitfield to hold, not what it actually _can_ hold.
+        *
+        * Note: Tree lock must be held before modifying these
+        * bit-fields.
+        *
+        * Note: The two "unsigned int :0;" unnamed bitfields on either
+        * side of the bitfields below are scaffolding that border the
+        * set of bitfields which are accessed after acquiring the tree
+        * lock. Please don't insert any other bitfield members between
+        * the unnamed bitfields unless they should also be accessed
+        * after acquiring the tree lock.
         */
+       unsigned int :0;                /* start of bitfields c/o tree lock */
        unsigned int is_root : 1;       /*%< range is 0..1 */
        unsigned int color : 1;         /*%< range is 0..1 */
        unsigned int find_callback : 1; /*%< range is 0..1 */
@@ -113,16 +124,7 @@ struct dns_rbtnode {
 
        /* node needs to be cleaned from rpz */
        unsigned int rpz : 1;
-
-       /*@{*/
-       /*!
-        * These values are used in the RBT DB implementation.  The appropriate
-        * node lock must be held before accessing them.
-        */
-       unsigned int dirty:1;
-       unsigned int wild:1;
-       unsigned int locknum:DNS_RBT_LOCKLENGTH;
-       /*@}*/
+       unsigned int :0;                /* end of bitfields c/o tree lock */
 
 #ifdef DNS_RBT_USEHASH
        unsigned int hashval;
@@ -145,11 +147,30 @@ struct dns_rbtnode {
        /*!
         * These values are used in the RBT DB implementation.  The appropriate
         * node lock must be held before accessing them.
+        *
+        * Note: The two "unsigned int :0;" unnamed bitfields on either
+        * side of the bitfields below are scaffolding that border the
+        * set of bitfields which are accessed after acquiring the node
+        * lock. Please don't insert any other bitfield members between
+        * the unnamed bitfields unless they should also be accessed
+        * after acquiring the node lock.
+        *
+        * NOTE: Do not merge these fields into bitfields above, as
+        * they'll all be put in the same qword that could be accessed
+        * without the node lock as it shares the qword with other
+        * members. Leave these members here so that they occupy a
+        * separate region of memory.
         */
        void *data;
+       unsigned int :0;                /* start of bitfields c/o node lock */
+       unsigned int dirty:1;
+       unsigned int wild:1;
+       unsigned int locknum:DNS_RBT_LOCKLENGTH;
 #ifndef DNS_RBT_USEISCREFCOUNT
        unsigned int references:DNS_RBT_REFLENGTH;
-#else
+#endif
+       unsigned int :0;                /* end of bitfields c/o node lock */
+#ifdef DNS_RBT_USEISCREFCOUNT
        isc_refcount_t references; /* note that this is not in the bitfield */
 #endif
        /*@}*/

diff --git a/lib/dns/rbt.c b/lib/dns/rbt.c
index 1e2257bb43a58ee2bc4cad2d67e01f6cef1abd52..8b6a6992163843638a483d2fd59572bd1c688375 100644 (file)
--- a/lib/dns/rbt.c
+++ b/lib/dns/rbt.c
@@ -2895,6 +2895,18 @@ check_properties_helper(dns_rbtnode_t *node) {
                        return (ISC_FALSE);
        }
 
+       if ((DOWN(node) != NULL) && (!IS_ROOT(DOWN(node))))
+               return (ISC_FALSE);
+
+       if (IS_ROOT(node)) {
+               if ((PARENT(node) != NULL) &&
+                   (DOWN(PARENT(node)) != node))
+                       return (ISC_FALSE);
+
+               if (get_upper_node(node) != PARENT(node))
+                       return (ISC_FALSE);
+       }
+
        /* If node is assigned to the down_ pointer of its parent, it is
         * a subtree root and must have the flag set.
         */