git/list[1] front-page[2] threads[3] people[4] search[5] about
 

[PATCH 05/10] add core.maxTreeDepth config

From
Jeff King <peff@peff.net>
Date
Aug 31, 2023, 06:21 UTC
Message-ID
<20230831062100.GE3185325@coredump.intra.peff.net>
In-Reply-To
<20230831061735.GA2702156@coredump.intra.peff.net>

Most of our tree traversal algorithms use recursion to visit sub-trees. For pathologically large trees, this can cause us to run out of stack space and abort in an uncontrolled way. Let's put our own limit here so that we can fail gracefully rather than segfaulting.

In similar cases where we recursed along the commit graph, we rewrote the algorithms to avoid recursion and keep any stack data on the heap. But the commit graph is meant to grow without bound, whereas it's not an imposition to put a limit on the maximum size of tree we'll handle.

And this has a bonus side effect: coupled with a limit on individual tree entry names, this limits the total size of a path we may encounter. This gives us an extra protection against code handling long path names which may suffer from integer overflows in the size (which could then be exploited by malicious trees).

The default of 4096 is set to be much longer than anybody would care about in the real world. Even with single-letter interior tree names (like "a/b/c"), such a path is at least 8191 bytes. While most operating systems will let you create such a path incrementally, trying to reference the whole thing in a system call (as Git would do when actually trying to access it) will result in ENAMETOOLONG. Coupled with the recent fsck.largePathname warning, the maximum total pathname Git will handle is (by default) 16MB.

This config option doesn't do anything yet; future patches will convert various algorithms to respect the limit.

Signed-off-by: Jeff King <peff@peff.net>
---
 Documentation/config/core.txt | 6 ++++++
 config.c                      | 5 +++++
 environment.c                 | 1 +
 environment.h                 | 1 +
 4 files changed, 13 insertions(+)
diff --git a/Documentation/config/core.txt b/Documentation/config/core.txt
index dfbdaf00b8..0e8c2832bf 100644
--- a/Documentation/config/core.txt
+++ b/Documentation/config/core.txt
@@ -736,3 +736,9 @@ core.abbrev::
 	If set to "no", no abbreviation is made and the object names
 	are shown in their full length.
 	The minimum length is 4.
+
+core.maxTreeDepth::
+	The maximum depth Git is willing to recurse while traversing a
+	tree (e.g., "a/b/cde/f" has a depth of 4). This is a fail-safe
+	to allow Git to abort cleanly, and should not generally need to
+	be adjusted. The default is 4096.
diff --git a/config.c b/config.c
index 3846a37be9..2ea39295cd 100644
--- a/config.c
+++ b/config.c
@@ -1801,6 +1801,11 @@ static int git_default_core_config(const char *var, const char *value,
 		return 0;
 	}
 
+	if (!strcmp(var, "core.maxtreedepth")) {
+		max_allowed_tree_depth = git_config_int(var, value, ctx->kvi);
+		return 0;
+	}
+
 	/* Add other config variables here and to Documentation/config.txt. */
 	return platform_core_config(var, value, ctx, cb);
 }
diff --git a/environment.c b/environment.c
index f98d76f080..8e25b5ef02 100644
--- a/environment.c
+++ b/environment.c
@@ -81,6 +81,7 @@ int merge_log_config = -1;
 int precomposed_unicode = -1; /* see probe_utf8_pathname_composition() */
 unsigned long pack_size_limit_cfg;
 enum log_refs_config log_all_ref_updates = LOG_REFS_UNSET;
+int max_allowed_tree_depth = 4096;
 
 #ifndef PROTECT_HFS_DEFAULT
 #define PROTECT_HFS_DEFAULT 0
diff --git a/environment.h b/environment.h
index c5377473c6..e5351c9dd9 100644
--- a/environment.h
+++ b/environment.h
@@ -132,6 +132,7 @@ extern size_t packed_git_limit;
 extern size_t delta_base_cache_limit;
 extern unsigned long big_file_threshold;
 extern unsigned long pack_size_limit_cfg;
+extern int max_allowed_tree_depth;
 
 /*
  * Accessors for the core.sharedrepository config which lazy-load the value
-- 
2.42.0.561.gaa987ecc69
Previous: Jeff KingNext: Jeff King
Message 6 of 16 in “tree name and depth limits”
  1. 0/10 tree name and depth limitsJeff King, Aug 31, 2023
  2. 01/10 tree-walk: reduce stack size for recursive functionsJeff King, Aug 31, 2023
  3. 02/10 tree-walk: drop MAX_TRAVERSE_TREES macroJeff King, Aug 31, 2023
  4. 03/10 tree-walk: rename "error" variableJeff King, Aug 31, 2023
  5. 04/10 fsck: detect very large tree pathnamesJeff King, Aug 31, 2023
  6. 05/10 add core.maxTreeDepth configJeff King, Aug 31, 2023
  7. 06/10 traverse_trees(): respect max_allowed_tree_depthJeff King, Aug 31, 2023
  8. 07/10 read_tree(): respect max_allowed_tree_depthJeff King, Aug 31, 2023
  9. 08/10 list-objects: respect max_allowed_tree_depthJeff King, Aug 31, 2023
  10. 09/10 tree-diff: respect max_allowed_tree_depthJeff King, Aug 31, 2023
  11. 10/10 lower core.maxTreeDepth default to 2048Jeff King, Aug 31, 2023
  12. Oswald BuddenhagenAug 31, 2023
  13. Jeff KingAug 31, 2023
  14. Junio C HamanoAug 31, 2023
  15. rsbecker@nexbridge.comAug 31, 2023
  16. Jeff KingAug 31, 2023

Read the whole thread, see it on lore, or plain text.

$ cat FOOTERMessages come from the public archive at lore.kernel.org/git, fetched every hour. The front page is chosen and written each morning by an AI editor and can be wrong; the threads themselves are the record. About and API. For agents: an MCP server at https://gitlist.dev/mcp, and any thread, story or person page as Markdown by adding .md to its URL (or sending Accept: text/markdown). Details in /llms.txt.