Skip to content

diff: read working directory files instead of mapping them - #7397

Open
jkoppel wants to merge 1 commit into
libgit2:mainfrom
jkoppel:diff-workdir-read-not-mmap
Open

jkoppel wants to merge 1 commit into
libgit2:mainfrom
jkoppel:diff-workdir-read-not-mmap

Conversation

@jkoppel

@jkoppel jkoppel commented Oct 2, 2026

Copy link
Copy Markdown

diff_file_content_load_workdir_file() maps unfiltered working-directory files with git_futils_mmap_ro(). If another process truncates such a file while it is mapped (an editor or tool rewriting it in place), the next access to a page past the new end of file raises SIGBUS and kills the whole process. A caller can't handle it: no error is returned, and catching SIGBUS partway through a libgit2 call isn't safe. xdiff (xdl_prepare_env → xdl_hash_record/xdl_recmatch) and the OID computation (git_odb__hash → SHA1DCUpdate) both read the mapping after it is created.

We hit this in production in an application that computes status and diffs while coding agents rewrite files in the working tree. The two crash stacks we saw were:

  • git_patch_from_diff → git_xdiff → xdl_diff → xdl_prepare_env → xdl_recmatch
  • diff_file_content_load_workdir → git_odb__hash → git_hash_vec → SHA1DCUpdate

Change

Always load the file with git_futils_readbuffer_fd(). That is the path libgit2 already takes for filtered files, and the fallback it uses when mmap fails. If the file is truncated during the read, the call now fails with an ordinary error ("could not read (expected N bytes, read M)") instead of crashing. GIT_DIFF_FLAG__UNMAP_DATA is no longer set anywhere, so it is removed.

The output bytes cannot change: with no filters, git_filter_list__convert_buf() just moves the buffer.

The other mappings in libgit2 are all of files inside .git: pack windows, indexes, the commit-graph, packed-refs and loose-object streams. Those are replaced by writing a new file and renaming it into place, not by truncating the old one, so they are not affected.

Reproduction

The reproducer below commits a 4 MiB file. A forked child then keeps truncating that file and rewriting it in place, while the parent loops over git_diff_index_to_workdir() + git_patch_from_diff() for 10 seconds.

Results, 5 runs per row:

Linux (Debian bookworm, aarch64) macOS 15 (arm64)
main @ 0551dfd 5/5 killed by SIGBUS 1/5 killed by SIGBUS
v1.9.2 — 5/5 killed by SIGBUS
this PR 0/5, clean errors only 0/5, clean errors only

#7315 (private mappings) makes the crash less frequent on macOS but does not fix it. A private mapping still faults when the file underneath it is truncated.

repro.c
/*
 * libgit2 crashes with SIGBUS when a working-tree file is truncated while
 * libgit2 is diffing it.
 *
 * diff_file_content_load_workdir_file() (src/libgit2/diff_file.c) mmaps the
 * working-tree file when no filter applies. If another process truncates the
 * file before libgit2 has finished with the mapping, the next access to a
 * page past the new end of file raises SIGBUS and kills the process.
 *
 * This program commits a 4 MiB file, then forks a child that keeps truncating
 * the file to 0 bytes and writing 4 MiB back, as an editor that rewrites in
 * place does. The parent computes index->workdir patches in a loop.
 *
 * Usage: ./repro <empty-directory>
 * Expected: an error from git_patch_from_diff now and then.
 * Actual:   the parent dies with SIGBUS, usually within a second.
 */
#include <git2.h>
#include <fcntl.h>
#include <signal.h>
#include <stdio.h>
#include <stdlib.h>
#include <string.h>
#include <sys/wait.h>
#include <time.h>
#include <unistd.h>

#define FILE_SIZE (4 * 1024 * 1024)

static void check(int error, const char *what)
{
	if (error < 0) {
		const git_error *e = git_error_last();
		fprintf(stderr, "%s: %s\n", what, e ? e->message : "?");
		exit(1);
	}
}

/* 64-byte lines, all distinct, so xdiff has real work to do. */
static void fill(char *buf, char c)
{
	for (size_t line = 0; line < FILE_SIZE / 64; line++)
		snprintf(buf + line * 64, 65, "%c line %08zu %*s\n", c, line, 47, "");
}

static void write_all(int fd, const char *buf, size_t len)
{
	while (len > 0) {
		ssize_t n = write(fd, buf, len);
		if (n < 0) { perror("write"); exit(1); }
		buf += n; len -= (size_t)n;
	}
}

