Segmentation fault git read-tree

Segmentation fault git read-tree

[Len Brown]

i pushed the branches necessary to reproduce this right
before it happened, so you should be able to pull them
and try it yourself.

thanks,
-Len

[lenb@t61 acpi (test)]$ git push lenb
lenb@master.kernel.org's password:
Counting objects: 723, done.
Compressing objects: 100% (237/237), done.
Writing objects: 100% (492/492), 82.22 KiB, done.
Total 492 (delta 389), reused 337 (delta 249)
To master.kernel.org:/pub/scm/linux/kernel/git/lenb/linux-acpi-2.6.git
   93d7446..dba92d3  linus -> linus
   93d7446..dba92d3  release -> release
   93d7446..c68a500  suspend -> suspend
   93d7446..5dceb6d  test -> test
[lenb@t61 acpi (test)]$ git merge test suspend
Already up-to-date with 5dceb6d3377c3cbd06f7b3282fec0e201273c302
Trying simple merge with c68a5009ab9938af22af668e0e2d646d2482c866
/home/lenb/bin/git-merge-octopus: line 52: 24287 Segmentation fault      git read-tree -u -m --aggressive $common $MRT $SHA1
Merge with strategy octopus failed.
[lenb@t61 acpi (test)]$ git merge test suspend
fatal: unable to create '.git/index.lock': File exists
Already up-to-date with 5dceb6d3377c3cbd06f7b3282fec0e201273c302
Trying simple merge with c68a5009ab9938af22af668e0e2d646d2482c866
fatal: unable to create '.git/index.lock': File exists
Merge with strategy octopus failed.
[lenb@t61 acpi (test)]$ git --version
git version 1.5.4.4.537.gb75aa
[lenb@t61 acpi (test)]$

Re: Segmentation fault git read-tree

[Linus Torvalds]

On Thu, 13 Mar 2008, Len Brown wrote:
>
> i pushed the branches necessary to reproduce this right
> before it happened, so you should be able to pull them
> and try it yourself.

Ok, I can reproduce this, I'm on it like a maggot on a two-week-dead baby 
donkey.

		Linus

Re: Segmentation fault git read-tree

[Linus Torvalds]

On Thu, 13 Mar 2008, Linus Torvalds wrote:
>
> Ok, I can reproduce this, I'm on it like a maggot on a two-week-dead baby 
> donkey.

Ooh, interesting. Compiling with debugging makes the SIGSEGV go away, and 
replaces it with an endless loop.

The SIGSEGV when non-debugging seems to be due to a corrupt "info->prev" 
pointer chain, but this was less obvious than I hoped it would be.

Still looking.

		Linus

Re: Segmentation fault in git read-tree

[Linus Torvalds]

On Thu, 13 Mar 2008, Linus Torvalds wrote:
> 
> Ok, I can reproduce this, I'm on it like a maggot on a two-week-dead baby 
> donkey.

Ok, that was embarrassing.

This should fix it. Spot the stupid stack corruption..

		Linus

---
 unpack-trees.c |    6 ++++--
 1 files changed, 4 insertions(+), 2 deletions(-)

diff --git a/unpack-trees.c b/unpack-trees.c
index be89d52..b62b054 100644
--- a/unpack-trees.c
+++ b/unpack-trees.c
@@ -8,6 +8,8 @@
 #include "progress.h"
 #include "refs.h"
 
