[PATCH 2/2] fix "bundle --stdin" segfault

Previous message: [thread] [date] [author]
Next message: [thread] [date] [author]
From: Jonathan Nieder
Date: Monday, April 19, 2010 - 1:03 am

When passed an empty list, objects_array_remove_duplicates() corrupts
it by changing the number of entries from 0 to 1.

The problem lies in the condition of its main loop:

	for (ref = 0; ref < array->nr - 1; ref++) {

The loop body manipulates the supplied object array.  In the case of
an empty array, it should not be doing anything at all.  But array->nr
is an unsigned quantity, so the code enters the loop, in particular
increasing array->nr.  Fix this by comparing (ref + 1 < array->nr)
instead.

This bug can be triggered by git bundle --stdin:

	$ echo HEAD | git bundle create some.bundle --stdin’
	Segmentation fault (core dumped)

The list of commits to bundle appears to be empty because of another
bug: by the time the revision-walking machinery gets to look at it,
standard input has already been consumed by rev-list, so
...remove_duplicates() gets an empty list of revisions.

After this patch, git bundle --stdin still does not work; it just
doesn’t segfault any more.

Reported-by: Joey Hess <joey@kitenet.net>
Signed-off-by: Jonathan Nieder <jrnieder@gmail.com>
---
 object.c |    4 ++--
 1 files changed, 2 insertions(+), 2 deletions(-)

diff --git a/object.c b/object.c
index 3ca92c4..277b3dd 100644
--- a/object.c
+++ b/object.c
@@ -252,10 +252,10 @@ void add_object_array_with_mode(struct object *obj, const char *name, struct obj
 
 void object_array_remove_duplicates(struct object_array *array)
 {
-	int ref, src, dst;
+	unsigned int ref, src, dst;
 	struct object_array_entry *objects = array->objects;
 
-	for (ref = 0; ref < array->nr - 1; ref++) {
+	for (ref = 0; ref + 1 < array->nr; ref++) {
 		for (src = ref + 1, dst = src;
 		     src < array->nr;
 		     src++) {
-- 
1.7.1.rc1

--
To unsubscribe from this list: send the line "unsubscribe git" in
the body of a message to majordomo@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Previous message: [thread] [date] [author]
Next message: [thread] [date] [author]

Messages in current thread:
bug: git-bundle create foo --stdin -&gt; segfault, Joey Hess, (Mon Jan 18, 5:26 pm)
Re: bug: git-bundle create foo --stdin -&gt; segfault, Johannes Schindelin, (Tue Jan 19, 4:52 pm)
[PATCH 1/2] t5704 (bundle): add tests for bundle --stdin, Jonathan Nieder, (Mon Apr 19, 1:03 am)
[PATCH 2/2] fix "bundle --stdin" segfault, Jonathan Nieder, (Mon Apr 19, 1:03 am)
[PATCH 2/8] bundle: use libified rev-list --boundary, Jonathan Nieder, (Fri Jun 25, 11:20 pm)
[PATCH 5/8] bundle: reuse setup_revisions result, Jonathan Nieder, (Fri Jun 25, 11:22 pm)
[PATCH 6/8] Fix bundle --stdin, Jonathan Nieder, (Fri Jun 25, 11:28 pm)
[PATCH 8/8] bundle_create: Do not exit when given no revs ..., Jonathan Nieder, (Fri Jun 25, 11:31 pm)
Re: [PATCH 2/8] bundle: use libified rev-list --boundary, Junio C Hamano, (Wed Jun 30, 10:57 am)
Re: [PATCH 2/8] bundle: use libified rev-list --boundary, Jonathan Nieder, (Wed Jun 30, 1:34 pm)