int main(int argc, char **argv)
{
	git_repository *repo;
	git_index *index;
	git_oid tree_id, commit_id;
	git_tree *tree;
	git_signature *sig;
	char path[4096];
	char *contents = malloc(FILE_SIZE);
	int fd;

	if (argc != 2) {
		fprintf(stderr, "usage: %s <empty-directory>\n", argv[0]);
		return 2;
	}

	git_libgit2_init();
	check(git_repository_init(&repo, argv[1], 0), "init");

	/* Commit big.txt, then make the working copy differ from it. */
	snprintf(path, sizeof(path), "%s/big.txt", argv[1]);
	fd = open(path, O_WRONLY | O_CREAT | O_TRUNC, 0644);
	fill(contents, 'a');
	write_all(fd, contents, FILE_SIZE);
	close(fd);

	check(git_repository_index(&index, repo), "index");
	check(git_index_add_bypath(index, "big.txt"), "add");
	check(git_index_write(index), "index write");
	check(git_index_write_tree(&tree_id, index), "write tree");
	check(git_tree_lookup(&tree, repo, &tree_id), "tree lookup");
	check(git_signature_now(&sig, "t", "t@t"), "signature");
	check(git_commit_create_v(&commit_id, repo, "HEAD", sig, sig, NULL,
		"init", tree, 0), "commit");

	fill(contents, 'b');

	pid_t writer = fork();
	if (writer == 0) {
		/* Another process rewriting the file in place, as editors do. */
		pid_t parent = getppid();
		fd = open(path, O_WRONLY);
		while (getppid() == parent) {
			ftruncate(fd, 0);
			lseek(fd, 0, SEEK_SET);
			write_all(fd, contents, FILE_SIZE);
			/* Rest at full size so the reader gets as far as mmap. */
			usleep(rand() % 5000);
		}
		_exit(0);
	}

	time_t deadline = time(NULL) + 10;
	unsigned long patches = 0, errors = 0;
	while (time(NULL) < deadline) {
		git_diff *diff;
		/* Hashing the file here uses read(); truncation is a clean error. */
		if (git_diff_index_to_workdir(&diff, repo, index, NULL) < 0) {
			errors++;
			continue;
		}
		for (size_t i = 0; i < git_diff_num_deltas(diff); i++) {
			git_patch *patch;
			/* Loads big.txt with mmap, then runs xdiff over the mapping. */
			if (git_patch_from_diff(&patch, diff, i) < 0)
				errors++;
			else {
				patches++;
				git_patch_free(patch);
			}
		}
		git_diff_free(diff);
	}

	kill(writer, SIGKILL);
	waitpid(writer, NULL, 0);
	printf("survived: %lu patches, %lu clean errors\n", patches, errors);
	return 0;
}

Build it with cc -o repro repro.c $(pkg-config --cflags --static --libs libgit2) and run mkdir r && ./repro r.

Tests

The full default libgit2_tests suite passes on macOS.

This PR doesn't add a unit test, because I couldn't find a deterministic one. The public API has no callback between loading the content and running xdiff, so a test can't truncate the file inside that window.

I tried truncating from the first hunk callback, after xdiff has already read the whole mapping. That did not fault on either Linux or macOS once the pages were resident, so it passed with and without the fix. That's why I didn't include it.

A fork()-based stress test like the reproducer would be timing-dependent and wouldn't run on Windows. I'm happy to add one if you'd prefer that, or a test hook in diff_file.c.

Cost

The file is copied once into heap memory instead of being mapped. libgit2 already does this for every filtered file. Files above the big-file threshold are treated as binary by size and never loaded. In our application, diffing about 3000 modified files (roughly 88 MB of working-tree reads per diff) showed no measurable timing difference.

diff_file_content_load_workdir_file() mapped unfiltered working
directory files with git_futils_mmap_ro(). If another process
truncates such a file while it is mapped (an editor or tool rewriting
it in place), the next access to a page past the new end of file
raises SIGBUS and kills the process; no error can be returned. Both
xdiff and the OID computation read the mapping after it is created.

Always load the file with git_futils_readbuffer_fd(), which was
already used for filtered files and as the fallback when mmap fails.
A file truncated during the read now produces an error instead.
Remove GIT_DIFF_FLAG__UNMAP_DATA, which nothing sets any more.

This branch has not been deployed

No deployments
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant