[PATCH] Fix segmentation fault when user doesn't have access permission to the repository.

Hi, all!

    Please cc: me as I'm not subscribed. I'm sending the patch inline
only for review, probably it is mangled.
    Please use the attached patch if you agree with it. Sorry about
sending it attached.

From b2af9e783e7d8974b969c01f7a2de07b9cd5cf70 Mon Sep 17 00:00:00 2001

From: Andre Goddard Rosa <redacted>
Date: Tue, 27 Nov 2007 10:14:57 -0200
Subject: [PATCH] Fix segmentation fault when user doesn't have access
permission to the repository.

When trying to "git-pull" with my personal user in a tree owned by root,
git was crashing with segmentation fault.

Signed-off-by: Andre Goddard Rosa <redacted>
---
 builtin-fetch--tool.c |   12 ++++++++++--
 builtin-fetch.c       |   14 +++++++++++---
 2 files changed, 21 insertions(+), 5 deletions(-)

diff --git a/builtin-fetch--tool.c b/builtin-fetch--tool.c
index ed60847..7460ab7 100644
--- a/builtin-fetch--tool.c
+++ b/builtin-fetch--tool.c

@@ -511,10 +511,14 @@ int cmd_fetch__tool(int argc, const char **argv,

const char *prefix)
 	if (!strcmp("append-fetch-head", argv[1])) {
 		int result;
 		FILE *fp;
+		char *filename;

 		if (argc != 8)
 			return error("append-fetch-head takes 6 args");
-		fp = fopen(git_path("FETCH_HEAD"), "a");
+		filename = git_path("FETCH_HEAD");
+		fp = fopen(filename, "a");
+		if (!fp)
+			return error("cannot open %s: %s\n", filename, strerror(errno));
 		result = append_fetch_head(fp, argv[2], argv[3],
 					   argv[4], argv[5],
 					   argv[6], !!argv[7][0],

@@ -525,10 +529,14 @@ int cmd_fetch__tool(int argc, const char **argv,

const char *prefix)
 	if (!strcmp("native-store", argv[1])) {
 		int result;
 		FILE *fp;
+		char *filename;

 		if (argc != 5)
 			return error("fetch-native-store takes 3 args");
-		fp = fopen(git_path("FETCH_HEAD"), "a");
+		filename = git_path("FETCH_HEAD");
+		fp = fopen(filename, "a");
+		if (!fp)
+			return error("cannot open %s: %s\n", filename, strerror(errno));
 		result = fetch_native_store(fp, argv[2], argv[3], argv[4],
 					    verbose, force);
 		fclose(fp);

diff --git a/builtin-fetch.c b/builtin-fetch.c
index be9e3ea..5909d2f 100644
--- a/builtin-fetch.c
+++ b/builtin-fetch.c

@@ -263,8 +263,11 @@ static void store_updated_refs(const char *url,

struct ref *ref_map)
 	char note[1024];
 	const char *what, *kind;
 	struct ref *rm;
+	char *filename = git_path("FETCH_HEAD");

-	fp = fopen(git_path("FETCH_HEAD"), "a");
+	fp = fopen(filename, "a");
+	if (!fp)
+		return error("cannot open %s: %s\n", filename, strerror(errno));
 	for (rm = ref_map; rm; rm = rm->next) {
 		struct ref *ref = NULL;

@@ -487,8 +490,13 @@ static int do_fetch(struct transport *transport,
 		die("Don't know how to fetch from %s", transport->url);

 	/* if not appending, truncate FETCH_HEAD */
-	if (!append)
-		fclose(fopen(git_path("FETCH_HEAD"), "w"));
+	if (!append) {
+		char *filename = git_path("FETCH_HEAD");
+		int fd = fopen(filename, "w");
+		if (!fd)
+			return error("cannot open %s: %s\n", filename, strerror(errno));
+		fclose(fd);
+	}

 	ref_map = get_ref_map(transport, refs, ref_count, tags, &autotags);

-- 
1.5.3.6.861.gd794-dirty

André Goddard Rosa, Thu, Nov 22, 2007 01:59:00 +0100:

This should have been "FILE *fp", not "int fd".

+		if (!fd)
+			return error("cannot open %s: %s\n", filename, strerror(errno));
+		fclose(fd);
+	}
 
 	ref_map = get_ref_map(transport, refs, ref_count, tags, &autotags);
 
-- 
1.5.3.6.861.gd794-dirty

On Nov 22, 2007 2:09 PM, Alex Riesen [off-list ref] wrote:

André Goddard Rosa, Thu, Nov 22, 2007 01:59:00 +0100:

quoted

@@ -487,8 +490,13 @@ static int do_fetch(struct transport *transport,
              die("Don't know how to fetch from %s", transport->url);

      /* if not appending, truncate FETCH_HEAD */
-     if (!append)
-             fclose(fopen(git_path("FETCH_HEAD"), "w"));
+     if (!append) {
+             char *filename = git_path("FETCH_HEAD");
+             int fd = fopen(filename, "w");

This should have been "FILE *fp", not "int fd".

Hi, Alex!

Many thanks, you're right.

I tested it here before posting but luckly (or not, as I didn't catch
this when compiling) it worked,
as a pointer have the sizeof(int) in my x86 platform. >:|

Would you please comment on the attached patch and see if it's ok?

From dbadc5213b9957fb575c6da8528e5dd7a3f1f43e Mon Sep 17 00:00:00 2001
From: =?utf-8?q?Andr=C3=A9=20Goddard=20Rosa?= <redacted>
Date: Thu, 22 Nov 2007 20:22:23 -0200
Subject: [PATCH] Fix segmentation fault when user doesn't have access
 permission to the repository.
MIME-Version: 1.0
Content-Type: text/plain; charset=utf-8
Content-Transfer-Encoding: 8bit

Signed-off-by: André Goddard Rosa <redacted>
---
 builtin-fetch--tool.c |   12 ++++++++++--
 builtin-fetch.c       |   21 ++++++++++++++++-----
 2 files changed, 26 insertions(+), 7 deletions(-)

diff --git a/builtin-fetch--tool.c b/builtin-fetch--tool.c
index ed60847..7460ab7 100644
--- a/builtin-fetch--tool.c
+++ b/builtin-fetch--tool.c

@@ -511,10 +511,14 @@ int cmd_fetch__tool(int argc, const char **argv,

const char *prefix)
 	if (!strcmp("append-fetch-head", argv[1])) {
 		int result;
 		FILE *fp;
+		char *filename;

 		if (argc != 8)
 			return error("append-fetch-head takes 6 args");
-		fp = fopen(git_path("FETCH_HEAD"), "a");
+		filename = git_path("FETCH_HEAD");
+		fp = fopen(filename, "a");
+		if (!fp)
+			return error("cannot open %s: %s\n", filename, strerror(errno));
 		result = append_fetch_head(fp, argv[2], argv[3],
 					   argv[4], argv[5],
 					   argv[6], !!argv[7][0],

@@ -525,10 +529,14 @@ int cmd_fetch__tool(int argc, const char **argv,

const char *prefix)
 	if (!strcmp("native-store", argv[1])) {
 		int result;
 		FILE *fp;
+		char *filename;

 		if (argc != 5)
 			return error("fetch-native-store takes 3 args");
-		fp = fopen(git_path("FETCH_HEAD"), "a");
+		filename = git_path("FETCH_HEAD");
+		fp = fopen(filename, "a");
+		if (!fp)
+			return error("cannot open %s: %s\n", filename, strerror(errno));
 		result = fetch_native_store(fp, argv[2], argv[3], argv[4],
 					    verbose, force);
 		fclose(fp);

diff --git a/builtin-fetch.c b/builtin-fetch.c
index be9e3ea..84c8ed4 100644
--- a/builtin-fetch.c
+++ b/builtin-fetch.c

@@ -255,7 +255,7 @@ static int update_local_ref(struct ref *ref,
 	}
 }

-static void store_updated_refs(const char *url, struct ref *ref_map)
+static int store_updated_refs(const char *url, struct ref *ref_map)
 {
 	FILE *fp;
 	struct commit *commit;

@@ -263,8 +263,13 @@ static void store_updated_refs(const char *url,

struct ref *ref_map)
 	char note[1024];
 	const char *what, *kind;
 	struct ref *rm;
+	char *filename = git_path("FETCH_HEAD");

-	fp = fopen(git_path("FETCH_HEAD"), "a");
+	fp = fopen(filename, "a");
+	if (!fp) {
+		error("cannot open %s: %s\n", filename, strerror(errno));
+		return 1;
+	}
 	for (rm = ref_map; rm; rm = rm->next) {
 		struct ref *ref = NULL;

@@ -335,6 +340,7 @@ static void store_updated_refs(const char *url,

struct ref *ref_map)
 		}
 	}
 	fclose(fp);
+	return 0;
 }

 /*

@@ -404,7 +410,7 @@ static int fetch_refs(struct transport *transport,

struct ref *ref_map)
 	if (ret)
 		ret = transport_fetch_refs(transport, ref_map);
 	if (!ret)
-		store_updated_refs(transport->url, ref_map);
+		ret |= store_updated_refs(transport->url, ref_map);
 	transport_unlock_pack(transport);
 	return ret;
 }

@@ -487,8 +493,13 @@ static int do_fetch(struct transport *transport,
 		die("Don't know how to fetch from %s", transport->url);

 	/* if not appending, truncate FETCH_HEAD */
-	if (!append)
-		fclose(fopen(git_path("FETCH_HEAD"), "w"));
+	if (!append) {
+		char *filename = git_path("FETCH_HEAD");
+		FILE *fp = fopen(filename, "w");
+		if (!fp)
+			return error("cannot open %s: %s\n", filename, strerror(errno));
+		fclose(fp);
+	}

 	ref_map = get_ref_map(transport, refs, ref_count, tags, &autotags);

-- 
1.5.3.6.861.gd794-dirty

On Nov 22, 2007 2:09 PM, Alex Riesen [off-list ref] wrote:

André Goddard Rosa, Thu, Nov 22, 2007 01:59:00 +0100:

quoted

@@ -487,8 +490,13 @@ static int do_fetch(struct transport *transport,
              die("Don't know how to fetch from %s", transport->url);

      /* if not appending, truncate FETCH_HEAD */
-     if (!append)
-             fclose(fopen(git_path("FETCH_HEAD"), "w"));
+     if (!append) {
+             char *filename = git_path("FETCH_HEAD");
+             int fd = fopen(filename, "w");

This should have been "FILE *fp", not "int fd".

Hi, Alex!

Many thanks, you're right.

I tested it here before posting but luckly (or not, as I didn't catch
this when compiling) it worked,
as a pointer have the sizeof(int) in my x86 platform. >:|

Would you please comment on the attached patch and see if it's ok?

From dbadc5213b9957fb575c6da8528e5dd7a3f1f43e Mon Sep 17 00:00:00 2001
From: =?utf-8?q?Andr=C3=A9=20Goddard=20Rosa?= <redacted>
Date: Thu, 22 Nov 2007 20:22:23 -0200
Subject: [PATCH] Fix segmentation fault when user doesn't have access
 permission to the repository.
MIME-Version: 1.0
Content-Type: text/plain; charset=utf-8
Content-Transfer-Encoding: 8bit

Signed-off-by: André Goddard Rosa <redacted>
---
 builtin-fetch--tool.c |   12 ++++++++++--
 builtin-fetch.c       |   21 ++++++++++++++++-----
 2 files changed, 26 insertions(+), 7 deletions(-)

diff --git a/builtin-fetch--tool.c b/builtin-fetch--tool.c
index ed60847..7460ab7 100644
--- a/builtin-fetch--tool.c
+++ b/builtin-fetch--tool.c

@@ -511,10 +511,14 @@ int cmd_fetch__tool(int argc, const char **argv,

const char *prefix)
        if (!strcmp("append-fetch-head", argv[1])) {
                int result;
                FILE *fp;
+               char *filename;

                if (argc != 8)
                        return error("append-fetch-head takes 6 args");
-               fp = fopen(git_path("FETCH_HEAD"), "a");
+               filename = git_path("FETCH_HEAD");
+               fp = fopen(filename, "a");
+               if (!fp)
+                       return error("cannot open %s: %s\n", filename,
strerror(errno));
                result = append_fetch_head(fp, argv[2], argv[3],
                                           argv[4], argv[5],
                                           argv[6], !!argv[7][0],

@@ -525,10 +529,14 @@ int cmd_fetch__tool(int argc, const char **argv,

const char *prefix)
        if (!strcmp("native-store", argv[1])) {
                int result;
                FILE *fp;
+               char *filename;

                if (argc != 5)
                        return error("fetch-native-store takes 3 args");
-               fp = fopen(git_path("FETCH_HEAD"), "a");
+               filename = git_path("FETCH_HEAD");
+               fp = fopen(filename, "a");
+               if (!fp)
+                       return error("cannot open %s: %s\n", filename,
strerror(errno));
                result = fetch_native_store(fp, argv[2], argv[3], argv[4],
                                            verbose, force);
                fclose(fp);

diff --git a/builtin-fetch.c b/builtin-fetch.c
index be9e3ea..84c8ed4 100644
--- a/builtin-fetch.c
+++ b/builtin-fetch.c

@@ -255,7 +255,7 @@ static int update_local_ref(struct ref *ref,
        }
 }

-static void store_updated_refs(const char *url, struct ref *ref_map)
+static int store_updated_refs(const char *url, struct ref *ref_map)
 {
        FILE *fp;
        struct commit *commit;

@@ -263,8 +263,13 @@ static void store_updated_refs(const char *url,

struct ref *ref_map)
        char note[1024];
        const char *what, *kind;
        struct ref *rm;
+       char *filename = git_path("FETCH_HEAD");

-       fp = fopen(git_path("FETCH_HEAD"), "a");
+       fp = fopen(filename, "a");
+       if (!fp) {
+               error("cannot open %s: %s\n", filename, strerror(errno));
+               return 1;
+       }
        for (rm = ref_map; rm; rm = rm->next) {
                struct ref *ref = NULL;

@@ -335,6 +340,7 @@ static void store_updated_refs(const char *url,

struct ref *ref_map)
                }
        }
        fclose(fp);
+       return 0;
 }

 /*

@@ -404,7 +410,7 @@ static int fetch_refs(struct transport *transport,

struct ref *ref_map)
        if (ret)
                ret = transport_fetch_refs(transport, ref_map);
        if (!ret)
-               store_updated_refs(transport->url, ref_map);
+               ret |= store_updated_refs(transport->url, ref_map);
        transport_unlock_pack(transport);
        return ret;
 }

@@ -487,8 +493,13 @@ static int do_fetch(struct transport *transport,
                die("Don't know how to fetch from %s", transport->url);

        /* if not appending, truncate FETCH_HEAD */
-       if (!append)
-               fclose(fopen(git_path("FETCH_HEAD"), "w"));
+       if (!append) {
+               char *filename = git_path("FETCH_HEAD");
+               FILE *fp = fopen(filename, "w");
+               if (!fp)
+                       return error("cannot open %s: %s\n", filename,

strerror(errno));
+               fclose(fp);

+       }

        ref_map = get_ref_map(transport, refs, ref_count, tags, &autotags);

--
1.5.3.6.861.gd794-dirty



-- 
[]s,
André Goddard