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

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

From
Junio C Hamano <gitster@pobox.com>
Date
Mar 14, 2025, 22:28 UTC
Message-ID
<xmqqikobdz7l.fsf@gitster.g>
In-Reply-To
<4e9b6b4c-aaa1-4c6f-93f4-7bb04607e843@web.de>
René Scharfe <l.s.r@web.de> writes:
Show 22 quoted lines
> 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(-)

Oh, I love a patch like this that is well thought out to carefully check the bounds, instead of blindly say "ah, counting number of things in size_t solves everything" ;-)

Show 23 quoted lines
> 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);
Looking good.
Thanks.  Will queue.
Previous: René ScharfeNext: René Scharfe
Message 3 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.