rust-lang / rust-lang/git2-rs

Upstream libgit2 builtin sha256 may have race condition

Open
#1,255 3 comments 0 reactions 0 assignees View on GitHub

Nobody has claimed this yet.

upstream
Dominant language
Rust
Stars
2.1k
Forks
450
Avg merge
11m
Merged PRs (30d)
1

Description

This was found in

during cargo test which the test harness libtest is multi-threaded.

thread 'indexer::tests::indexer_sha256' (11949) panicked at src/test.rs:36:14:
called `Result::unwrap()` on an `Err` value: Error { code: -33, klass: 9, message: "object hash mismatch - expected 7991f5e7c0692696aec4fdf250571006878b2667a3123fbdb5329653591af542 but got 831eed201a25d1677172d9c5e6cc953308062f5e007f5e9a28c0e38250c17d8b" }

Observed them failed on different toolchains and platforms, and only when unstable-sha256 is enabled with http feature deactivated.

After some investigation, I found that in upstream libgit2. It use a static addTemp which doesn't have any thread-safe mechanism around it: https://github.com/libgit2/libgit2/blob/d23f18fd881b8c5bb5e7a7abfcbc71ddf0c9a5ea/src/util/hash/rfc6234/sha224-256.c#L86-L91

Combined with the "without http feature" finding, I think that means the GIT_SHA256_BUILTIN is not thread safe. This is not too hard to reproduce by adjust test running in a fairly amount of threads (i.e., 16 or 32) and rerunning the entire test suite a couple of times.

cc https://github.com/rust-lang/git2-rs/pull/1206

Contributor guide

Open the contributing guide

First steps

  1. Read the whole issue, then the project's contributing guide.
  2. Comment on the issue to say you are picking it up — it saves two people doing the same work.
  3. Fork the repository and make your change on a branch.
  4. Open a pull request that references the issue number.

Research direction

Run cargo test with unstable-sha256 enabled and the http feature disabled, focusing on indexer::tests::indexer_sha256 and the failure in src/test.rs. Inspect upstream src/util/hash/rfc6234/sha224-256.c, especially the static addTemp; done means concurrent test runs no longer produce object hash mismatches.

Written by the indexing model from the issue text.

Assessment

Tech stack
git, rust
Domain
devtools
Issue type
Bug
Difficulty
4/5
Estimated time
3-5 days
Activity status
Quiet
Clarity
Mostly clear
Newbie friendliness
48/100

Get new issues in your inbox

A short digest of beginner-friendly GitHub issues.