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

[PATCH] xdiff: avoid arithmetic overflow in xdl_get_hunk()

From
René Scharfe <l.s.r@web.de>
Date
Mar 14, 2025, 22:00 UTC
Message-ID
<4e9b6b4c-aaa1-4c6f-93f4-7bb04607e843@web.de>
In-Reply-To
<NYMqsJ7uttDzFT2OOEg5LLsxCSoQhTzqBs16KrMHGEKC7LzOAiYnYTEZavRQWqGH41UgjdwScwer7MssNzI7AEDHnD8GTBWvoBIqJ2e7D6g=@proton.me>

xdl_get_hunk() calculates the maximum number of common lines between two changes that would fit into the same hunk for the given context options. It involves doubling and addition and thus can overflow if the terms are huge.

The type of ctxlen and interhunkctxlen in xdemitconf_t is long, while the type of the corresponding context and interhunkcontext in struct diff_options is int. On many platforms longs are bigger that ints, which prevents the overflow. On Windows they have the same range and the overflow manifests as hunks that are split erroneously and lines being repeated between them.

Fix the overflow by checking and not going beyond LONG_MAX. This allows specifying a huge context line count and getting all lines of a changed files in a single hunk, as expected.

Reported-by: Jason Cho <jason11choca@proton.me>
Signed-off-by: René Scharfe <l.s.r@web.de>
---
 t/t4055-diff-context.sh | 10 ++++++++++
 xdiff/xemit.c           |  8 +++++++-
 2 files changed, 17 insertions(+), 1 deletion(-)
diff --git a/t/t4055-diff-context.sh b/t/t4055-diff-context.sh
index f7ff234cf9..ec2804eea6 100755
--- a/t/t4055-diff-context.sh
+++ b/t/t4055-diff-context.sh
@@ -89,4 +89,14 @@ test_expect_success '-U0 is valid, so is diff.context=0' '
 	grep "^+MODIFIED" output
 '

+test_expect_success '-U2147483647 works' '
+	echo APPENDED >>x &&
+	test_line_count = 16 x &&
+	git diff -U2147483647 >output &&
+	test_line_count = 22 output &&
+	grep "^-ADDED" output &&
+	grep "^+MODIFIED" output &&
+	grep "^+APPENDED" output
+'
+
 test_done
diff --git a/xdiff/xemit.c b/xdiff/xemit.c
index f8e3f25b03..1d40c9cb40 100644
--- a/xdiff/xemit.c
+++ b/xdiff/xemit.c
@@ -43,6 +43,10 @@ static int xdl_emit_record(xdfile_t *xdf, long ri, char const *pre, xdemitcb_t *
 	return 0;
 }

+static long saturating_add(long a, long b)
+{
+	return signed_add_overflows(a, b) ? LONG_MAX : a + b;
+}

 /*
  * Starting at the passed change atom, find the latest change atom to be included
@@ -52,7 +56,9 @@ static int xdl_emit_record(xdfile_t *xdf, long ri, char const *pre, xdemitcb_t *
 xdchange_t *xdl_get_hunk(xdchange_t **xscr, xdemitconf_t const *xecfg)
 {
 	xdchange_t *xch, *xchp, *lxch;
-	long max_common = 2 * xecfg->ctxlen + xecfg->interhunkctxlen;
+	long max_common = saturating_add(saturating_add(xecfg->ctxlen,
+							xecfg->ctxlen),
+					 xecfg->interhunkctxlen);
 	long max_ignorable = xecfg->ctxlen;
 	long ignored = 0; /* number of ignored blank lines */

--
2.48.1
Previous: Jason ChoNext: Junio C Hamano
Message 2 of 7 in “Iffy output given git diff --unified=2147483647”
  1. Jason ChoMar 12, 2025
  2. xdiff: avoid arithmetic overflow in xdl_get_hunk()René Scharfe, Mar 14, 2025
  3. Junio C HamanoMar 14, 2025
  4. René ScharfeMar 15, 2025
  5. Junio C HamanoMar 16, 2025
  6. René ScharfeMar 17, 2025
  7. Jason ChoMar 14, 2025

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.