Author: stefan2
Date: Mon Sep 8 13:45:48 2014
New Revision: 1623398
URL: http://svn.apache.org/r1623398
Log:
Make 'svnadmin verify' dectect cycles in our DAG and prevent stack overflow
during that check. Found by fuzzing tests.
* subversion/libsvn_fs_fs/tree.c
(verify_node): Add list of parent nodes as extra parameter and check for
matches with the current node, i.e. cycles.
(svn_fs_fs__verify_root): Update caller.
Modified:
subversion/trunk/subversion/libsvn_fs_fs/tree.c
Modified: subversion/trunk/subversion/libsvn_fs_fs/tree.c
URL:
http://svn.apache.org/viewvc/subversion/trunk/subversion/libsvn_fs_fs/tree.c?rev=1623398&r1=1623397&r2=1623398&view=diff
==============================================================================
--- subversion/trunk/subversion/libsvn_fs_fs/tree.c (original)
+++ subversion/trunk/subversion/libsvn_fs_fs/tree.c Mon Sep 8 13:45:48 2014
@@ -4338,10 +4338,12 @@ stringify_node(dag_node_t *node,
/* Check metadata sanity on NODE, and on its children. Manually verify
information for DAG nodes in revision REV, and trust the metadata
- accuracy for nodes belonging to older revisions. */
+ accuracy for nodes belonging to older revisions. To detect cycles,
+ provide all parent dag_node_t * in PARENT_NODES. */
static svn_error_t *
verify_node(dag_node_t *node,
svn_revnum_t rev,
+ apr_array_header_t *parent_nodes,
apr_pool_t *pool)
{
svn_boolean_t has_mergeinfo;
@@ -4351,6 +4353,18 @@ verify_node(dag_node_t *node,
int pred_count;
svn_node_kind_t kind;
apr_pool_t *iterpool = svn_pool_create(pool);
+ int i;
+
+ /* Detect (non-)DAG cycles. */
+ for (i = 0; i < parent_nodes->nelts; ++i)
+ {
+ dag_node_t *parent = APR_ARRAY_IDX(parent_nodes, i, dag_node_t *);
+ if (svn_fs_fs__id_eq(svn_fs_fs__dag_get_id(parent),
+ svn_fs_fs__dag_get_id(node)))
+ return svn_error_createf(SVN_ERR_FS_CORRUPT, NULL,
+ "Node is its own direct or indirect parent
'%s'",
+ stringify_node(node, iterpool));
+ }
/* Fetch some data. */
SVN_ERR(svn_fs_fs__dag_has_mergeinfo(&has_mergeinfo, node));
@@ -4402,8 +4416,8 @@ verify_node(dag_node_t *node,
if (kind == svn_node_dir)
{
apr_array_header_t *entries;
- int i;
apr_int64_t children_mergeinfo = 0;
+ APR_ARRAY_PUSH(parent_nodes, dag_node_t*) = node;
SVN_ERR(svn_fs_fs__dag_dir_entries(&entries, node, pool));
@@ -4422,7 +4436,7 @@ verify_node(dag_node_t *node,
{
SVN_ERR(svn_fs_fs__dag_get_node(&child, fs, dirent->id,
iterpool));
- SVN_ERR(verify_node(child, rev, iterpool));
+ SVN_ERR(verify_node(child, rev, parent_nodes, iterpool));
SVN_ERR(svn_fs_fs__dag_get_mergeinfo_count(&child_mergeinfo,
child));
}
@@ -4447,6 +4461,10 @@ verify_node(dag_node_t *node,
stringify_node(node, iterpool),
mergeinfo_count, has_mergeinfo,
children_mergeinfo);
+
+ /* If we don't make it here, there was an error / corruption.
+ * In that case, nobody will need PARENT_NODES anymore. */
+ apr_array_pop(parent_nodes);
}
svn_pool_destroy(iterpool);
@@ -4459,6 +4477,7 @@ svn_fs_fs__verify_root(svn_fs_root_t *ro
{
svn_fs_t *fs = root->fs;
dag_node_t *root_dir;
+ apr_array_header_t *parent_nodes;
/* Issue #4129: bogus pred-counts and minfo-cnt's on the root node-rev
(and elsewhere). This code makes more thorough checks than the
@@ -4482,7 +4501,8 @@ svn_fs_fs__verify_root(svn_fs_root_t *ro
}
/* Recursively verify ROOT_DIR. */
- SVN_ERR(verify_node(root_dir, root->rev, pool));
+ parent_nodes = apr_array_make(pool, 16, sizeof(dag_node_t *));
+ SVN_ERR(verify_node(root_dir, root->rev, parent_nodes, pool));
/* Verify explicitly the predecessor of the root. */
{
