git@vger.kernel.org mailing list mirror (one of many)
 help / color / mirror / code / Atom feed
From: Thomas Gummerer <t.gummerer@gmail.com>
To: Git mailing list <git@vger.kernel.org>
Cc: ryenus <ryenus@gmail.com>, Derrick Stolee <stolee@gmail.com>,
	Johannes Schindelin <johannes.schindelin@gmx.de>,
	Junio C Hamano <gitster@pobox.com>
Subject: [PATCH v2] linear-assignment: fix potential out of bounds memory access
Date: Thu, 13 Sep 2018 23:38:34 +0100	[thread overview]
Message-ID: <20180913223834.GF1719@hank.intra.tgummerer.com> (raw)
In-Reply-To: <20180912190108.GE4865@hank.intra.tgummerer.com>

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>
---
 linear-assignment.c   | 6 ++++++
 t/t3206-range-diff.sh | 5 +++++
 2 files changed, 11 insertions(+)

diff --git a/linear-assignment.c b/linear-assignment.c
index 9b3e56e283..ecffc09be6 100644
--- 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 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


  parent reply	other threads:[~2018-09-13 22:38 UTC|newest]

Thread overview: 19+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2018-09-11 15:25 Git 2.19 Segmentation fault 11 on macOS ryenus
2018-09-11 15:38 ` Derrick Stolee
2018-09-11 16:04   ` Derrick Stolee
2018-09-11 16:13     ` Derrick Stolee
2018-09-11 16:34       ` Thomas Gummerer
2018-09-11 17:29         ` Thomas Gummerer
2018-09-12 19:01           ` [PATCH] linear-assignment: fix potential out of bounds memory access (was: Re: Git 2.19 Segmentation fault 11 on macOS) Thomas Gummerer
2018-09-12 20:11             ` [PATCH] linear-assignment: fix potential out of bounds memory access Junio C Hamano
2018-09-12 22:44               ` Thomas Gummerer
2018-09-13  2:38             ` [PATCH] linear-assignment: fix potential out of bounds memory access (was: Re: Git 2.19 Segmentation fault 11 on macOS) Johannes Schindelin
2018-09-13 22:13               ` Thomas Gummerer
2018-09-13 10:14             ` Eric Sunshine
2018-09-13 22:38             ` Thomas Gummerer [this message]
2018-09-17 18:48               ` [PATCH v2] linear-assignment: fix potential out of bounds memory access Jonathan Nieder
2018-09-11 16:48       ` Git 2.19 Segmentation fault 11 on macOS Junio C Hamano
2018-09-11 15:47 ` Elijah Newren
2018-09-11 15:49 ` Thomas Gummerer
2018-09-11 16:03   ` ryenus
2018-09-11 16:35     ` Elijah Newren

Reply instructions:

You may reply publicly to this message via plain-text email
using any one of the following methods:

* Save the following mbox file, import it into your mail client,
  and reply-to-all from there: mbox

  Avoid top-posting and favor interleaved quoting:
  https://en.wikipedia.org/wiki/Posting_style#Interleaved_style

  List information: http://vger.kernel.org/majordomo-info.html

* Reply using the --to, --cc, and --in-reply-to
  switches of git-send-email(1):

  git send-email \
    --in-reply-to=20180913223834.GF1719@hank.intra.tgummerer.com \
    --to=t.gummerer@gmail.com \
    --cc=git@vger.kernel.org \
    --cc=gitster@pobox.com \
    --cc=johannes.schindelin@gmx.de \
    --cc=ryenus@gmail.com \
    --cc=stolee@gmail.com \
    /path/to/YOUR_REPLY

  https://kernel.org/pub/software/scm/git/docs/git-send-email.html

* If your mail client supports setting the In-Reply-To header
  via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line before the message body.
Code repositories for project(s) associated with this public inbox

	https://80x24.org/mirrors/git.git

This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for read-only IMAP folder(s) and NNTP newsgroup(s).