linear-assignment: fix potential out of bounds memory access

Currently the 'compute_assignment()' function may read memory out
of bounds, even if used correctly.  Namely this happens when we only
have one column.  In that case we try to calculate the initial
minimum cost using '!j1' as column in the reduction transfer code.
That in turn causes us to try and get the cost from column 1 in the
cost matrix, which does not exist, and thus results in an out of
bounds memory read.

In the original paper [1], the example code initializes that minimum
cost to "infinite".  We could emulate something similar by setting the
minimum cost to INT_MAX, which would result in the same minimum cost
as the current algorithm, as we'd always go into the if condition at
least once, except when we only have one column, and column_count thus
equals 1.

If column_count does equal 1, the condition in the loop would always
be false, and we'd end up with a minimum of INT_MAX, which may lead to
integer overflows later in the algorithm.

For a column count of 1, we however do not even really need to go
through the whole algorithm.  A column count of 1 means that there's
no possible assignments, and we can just zero out the column2row and
row2column arrays, and return early from the function, while keeping
the reduction transfer part of the function the same as it is
currently.

Another solution would be to just not call the 'compute_assignment()'
function from the range diff code in this case, however it's better to
make the compute_assignment function more robust, so future callers
don't run into this potential problem.

Note that the test only fails under valgrind on Linux, but the same
command has been reported to segfault on Mac OS.

[1]: Jonker, R., & Volgenant, A. (1987). A shortest augmenting path
algorithm for dense and sparse linear assignment
problems. Computing, 38(4), 325–340.

Reported-by: ryenus <ryenus@gmail.com>
Helped-by: Derrick Stolee <stolee@gmail.com>
Signed-off-by: Thomas Gummerer <t.gummerer@gmail.com>
Signed-off-by: Junio C Hamano <gitster@pobox.com>

diff --git a/linear-assignment.c b/linear-assignment.c
index 9b3e56e283ccb98c57ca8a7b85dcf48b05b15a4f..ecffc09be6ec5c1a9646ac152a899f3415f6321d 100644 (file)
--- a/linear-assignment.c
+++ b/linear-assignment.c
@@ -19,6 +19,12 @@ void compute_assignment(int column_count, int row_count, int *cost,
        int *free_row, free_count = 0, saved_free_count, *pred, *col;
        int i, j, phase;
 
+       if (column_count < 2) {
+               memset(column2row, 0, sizeof(int) * column_count);
+               memset(row2column, 0, sizeof(int) * row_count);
+               return;
+       }
+
        memset(column2row, -1, sizeof(int) * column_count);
        memset(row2column, -1, sizeof(int) * row_count);
        ALLOC_ARRAY(v, column_count);

diff --git a/t/t3206-range-diff.sh b/t/t3206-range-diff.sh
index 2237c7f4af9464cbee087696ec3a86aeaa86e033..fb4c13a84ade63470f4dabc976ff5f5369c17e1e 100755 (executable)
--- 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