+#define MAX_UNPACK_TREES 4
+
 static void add_entry(struct unpack_trees_options *o, struct cache_entry *ce,
 	unsigned int set, unsigned int clear)
 {
@@ -123,7 +125,7 @@ static int unpack_index_entry(struct cache_entry *ce, struct unpack_trees_option
 int traverse_trees_recursive(int n, unsigned long dirmask, unsigned long df_conflicts, struct name_entry *names, struct traverse_info *info)
 {
 	int i;
-	struct tree_desc t[3];
+	struct tree_desc t[MAX_UNPACK_TREES];
 	struct traverse_info newinfo;
 	struct name_entry *p;
 
@@ -327,7 +329,7 @@ int unpack_trees(unsigned len, struct tree_desc *t, struct unpack_trees_options
 {
 	static struct cache_entry *dfc;
 
-	if (len > 4)
+	if (len > MAX_UNPACK_TREES)
 		die("unpack_trees takes at most four trees");
 	memset(&state, 0, sizeof(state));
 	state.base_dir = "";

Re: Segmentation fault in git read-tree

[Junio C Hamano]

Again?

Re: Segmentation fault in git read-tree

[Junio C Hamano]

Linus Torvalds <torvalds@linux-foundation.org> writes:

> On Thu, 13 Mar 2008, Linus Torvalds wrote:
>> 
>> Ok, I can reproduce this, I'm on it like a maggot on a two-week-dead baby 
>> donkey.
>
> Ok, that was embarrassing.
>
> This should fix it. Spot the stupid stack corruption..
>
> 		Linus

> diff --git a/unpack-trees.c b/unpack-trees.c
> index be89d52..b62b054 100644
> --- a/unpack-trees.c
> +++ b/unpack-trees.c
> @@ -8,6 +8,8 @@
>  #include "progress.h"
>  #include "refs.h"
>  
> +#define MAX_UNPACK_TREES 4

Somehow this reminds me of a9ab200 (Clean-up read-tree error condition.,
Aug 16, 2007) and f34f2b0 (Fix read-tree merging more than 3 trees using
3-way merge, Aug 15, 2007).

Re: Segmentation fault in git read-tree

[Junio C Hamano]

Thanks for the fix.

Let's do this right, as the two limits should be in sync.

-- >8 --
read-tree() and unpack_trees(): use consistent limit

read-tree -m can read up to MAX_TREES, which was arbitrarily set to 8 since
August 2007 (4 is needed to deal with 2 merge-base case).

However, the updated unpack_trees() code had an advertised limit of 4
(which it enforced).  In reality the code was prepared to take only 3
trees and giving 4 caused it to stomp on its stack.  Rename the MAX_TREES
constant to MAX_UNPACK_TREES, move it to the unpack-trees.h common header
file, and use it from both places to avoid future confusion.

Bug-reintroduced-and-fixed-by: Linus Torvalds <torvalds@linux-foundation.org>
Patch-munged-and-tested-by: Junio C Hamano <gitster@pobox.com>
---
 builtin-read-tree.c |    9 ++++-----
 unpack-trees.c      |    6 +++---
 unpack-trees.h      |    2 ++
 3 files changed, 9 insertions(+), 8 deletions(-)

diff --git a/builtin-read-tree.c b/builtin-read-tree.c
index 160456d..e9cfd2b 100644
--- a/builtin-read-tree.c
+++ b/builtin-read-tree.c
@@ -13,16 +13,15 @@
 #include "dir.h"
 #include "builtin.h"
 
-#define MAX_TREES 8
 static int nr_trees;
-static struct tree *trees[MAX_TREES];
+static struct tree *trees[MAX_UNPACK_TREES];
 
 static int list_tree(unsigned char *sha1)
 {
 	struct tree *tree;
 
-	if (nr_trees >= MAX_TREES)
-		die("I cannot read more than %d trees", MAX_TREES);
+	if (nr_trees >= MAX_UNPACK_TREES)
+		die("I cannot read more than %d trees", MAX_UNPACK_TREES);
 	tree = parse_tree_indirect(sha1);
 	if (!tree)
 		return -1;
@@ -97,7 +96,7 @@ int cmd_read_tree(int argc, const char **argv, const char *unused_prefix)
 {
 	int i, newfd, stage = 0;
 	unsigned char sha1[20];
-	struct tree_desc t[MAX_TREES];
+	struct tree_desc t[MAX_UNPACK_TREES];
 	struct unpack_trees_options opts;
 
 	memset(&opts, 0, sizeof(opts));
diff --git a/unpack-trees.c b/unpack-trees.c
index be89d52..91649f3 100644
--- a/unpack-trees.c
+++ b/unpack-trees.c
@@ -123,7 +123,7 @@ static int unpack_index_entry(struct cache_entry *ce, struct unpack_trees_option
 int traverse_trees_recursive(int n, unsigned long dirmask, unsigned long df_conflicts, struct name_entry *names, struct traverse_info *info)
 {
 	int i;
-	struct tree_desc t[3];
+	struct tree_desc t[MAX_UNPACK_TREES];
 	struct traverse_info newinfo;
 	struct name_entry *p;
 
@@ -327,8 +327,8 @@ int unpack_trees(unsigned len, struct tree_desc *t, struct unpack_trees_options
 {
 	static struct cache_entry *dfc;
 
-	if (len > 4)
-		die("unpack_trees takes at most four trees");
+	if (len > MAX_UNPACK_TREES)
+		die("unpack_trees takes at most %d trees", MAX_UNPACK_TREES);
 	memset(&state, 0, sizeof(state));
 	state.base_dir = "";
 	state.force = 1;
diff --git a/unpack-trees.h b/unpack-trees.h
index e8abbcd..50453ed 100644
--- a/unpack-trees.h
+++ b/unpack-trees.h
@@ -1,6 +1,8 @@
 #ifndef UNPACK_TREES_H
 #define UNPACK_TREES_H
 
+#define MAX_UNPACK_TREES 8
+
 struct unpack_trees_options;
 
 typedef int (*merge_fn_t)(struct cache_entry **src,

Re: Segmentation fault in git read-tree

[Linus Torvalds]

On Thu, 13 Mar 2008, Junio C Hamano wrote:
> 
> Somehow this reminds me of a9ab200 (Clean-up read-tree error condition.,
> Aug 16, 2007) and f34f2b0 (Fix read-tree merging more than 3 trees using
> 3-way merge, Aug 15, 2007).

Yeah. I think we don't actually have any test for more than three trees. 
If we really are supposed to do eight trees, maybe we should test for it.

		Linus

Re: Segmentation fault in git read-tree

[Len Brown]

confirmed fixed.

thanks guys, for the great support!

I'm pleased that I can (almost fearlessly) update git on a daily basis.
On the rare occasion I run into issues, you kill 'em quick.

-Len

[lenb@t61 acpi (test)]$ git merge test suspend
Already up-to-date with 5dceb6d3377c3cbd06f7b3282fec0e201273c302
Trying simple merge with c68a5009ab9938af22af668e0e2d646d2482c866
Merge made by octopus.
 arch/frv/kernel/pm.c            |    8 --
 arch/mips/au1000/common/power.c |   35 +-------
 arch/x86/kernel/apm_32.c        |   15 ---
 kernel/power/Kconfig            |   10 --
 kernel/power/Makefile           |    1 -
 kernel/power/pm.c               |  205 ---------------------------------------
 6 files changed, 1 insertions(+), 273 deletions(-)
 delete mode 100644 kernel/power/pm.c
[lenb@t61 acpi (test)]$ git version
git version 1.5.4.4.551.g1658