From 16abad336073665c13c09b1026dd1227688b9c10 Mon Sep 17 00:00:00 2001 From: Yannik Tausch Date: Mon, 14 Sep 2026 09:25:51 +0200 Subject: [PATCH 1/2] dir: do not apply prefix to negative pathspecs common_prefix_len() derives the common prefix solely from non-exclude pathspec items. However, match_pathspec_with_flags() also passes that prefix when matching exclude items. This can produce incorrect results because that prefix does not necessarily match an exclude item. For example, given non-exclude items "a/b" and "a/c" and an exclude item "x/b", stripping the two-byte prefix from both the pathname "a/b/m" and pattern "x/b" makes the remaining strings match and incorrectly excludes the pathname. If an exclude item is shorter than the prefix, match_pathspec_item() instead advances item->match beyond its allocation and subtracts the prefix from item->len, producing a negative matchlen. It then dereferences the out-of-bounds pointer. If the resulting byte is not NUL, matchlen is converted to size_t when passed to ps_strncmp(), which may cause a much larger out-of-bounds read. The out-of-bounds access can be reproduced with AddressSanitizer: make SANITIZE=address CFLAGS="-g -O0" git git init test && cd test && DIR=$(printf "a%.0s" {1..150}) && mkdir -p "$DIR" && touch "$DIR/f.txt" && git add -A && git commit -m test && ../git ls-files -- "$DIR/" ":(exclude)xy" Fix the bug by using a zero prefix when matching exclude items. Add regression tests for both the deterministic incorrect match and the shorter exclude item that causes the out-of-bounds access. Signed-off-by: Yannik Tausch Signed-off-by: Junio C Hamano --- dir.c | 2 +- t/t6132-pathspec-exclude.sh | 18 ++++++++++++++++++ 2 files changed, 19 insertions(+), 1 deletion(-) diff --git a/dir.c b/dir.c index 32430090dc..5f42c992d3 100644 --- a/dir.c +++ b/dir.c @@ -593,7 +593,7 @@ static int match_pathspec_with_flags(struct index_state *istate, if (!(ps->magic & PATHSPEC_EXCLUDE) || !positive) return positive; negative = do_match_pathspec(istate, ps, name, namelen, - prefix, seen, + 0, seen, flags | DO_MATCH_EXCLUDE); return negative ? 0 : positive; } diff --git a/t/t6132-pathspec-exclude.sh b/t/t6132-pathspec-exclude.sh index 9fdafeb1e9..e0c3f73ef0 100755 --- a/t/t6132-pathspec-exclude.sh +++ b/t/t6132-pathspec-exclude.sh @@ -183,6 +183,24 @@ EOF test_cmp expect actual ' +test_expect_success 'negative pathspec shorter than positive pathspec prefix' ' + git ls-files -- sub/sub/ ":(exclude)sub2" >actual && + cat <<-\EOF >expect && + sub/sub/file + sub/sub/sub/file + EOF + test_cmp expect actual +' + +test_expect_success 'exclude is matched against the full path' ' + git ls-files -- sub/sub/ ":(exclude)zzzzzzz" >actual && + cat <<-\EOF >expect && + sub/sub/file + sub/sub/sub/file + EOF + test_cmp expect actual +' + test_expect_success 'multiple exclusions' ' git ls-files -- ":^*/file2" ":^sub2" >actual && cat <<-\EOF >expect && From b6f17686b20647fc904e6201fa6d168731e588eb Mon Sep 17 00:00:00 2001 From: Yannik Tausch Date: Mon, 14 Sep 2026 09:27:05 +0200 Subject: [PATCH 2/2] dir: preserve pathspec prefix optimization with leading excludes Directory walks use the common directory prefix of non-exclude pathspec items to avoid scanning unrelated portions of the working tree or index. Exclude items only remove paths from that candidate set, so they do not need to widen the traversal. When an exclude item is the first pathspec item, common_prefix_len() fails to establish a comparison base and returns a zero-length prefix. The result is correct, but Git unnecessarily traverses from a broader starting point even when all non-exclude items share a directory. Use the first non-exclude item as the comparison base and return its string together with the prefix length, allowing callers to start from the recovered directory prefix. Exclude matching continues to use full paths, so this restores the optimization without changing which paths are selected. Add a unit test covering an exclude item before two non-exclude items with a common directory. Signed-off-by: Yannik Tausch Signed-off-by: Junio C Hamano --- dir.c | 37 +++++++++++++++++++++---------------- t/unit-tests/u-dir.c | 28 ++++++++++++++++++++++++++++ 2 files changed, 49 insertions(+), 16 deletions(-) diff --git a/dir.c b/dir.c index 5f42c992d3..abc4a78f31 100644 --- a/dir.c +++ b/dir.c @@ -212,9 +212,10 @@ static int fnmatch_icase_mem(const char *pattern, int patternlen, return match_status; } -static size_t common_prefix_len(const struct pathspec *pathspec) +static size_t common_prefix_len(const struct pathspec *pathspec, + const char **matched_prefix) { - int n; + int n, first = -1; size_t max = 0; /* @@ -237,43 +238,47 @@ static size_t common_prefix_len(const struct pathspec *pathspec) size_t i = 0, len = 0, item_len; if (pathspec->items[n].magic & PATHSPEC_EXCLUDE) continue; + if (first < 0) + first = n; if (pathspec->items[n].magic & PATHSPEC_ICASE) item_len = pathspec->items[n].prefix; else item_len = pathspec->items[n].nowildcard_len; - while (i < item_len && (n == 0 || i < max)) { + while (i < item_len && (n == first || i < max)) { char c = pathspec->items[n].match[i]; - if (c != pathspec->items[0].match[i]) + if (c != pathspec->items[first].match[i]) break; if (c == '/') len = i + 1; i++; } - if (n == 0 || len < max) { + if (n == first || len < max) { max = len; if (!max) break; } } + *matched_prefix = first < 0 ? NULL : pathspec->items[first].match; return max; } /* - * Returns a copy of the longest leading path common among all - * pathspecs. + * Returns a copy of the longest leading path common among all pathspec + * items that are not excluded. */ char *common_prefix(const struct pathspec *pathspec) { - unsigned long len = common_prefix_len(pathspec); + const char *matched_prefix; + size_t len = common_prefix_len(pathspec, &matched_prefix); - return len ? xmemdupz(pathspec->items[0].match, len) : NULL; + return len ? xmemdupz(matched_prefix, len) : NULL; } int fill_directory(struct dir_struct *dir, struct index_state *istate, const struct pathspec *pathspec) { - const char *prefix; + const char *matched_prefix; size_t prefix_len; unsigned exclusive_flags = DIR_SHOW_IGNORED | DIR_SHOW_IGNORED_TOO; @@ -284,11 +289,11 @@ int fill_directory(struct dir_struct *dir, * Calculate common prefix for the pathspec, and * use that to optimize the directory walk */ - prefix_len = common_prefix_len(pathspec); - prefix = prefix_len ? pathspec->items[0].match : ""; + prefix_len = common_prefix_len(pathspec, &matched_prefix); /* Read the directory and prune it */ - read_directory(dir, istate, prefix, prefix_len, pathspec); + read_directory(dir, istate, prefix_len ? matched_prefix : "", + prefix_len, pathspec); return prefix_len; } @@ -394,7 +399,7 @@ static int match_pathspec_item(struct index_state *istate, /* * The normal call pattern is: - * 1. prefix = common_prefix_len(ps); + * 1. prefix = common_prefix_len(ps, &matched_prefix); * 2. prune something, or fill_directory * 3. match_pathspec() * @@ -414,8 +419,8 @@ static int match_pathspec_item(struct index_state *istate, * Normally the caller (common_prefix_len() in fact) does * _exact_ matching on name[-prefix+1..-1] and we do not need * to check that part. Be defensive and check it anyway, in - * case common_prefix_len is changed, or a new caller is - * introduced that does not use common_prefix_len. + * case common_prefix_len() is changed, or a new caller is + * introduced that does not use common_prefix_len(). * * If the penalty turns out too high when prefix is really * long, maybe change it to diff --git a/t/unit-tests/u-dir.c b/t/unit-tests/u-dir.c index 2d0adaa39e..a3442c3d3c 100644 --- a/t/unit-tests/u-dir.c +++ b/t/unit-tests/u-dir.c @@ -45,3 +45,31 @@ void test_dir__within_depth(void) } + +void test_dir__common_prefix_skips_excluded_pathspec_items(void) +{ + struct pathspec_item items[] = { + { + .match = "unrelated/path", + .magic = PATHSPEC_EXCLUDE, + .nowildcard_len = 14, + }, + { + .match = "foo/bar", + .nowildcard_len = 7, + }, + { + .match = "foo/baz", + .nowildcard_len = 7, + }, + }; + struct pathspec pathspec = { + .nr = ARRAY_SIZE(items), + .magic = PATHSPEC_EXCLUDE, + .items = items, + }; + char *prefix = common_prefix(&pathspec); + + cl_assert_equal_s(prefix, "foo/"); + free(prefix); +}