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

[PATCH] commit-graph: fix truncated generation numbers

From
Patrick Steinhardt <ps@pks.im>
Date
Mar 27, 2023, 08:08 UTC
Message-ID
<f8a0a869e8b0882f05cac49d78f49ba3553d3c44.1679904401.git.ps@pks.im>

In 80c928d947 (commit-graph: simplify compute_generation_numbers(), 2023-03-20), the code to compute generation numbers was simplified to use the same infrastructure as is used to compute topological levels. This refactoring introduced a bug where the generation numbers are truncated when they exceed UINT32_MAX because we explicitly cast the computed generation number to `uint32_t`. This is not required though: both the computed value and the field of `struct commit_graph_data` are of the same type `timestamp_t` already, so casting to `uint32_t` will cause truncation.

This cast can cause us to miscompute generation data overflows:
    1. Given a commit with no parents and committer date
       `UINT32_MAX + 1`.
    2. We compute its generation number as `UINT32_MAX + 1`, but
       truncate it to `1`.
    3. We calculate the generation offset via `$generation - $date`,
       which is thus `1 - (UINT32_MAX + 1)`. The computation underflows
       and we thus end up with an offset that is bigger than the maximum
       allowed offset.

As a result, we'd be writing generation data overflow information into the commit-graph that is bogus and ultimately not even required.

Fix this bug by removing the needless cast.
Signed-off-by: Patrick Steinhardt <ps@pks.im>
---

This commit applies on top of cbfe360b14 (commit-reach: add tips_reachable_from_bases(), 2023-03-20), which has recently been merged to next.

 commit-graph.c                     | 2 +-
 t/t5328-commit-graph-64bit-time.sh | 9 +++++++++
 2 files changed, 10 insertions(+), 1 deletion(-)
diff --git a/commit-graph.c b/commit-graph.c
index 172e679db1..b96509354e 100644
--- a/commit-graph.c
+++ b/commit-graph.c
@@ -1565,7 +1565,7 @@ static timestamp_t get_generation_from_graph_data(struct commit *c, void *data)
 static void set_generation_v2(struct commit *c, timestamp_t t, void *data)
 {
 	struct commit_graph_data *g = commit_graph_data_at(c);
-	g->generation = (uint32_t)t;
+	g->generation = t;
 }
 
 static void compute_generation_numbers(struct write_commit_graph_context *ctx)
diff --git a/t/t5328-commit-graph-64bit-time.sh b/t/t5328-commit-graph-64bit-time.sh
index 093f0c067a..57e4d9c699 100755
--- a/t/t5328-commit-graph-64bit-time.sh
+++ b/t/t5328-commit-graph-64bit-time.sh
@@ -63,4 +63,13 @@ test_expect_success 'set up and verify repo with generation data overflow chunk'
 
 graph_git_behavior 'overflow 2' repo left right
 
+test_expect_success 'single commit with generation data exceeding UINT32_MAX' '
+	git init repo-uint32-max &&
+	cd repo-uint32-max &&
+	test_commit --date "@4294967297 +0000" 1 &&
+	git commit-graph write --reachable &&
+	graph_read_expect 1 "generation_data" &&
+	git commit-graph verify
+'
+
 test_done
-- 
2.40.0
Next: Junio C Hamano
Message 1 of 4 in “commit-graph: fix truncated generation numbers”
  1. commit-graph: fix truncated generation numbersPatrick Steinhardt, Mar 27, 2023
  2. Junio C HamanoMar 28, 2023
  3. Derrick StoleeMar 28, 2023
  4. Taylor BlauMar 28, 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.