Re: [PATCH] linear-assignment: fix potential out of bounds memory access (was: Re: Git 2.19 Segmentation fault 11 on macOS)
- From
Johannes Schindelin <johannes.schindelin@gmx.de>
- Date
- Sep 13, 2018, 02:38 UTC
- Message-ID
- <nycvar.QRO.7.76.6.1809122136020.73@tvgsbejvaqbjf.bet>
- In-Reply-To
- <20180912190108.GE4865@hank.intra.tgummerer.com>
Hi Thomas,
[quickly, as I will go back to a proper vacation after this]
On Wed, 12 Sep 2018, Thomas Gummerer wrote:
Show 11 quoted lines
> diff --git a/linear-assignment.c b/linear-assignment.c
> index 9b3e56e283..7700b80eeb 100644
> --- a/linear-assignment.c
> +++ b/linear-assignment.c
> @@ -51,8 +51,8 @@ void compute_assignment(int column_count, int row_count, int *cost,
> else if (j1 < -1)
> row2column[i] = -2 - j1;
> else {
> - int min = COST(!j1, i) - v[!j1];
> - for (j = 1; j < column_count; j++)
> + int min = INT_MAX;I am worried about this, as I tried very hard to avoid integer overruns.
Wouldn't it be possible to replace the `else {` by an appropriate `else if (...) { ... } else {`? E.g. `else if (column_count < 2)` or some such?
Ciao, Dscho
Show 22 quoted lines
> + for (j = 0; j < column_count; j++)
> if (j != j1 && min > COST(j, i) - v[j])
> min = COST(j, i) - v[j];
> v[j1] -= min;
> diff --git a/t/t3206-range-diff.sh b/t/t3206-range-diff.sh
> index 2237c7f4af..fb4c13a84a 100755
> --- a/t/t3206-range-diff.sh
> +++ b/t/t3206-range-diff.sh
> @@ -142,4 +142,9 @@ test_expect_success 'changed message' '
> test_cmp expected actual
> '
>
> +test_expect_success 'no commits on one side' '
> + git commit --amend -m "new message" &&
> + git range-diff master HEAD@{1} HEAD
> +'
> +
> test_done
> --
> 2.19.0.397.gdd90340f6a
>
>