upstream/bind9
1
0
Fork 0
mirror of https://github.com/isc-projects/bind9.git synced 2026-06-12 14:30:00 -04:00
Code Issues Projects Releases Packages Wiki Activity Actions

[v9_9] recursively clean empty interior nodes when deleting database records

Browse source 
4324.	[bug]		When deleting records from a zone database, interior
			nodes could be left empty but not deleted, damaging
			search performance afterward. [RT #40997]

(cherry picked from commit 44c86318ed)
(cherry picked from commit db06cd726c)
This commit is contained in:
Evan Hunt 2016-03-03 21:19:59 -08:00
parent 3d6be0705b
commit b5957e6c2d
4 changed files with 103 additions and 57 deletions
Show stats Download patch file Download diff file Expand all files Collapse all files

4
CHANGES
View file

@ -1,3 +1,7 @@
4324. [bug] When deleting records from a zone database, interior
nodes could be left empty but not deleted, damaging
search performance afterward. [RT #40997]
4323. [bug] Improve HTTP header processing on statschannel.
[RT #41674]

7
doc/arm/notes.xml
View file

@ -122,6 +122,13 @@
<section xml:id="relnotes_bugs"><info><title>Bug Fixes</title></info>
<itemizedlist>
<listitem>
<para>
When deleting records from a zone database, interior nodes
could be left empty but not deleted, damaging search
performance afterward. [RT #40997]
</para>
</listitem>
<listitem>
<para>
The server could crash due to a use-after-free if a

119
lib/dns/rbt.c
View file

@ -1371,54 +1371,91 @@ dns_rbt_deletenode(dns_rbt_t *rbt, dns_rbtnode_t *node, isc_boolean_t recurse)
}
}
/*
* Note the node that points to the level of the node that is being
* deleted. If the deleted node is the top level, parent will be set
* to NULL.
*/
parent = get_upper_node(node);
for (;;) {
/*
* Note the node that points to the level of the node
* that is being deleted. If the deleted node is the
* top level, parent will be set to NULL.
*/
parent = get_upper_node(node);
/*
* This node now has no down pointer (either because it didn't
* have one to start, or because it was recursively removed).
* So now the node needs to be removed from this level.
*/
dns_rbt_deletefromlevel(node, parent == NULL ? &rbt->root :
/*
* This node now has no down pointer (either because it didn't
* have one to start, or because it was recursively removed).
* So now the node needs to be removed from this level.
*/
dns_rbt_deletefromlevel(node, parent == NULL ? &rbt->root :
&DOWN(parent));
if (DATA(node) != NULL && rbt->data_deleter != NULL)
rbt->data_deleter(DATA(node), rbt->deleter_arg);
if (DATA(node) != NULL && rbt->data_deleter != NULL)
rbt->data_deleter(DATA(node), rbt->deleter_arg);
unhash_node(rbt, node);
unhash_node(rbt, node);
#if DNS_RBT_USEMAGIC
node->magic = 0;
node->magic = 0;
#endif
dns_rbtnode_refdestroy(node);
isc_mem_put(rbt->mctx, node, NODE_SIZE(node));
rbt->nodecount--;
dns_rbtnode_refdestroy(node);
isc_mem_put(rbt->mctx, node, NODE_SIZE(node));
rbt->nodecount--;
/*
* There are now two special cases that can exist that would
* not have existed if the tree had been created using only
* the names that now exist in it. (This is all related to
* join_nodes() as described in this function's introductory comment.)
* Both cases exist when the deleted node's parent (the node
* that pointed to the deleted node's level) is not null but
* it has no data: parent != NULL && DATA(parent) == NULL.
*
* The first case is that the deleted node was the last on its level:
* DOWN(parent) == NULL. This case can only exist if the parent was
* previously deleted -- and so now, apparently, the parent should go
* away. That can't be done though because there might be external
* references to it, such as through a nodechain.
*
* The other case also involves a parent with no data, but with the
* deleted node being the next-to-last node instead of the last:
* LEFT(DOWN(parent)) == NULL && RIGHT(DOWN(parent)) == NULL.
* Presumably now the remaining node on the level should be joined
* with the parent, but it's already been described why that can't be
* done.
*/
/*
* There are now two special cases that can exist that
* would not have existed if the tree had been created
* using only the names that now exist in it. (This is
* all related to join_nodes() as described in this
* function's introductory comment.) Both cases exist
* when the deleted node's parent (the node that pointed
* to the deleted node's level) is not null but it has
* no data: parent != NULL && DATA(parent) == NULL.
*
* The first case is that the deleted node was the last
* on its level: DOWN(parent) == NULL. This case can
* only exist if the parent was previously deleted --
* and so now, apparently, the parent should go away.
* That can't be done though because there might be
* external references to it, such as through a
* nodechain. XXXMUKS: We don't care about this.
*
* The other case also involves a parent with no data,
* but with the deleted node being the next-to-last node
* instead of the last: LEFT(DOWN(parent)) == NULL &&
* RIGHT(DOWN(parent)) == NULL. Presumably now the
* remaining node on the level should be joined with the
* parent, but it's already been described why that
* can't be done.
*/
/*
* Is this the root?.
*/
if (parent == NULL)
break;
/*
* If the node deletion did not cause the subtree to disappear
* completely, return early.
*/
if (DOWN(parent) != NULL)
break;
/*
* If the upper node is not empty, it cannot be deleted.
*/
if (DATA(parent) != NULL)
break;
/*
* If upper node is the root node (.), don't attempt to
* delete it. The root node must always exist.
*/
if (parent == rbt->root)
break;
/*
* Ascend up the tree and delete the upper node.
*/
node = parent;
}
/*
* This function never fails.

30
lib/dns/rbtdb.c
View file

@ -442,7 +442,7 @@ struct acachectl {
#define NEGATIVE(header) \
(((header)->attributes & RDATASET_ATTR_NEGATIVE) != 0)
#define DEFAULT_NODE_LOCK_COUNT 7 /*%< Should be prime. */
#define DEFAULT_NODE_LOCK_COUNT 523 /*%< Should be prime. */
/*%
* Number of buckets for cache DB entries (locks, LRU lists, TTL heaps).
@ -1637,6 +1637,19 @@ delete_node(dns_rbtdb_t *rbtdb, dns_rbtnode_t *node)
INSIST(!ISC_LINK_LINKED(node, deadlink));
if (isc_log_wouldlog(dns_lctx, ISC_LOG_DEBUG(1))) {
char printname[DNS_NAME_FORMATSIZE];
isc_log_write(dns_lctx,
DNS_LOGCATEGORY_DATABASE,
DNS_LOGMODULE_CACHE,
ISC_LOG_DEBUG(1),
"delete_node(): %p %s (bucket %d)",
node,
dns_rbt_formatnodename(node,
printname, sizeof(printname)),
node->locknum);
}
switch (node->nsec) {
case DNS_RBT_NSEC_NORMAL:
#ifdef BIND9
@ -1969,21 +1982,6 @@ decrement_reference(dns_rbtdb_t *rbtdb, dns_rbtnode_t *node,
deadlink);
}
} else {
if (isc_log_wouldlog(dns_lctx, ISC_LOG_DEBUG(1))) {
char printname[DNS_NAME_FORMATSIZE];
isc_log_write(dns_lctx,
DNS_LOGCATEGORY_DATABASE,
DNS_LOGMODULE_CACHE,
ISC_LOG_DEBUG(1),
"decrement_reference: "
"delete from rbt: %p %s",
node,
dns_rbt_formatnodename(node,
printname,
sizeof(printname)));
}
delete_node(rbtdb, node);
}
} else {