{"thread":{"id":"31796","subject":"git fails: segfault at 0 ip 00000000004076d5 sp 00007fff7806ebc0","startedAt":"2012-10-12T04:58:21Z","lastAt":"2012-10-12T17:12:56Z","messageCount":10,"participants":["Brad Hein","Jeff King","Erik Faye-Lund","Junio C Hamano"],"isPatch":false,"patchVersion":null,"patchTotal":null},"messages":[{"id":"201038","messageId":"CAJa+X0MefHXTu_D7-SN8itQVO4sMyc4O7=bK=-sUP+W4GHOu9g@mail.gmail.com","threadId":"31796","inReplyTo":"CAJa+X0OkzAX9E2SnDmU=on0yzzVZ9OMa2dJZgKMK=gQu2Rhf_Q@mail.gmail.com","subject":"git fails: segfault at 0 ip 00000000004076d5 sp 00007fff7806ebc0","fromName":"Brad Hein","fromEmail":"linuxbrad@gmail.com","sentAt":"2012-10-12T04:58:21Z","receivedAt":"2012-10-12T04:58:21Z","isPatch":false,"sender":{"key":"linuxbrad@gmail.com","avatar":null},"body":"In Fedora 17\nWith git-1.7.11.7-1.fc17.x86_64 (rpm)\n\nI try to clone a particular repository but git just returns, having\nnot cloned the repo. Seems like a bug. Details follow:\n  $ git clone http://gnuradio.org/git/gnuradio.git\n\nWhile the command fails a message is logged to syslog. Repeated\nattempts to clone the repo yield the same result:\n  Oct 11 21:38:25 localhost kernel: [662703.442645]\ngit-remote-http[25796]: segfault at 0 ip 00000000004076d5 sp\n00007fff7806ebc0 error 4 in git-remote-http[400000+96000]\n  Oct 11 21:39:00 localhost kernel: [662737.899829]\ngit-remote-http[25837]: segfault at 0 ip 00000000004076d5 sp\n00007fff37c5ef20 error 4 in git-remote-http[400000+96000]\n  Oct 11 21:39:25 localhost kernel: [662763.341248]\ngit-remote-http[25873]: segfault at 0 ip 00000000004076d5 sp\n00007fff6310d470 error 4 in git-remote-http[400000+96000]\n\nA tcpdump reveals that the last thing the client does is requests a\nfile that doesn't exist on the server (404). Details are in my post on\nFedoraForums: http://forums.fedoraforum.org/showthread.php?p=1607891&posted=1#post1607891\n\nProblem mitigated by downgrade to \"git-1.7.10.1-1.fc17.x86_64\" or\n\"git-1.7.11.4-3.fc17.x86_64\" or try to clone a different repository.\n"},{"id":"201039","messageId":"20121012062249.GB17026@sigill.intra.peff.net","threadId":"31796","inReplyTo":"CAJa+X0MefHXTu_D7-SN8itQVO4sMyc4O7=bK=-sUP+W4GHOu9g@mail.gmail.com","subject":"[PATCH] http: fix segfault in handle_curl_result","fromName":"Jeff King","fromEmail":"peff@peff.net","sentAt":"2012-10-12T06:22:49Z","receivedAt":"2012-10-12T06:22:49Z","isPatch":true,"sender":{"key":"peff@peff.net","avatar":"https://avatars.githubusercontent.com/u/45925?v=4"},"body":"On Fri, Oct 12, 2012 at 12:58:21AM -0400, Brad Hein wrote:\n\n> In Fedora 17\n> With git-1.7.11.7-1.fc17.x86_64 (rpm)\n> \n> I try to clone a particular repository but git just returns, having\n> not cloned the repo. Seems like a bug. Details follow:\n>   $ git clone http://gnuradio.org/git/gnuradio.git\n\nThanks for a thorough bug report. I can reproduce here, and it was easy\nto bisect. The culprit is 8809703 (http: factor out http error code\nhandling, 2012-08-27). The patch below should fix it.\n\n-- >8 --\nWhen we create an http active_request_slot, we can set its\n\"results\" pointer back to local storage. The http code will\nfill in the details of how the request went, and we can\naccess those details even after the slot has been cleaned\nup.\n\nCommit 8809703 (http: factor out http error code handling)\nswitched us from accessing our local results struct directly\nto accessing it via the \"results\" pointer of the slot. That\nmeans we're accessing the slot after it has been marked as\nfinished, defeating the whole purpose of keeping the results\nstorage separate.\n\nMost of the time this doesn't matter, as finishing the slot\ndoes not actually clean up the pointer. However, when using\ncurl's multi interface with the dumb-http revision walker,\nwe might actually start a new request before handing control\nback to the original caller. In that case, we may reuse the\nslot, zeroing its results pointer, and leading the original\ncaller to segfault while looking for its results inside the\nslot.\n\nInstead, we need to pass a pointer to our local results\nstorage to the handle_curl_result function, rather than\nrelying on the pointer in the slot struct. This matches what\nthe original code did before the refactoring (which did not\nuse a separate function, and therefore just accessed the\nresults struct directly).\n\nSigned-off-by: Jeff King <peff@peff.net>\n---\nJunio, this goes on top of the jk/maint-http-half-auth-push topic.\n\nThe bug is released in v1.7.11.7 and in v1.7.12.1. No clue how difficult\nit is to trigger in practice (it's very repeatable with this repo, but\nit does not trigger when our test scripts do dumb-http fetches, so there\nmay be something with the distribution of loose objects, packs, and so\nforth to trigger the right sequence of requests).\n\nWe should probably not be passing the slot to handle_curl_results at\nall, since it may have already been reused and is not safe to read. The\nonly thing we do with it is to set up any new auth information in the\ncurl handle.  This doesn't suffer the same problem because a reused slot\nwill always have a curl handle. However, it means we may be setting the\nauth information for a random handle. Which is OK, since all handles use\nthe same auth information anyway.  But it should also be pointless,\nbecause since dfa1725 (fix http auth with multiple curl handles) we\nalways refresh the curl handle's auth information whenever we get an\nactive slot.\n\nHowever, I'm leaving that out of this patch.  Commit 8809703 was\nsupposed to be a refactor with zero behavior changes, but it regressed.\nThis fixes the regression by behaving exactly as we did beforehand. We\ncan build the other thing on top.\n\n http.c        | 7 +++----\n http.h        | 3 ++-\n remote-curl.c | 2 +-\n 3 files changed, 6 insertions(+), 6 deletions(-)\n\ndiff --git a/http.c b/http.c\nindex 7c4a407..9334386 100644\n--- a/http.c\n+++ b/http.c\n@@ -744,10 +744,9 @@ int handle_curl_result(struct active_request_slot *slot)\n \treturn strbuf_detach(&buf, NULL);\n }\n \n-int handle_curl_result(struct active_request_slot *slot)\n+int handle_curl_result(struct active_request_slot *slot,\n+\t\t       struct slot_results *results)\n {\n-\tstruct slot_results *results = slot->results;\n-\n \tif (results->curl_result == CURLE_OK) {\n \t\tcredential_approve(&http_auth);\n \t\treturn HTTP_OK;\n@@ -818,7 +817,7 @@ static int http_request(const char *url, void *result, int target, int options)\n \n \tif (start_active_slot(slot)) {\n \t\trun_active_slot(slot);\n-\t\tret = handle_curl_result(slot);\n+\t\tret = handle_curl_result(slot, &results);\n \t} else {\n \t\terror(\"Unable to start HTTP request for %s\", url);\n \t\tret = HTTP_START_FAILED;\ndiff --git a/http.h b/http.h\nindex 12de255..0bd1e84 100644\n--- a/http.h\n+++ b/http.h\n@@ -78,7 +78,8 @@ extern void finish_all_active_slots(void);\n extern void run_active_slot(struct active_request_slot *slot);\n extern void finish_active_slot(struct active_request_slot *slot);\n extern void finish_all_active_slots(void);\n-extern int handle_curl_result(struct active_request_slot *slot);\n+extern int handle_curl_result(struct active_request_slot *slot,\n+\t\t\t      struct slot_results *results);\n \n #ifdef USE_CURL_MULTI\n extern void fill_active_slots(void);\ndiff --git a/remote-curl.c b/remote-curl.c\nindex 3ec474f..6054e47 100644\n--- a/remote-curl.c\n+++ b/remote-curl.c\n@@ -369,7 +369,7 @@ static int run_slot(struct active_request_slot *slot)\n \tslot->curl_result = curl_easy_perform(slot->curl);\n \tfinish_active_slot(slot);\n \n-\terr = handle_curl_result(slot);\n+\terr = handle_curl_result(slot, &results);\n \tif (err != HTTP_OK && err != HTTP_REAUTH) {\n \t\terror(\"RPC failed; result=%d, HTTP code = %ld\",\n \t\t      results.curl_result, results.http_code);\n-- \n1.8.0.rc2.3.g303f317\n"},{"id":"201043","messageId":"20121012073053.GC17026@sigill.intra.peff.net","threadId":"31796","inReplyTo":"20121012062249.GB17026@sigill.intra.peff.net","subject":"Re: [PATCH] http: fix segfault in handle_curl_result","fromName":"Jeff King","fromEmail":"peff@peff.net","sentAt":"2012-10-12T07:30:53Z","receivedAt":"2012-10-12T07:30:53Z","isPatch":true,"sender":{"key":"peff@peff.net","avatar":"https://avatars.githubusercontent.com/u/45925?v=4"},"body":"On Fri, Oct 12, 2012 at 02:22:49AM -0400, Jeff King wrote:\n\n> We should probably not be passing the slot to handle_curl_results at\n> all, since it may have already been reused and is not safe to read. The\n> only thing we do with it is to set up any new auth information in the\n> curl handle.  This doesn't suffer the same problem because a reused slot\n> will always have a curl handle. However, it means we may be setting the\n> auth information for a random handle. Which is OK, since all handles use\n> the same auth information anyway.  But it should also be pointless,\n> because since dfa1725 (fix http auth with multiple curl handles) we\n> always refresh the curl handle's auth information whenever we get an\n> active slot.\n> \n> However, I'm leaving that out of this patch.  Commit 8809703 was\n> supposed to be a refactor with zero behavior changes, but it regressed.\n> This fixes the regression by behaving exactly as we did beforehand. We\n> can build the other thing on top.\n\nI took a look at this, and indeed, it breaks existing code. But the\nbroken code is wrong and is easy to fix. So here is a series to do this,\non top of the one I am responding to.\n\nThese ones shouldn't have any functional impact, but do clean up the\ncode. I'd recommend the regression fix I already posted for maint and\n1.8.0, and leave these to cook for post-1.8.0.\n\n  [1/2]: remote-curl: do not call run_slot repeatedly\n  [2/2]: http: do not set up curl auth after a 401\n\n-Peff\n"},{"id":"201045","messageId":"20121012073533.GA16441@sigill.intra.peff.net","threadId":"31796","inReplyTo":"20121012073053.GC17026@sigill.intra.peff.net","subject":"[PATCH 1/2] remote-curl: do not call run_slot repeatedly","fromName":"Jeff King","fromEmail":"peff@peff.net","sentAt":"2012-10-12T07:35:33Z","receivedAt":"2012-10-12T07:35:33Z","isPatch":true,"sender":{"key":"peff@peff.net","avatar":"https://avatars.githubusercontent.com/u/45925?v=4"},"body":"Commit b81401c (http: prompt for credentials on failed POST)\ntaught post_rpc to call run_slot in a loop in order to retry\na request after asking the user for credentials. However,\nafter a call to run_slot we will have called\nfinish_active_slot. This means we have released the slot,\nand we should no longer look at it.\n\nAs it happens, this does not cause any bugs in the current\ncode, since we know that we are not using curl_multi in this\ncode path, and therefore nobody will have taken over our\nslot in the meantime. However, it is good form to actually\ncall get_active_slot again. It also future proofs us against\nchanges in the http code.\n\nWe can do this by jumping back to a retry label at the top\nof our function. We just need to reorder a few setup lines\nthat should not be repeated; everything else within the loop\nis either idempotent, needs to be repeated, or in a path we\ndo not follow (e.g., we do not even try when large_request\nis set, because we don't know how much data we might have\nstreamed from our helper program).\n\nSigned-off-by: Jeff King <peff@peff.net>\n---\nWithout this future-proofing, the next patch causes test\nfailures.\n\nIf the goto is too ugly, we can pull it out into a loop (it\njust makes the patch harder to read, because the meat of the\nfunction gets indented further).\n\n remote-curl.c | 15 ++++++++-------\n 1 file changed, 8 insertions(+), 7 deletions(-)\n\ndiff --git a/remote-curl.c b/remote-curl.c\nindex 6054e47..4281262 100644\n--- a/remote-curl.c\n+++ b/remote-curl.c\n@@ -444,6 +444,11 @@ static int post_rpc(struct rpc_state *rpc)\n \t\t\treturn -1;\n \t}\n \n+\theaders = curl_slist_append(headers, rpc->hdr_content_type);\n+\theaders = curl_slist_append(headers, rpc->hdr_accept);\n+\theaders = curl_slist_append(headers, \"Expect:\");\n+\n+retry:\n \tslot = get_active_slot();\n \n \tcurl_easy_setopt(slot->curl, CURLOPT_NOBODY, 0);\n@@ -451,10 +456,6 @@ static int post_rpc(struct rpc_state *rpc)\n \tcurl_easy_setopt(slot->curl, CURLOPT_URL, rpc->service_url);\n \tcurl_easy_setopt(slot->curl, CURLOPT_ENCODING, \"\");\n \n-\theaders = curl_slist_append(headers, rpc->hdr_content_type);\n-\theaders = curl_slist_append(headers, rpc->hdr_accept);\n-\theaders = curl_slist_append(headers, \"Expect:\");\n-\n \tif (large_request) {\n \t\t/* The request body is large and the size cannot be predicted.\n \t\t * We must use chunked encoding to send it.\n@@ -528,9 +529,9 @@ static int post_rpc(struct rpc_state *rpc)\n \tcurl_easy_setopt(slot->curl, CURLOPT_WRITEFUNCTION, rpc_in);\n \tcurl_easy_setopt(slot->curl, CURLOPT_FILE, rpc);\n \n-\tdo {\n-\t\terr = run_slot(slot);\n-\t} while (err == HTTP_REAUTH && !large_request && !use_gzip);\n+\terr = run_slot(slot);\n+\tif (err == HTTP_REAUTH && !large_request && !use_gzip)\n+\t\tgoto retry;\n \tif (err != HTTP_OK)\n \t\terr = -1;\n \n-- \n1.8.0.rc2.3.g303f317\n"},{"id":"201046","messageId":"20121012073559.GB16441@sigill.intra.peff.net","threadId":"31796","inReplyTo":"20121012073053.GC17026@sigill.intra.peff.net","subject":"[PATCH 2/2] http: do not set up curl auth after a 401","fromName":"Jeff King","fromEmail":"peff@peff.net","sentAt":"2012-10-12T07:35:59Z","receivedAt":"2012-10-12T07:35:59Z","isPatch":true,"sender":{"key":"peff@peff.net","avatar":"https://avatars.githubusercontent.com/u/45925?v=4"},"body":"When we get an http 401, we prompt for credentials and put\nthem in our global credential struct. We also feed them to\nthe curl handle that produced the 401, with the intent that\nthey will be used on a retry.\n\nWhen the code was originally introduced in commit 42653c0,\nthis was a necessary step. However, since dfa1725, we always\nfeed our global credential into every curl handle when we\ninitialize the slot with get_active_slot. So every further\nrequest already feeds the credential to curl.\n\nMoreover, accessing the slot here is somewhat dubious. After\nthe slot has produced a response, we don't actually control\nit any more.  If we are using curl_multi, it may even have\nbeen re-initialized to handle a different request.\n\nIt just so happens that we will reuse the curl handle within\nthe slot in such a case, and that because we only keep one\nglobal credential, it will be the one we want.  So the\ncurrent code is not buggy, but it is misleading.\n\nBy cleaning it up, we can remove the slot argument entirely\nfrom handle_curl_result, making it much more obvious that\nslots should not be accessed after they are marked as\nfinished.\n\nSigned-off-by: Jeff King <peff@peff.net>\n---\n http.c        | 6 ++----\n http.h        | 3 +--\n remote-curl.c | 2 +-\n 3 files changed, 4 insertions(+), 7 deletions(-)\n\ndiff --git a/http.c b/http.c\nindex 9334386..0a74345 100644\n--- a/http.c\n+++ b/http.c\n@@ -744,8 +744,7 @@ char *get_remote_object_url(const char *url, const char *hex,\n \treturn strbuf_detach(&buf, NULL);\n }\n \n-int handle_curl_result(struct active_request_slot *slot,\n-\t\t       struct slot_results *results)\n+int handle_curl_result(struct slot_results *results)\n {\n \tif (results->curl_result == CURLE_OK) {\n \t\tcredential_approve(&http_auth);\n@@ -758,7 +757,6 @@ int handle_curl_result(struct active_request_slot *slot,\n \t\t\treturn HTTP_NOAUTH;\n \t\t} else {\n \t\t\tcredential_fill(&http_auth);\n-\t\t\tinit_curl_http_auth(slot->curl);\n \t\t\treturn HTTP_REAUTH;\n \t\t}\n \t} else {\n@@ -817,7 +815,7 @@ static int http_request(const char *url, void *result, int target, int options)\n \n \tif (start_active_slot(slot)) {\n \t\trun_active_slot(slot);\n-\t\tret = handle_curl_result(slot, &results);\n+\t\tret = handle_curl_result(&results);\n \t} else {\n \t\terror(\"Unable to start HTTP request for %s\", url);\n \t\tret = HTTP_START_FAILED;\ndiff --git a/http.h b/http.h\nindex 0bd1e84..0a80d30 100644\n--- a/http.h\n+++ b/http.h\n@@ -78,8 +78,7 @@ extern void finish_all_active_slots(void);\n extern void run_active_slot(struct active_request_slot *slot);\n extern void finish_active_slot(struct active_request_slot *slot);\n extern void finish_all_active_slots(void);\n-extern int handle_curl_result(struct active_request_slot *slot,\n-\t\t\t      struct slot_results *results);\n+extern int handle_curl_result(struct slot_results *results);\n \n #ifdef USE_CURL_MULTI\n extern void fill_active_slots(void);\ndiff --git a/remote-curl.c b/remote-curl.c\nindex 4281262..aefafd3 100644\n--- a/remote-curl.c\n+++ b/remote-curl.c\n@@ -369,7 +369,7 @@ static int run_slot(struct active_request_slot *slot)\n \tslot->curl_result = curl_easy_perform(slot->curl);\n \tfinish_active_slot(slot);\n \n-\terr = handle_curl_result(slot, &results);\n+\terr = handle_curl_result(&results);\n \tif (err != HTTP_OK && err != HTTP_REAUTH) {\n \t\terror(\"RPC failed; result=%d, HTTP code = %ld\",\n \t\t      results.curl_result, results.http_code);\n-- \n1.8.0.rc2.3.g303f317\n"},{"id":"201054","messageId":"CAJa+X0OHsmQakhMr8QRwO6n9Ni9gXvXWnFTKFdb7X_qoDmEsvQ@mail.gmail.com","threadId":"31796","inReplyTo":"20121012073559.GB16441@sigill.intra.peff.net","subject":"Re: [PATCH 2/2] http: do not set up curl auth after a 401","fromName":"Brad Hein","fromEmail":"linuxbrad@gmail.com","sentAt":"2012-10-12T13:39:17Z","receivedAt":"2012-10-12T13:39:17Z","isPatch":true,"sender":{"key":"linuxbrad@gmail.com","avatar":null},"body":"Nice work sorting this out. I don't see the commit on github\n(https://github.com/git/git/commits/master) yet but once the code is\navailable I'll be happy to re-test if needed.\n\n\nOn Fri, Oct 12, 2012 at 3:35 AM, Jeff King <peff@peff.net> wrote:\n> When we get an http 401, we prompt for credentials and put\n> them in our global credential struct. We also feed them to\n> the curl handle that produced the 401, with the intent that\n> they will be used on a retry.\n>\n> When the code was originally introduced in commit 42653c0,\n> this was a necessary step. However, since dfa1725, we always\n> feed our global credential into every curl handle when we\n> initialize the slot with get_active_slot. So every further\n> request already feeds the credential to curl.\n>\n> Moreover, accessing the slot here is somewhat dubious. After\n> the slot has produced a response, we don't actually control\n> it any more.  If we are using curl_multi, it may even have\n> been re-initialized to handle a different request.\n>\n> It just so happens that we will reuse the curl handle within\n> the slot in such a case, and that because we only keep one\n> global credential, it will be the one we want.  So the\n> current code is not buggy, but it is misleading.\n>\n> By cleaning it up, we can remove the slot argument entirely\n> from handle_curl_result, making it much more obvious that\n> slots should not be accessed after they are marked as\n> finished.\n>\n> Signed-off-by: Jeff King <peff@peff.net>\n> ---\n>  http.c        | 6 ++----\n>  http.h        | 3 +--\n>  remote-curl.c | 2 +-\n>  3 files changed, 4 insertions(+), 7 deletions(-)\n>\n> diff --git a/http.c b/http.c\n> index 9334386..0a74345 100644\n> --- a/http.c\n> +++ b/http.c\n> @@ -744,8 +744,7 @@ char *get_remote_object_url(const char *url, const char *hex,\n>         return strbuf_detach(&buf, NULL);\n>  }\n>\n> -int handle_curl_result(struct active_request_slot *slot,\n> -                      struct slot_results *results)\n> +int handle_curl_result(struct slot_results *results)\n>  {\n>         if (results->curl_result == CURLE_OK) {\n>                 credential_approve(&http_auth);\n> @@ -758,7 +757,6 @@ int handle_curl_result(struct active_request_slot *slot,\n>                         return HTTP_NOAUTH;\n>                 } else {\n>                         credential_fill(&http_auth);\n> -                       init_curl_http_auth(slot->curl);\n>                         return HTTP_REAUTH;\n>                 }\n>         } else {\n> @@ -817,7 +815,7 @@ static int http_request(const char *url, void *result, int target, int options)\n>\n>         if (start_active_slot(slot)) {\n>                 run_active_slot(slot);\n> -               ret = handle_curl_result(slot, &results);\n> +               ret = handle_curl_result(&results);\n>         } else {\n>                 error(\"Unable to start HTTP request for %s\", url);\n>                 ret = HTTP_START_FAILED;\n> diff --git a/http.h b/http.h\n> index 0bd1e84..0a80d30 100644\n> --- a/http.h\n> +++ b/http.h\n> @@ -78,8 +78,7 @@ extern void finish_all_active_slots(void);\n>  extern void run_active_slot(struct active_request_slot *slot);\n>  extern void finish_active_slot(struct active_request_slot *slot);\n>  extern void finish_all_active_slots(void);\n> -extern int handle_curl_result(struct active_request_slot *slot,\n> -                             struct slot_results *results);\n> +extern int handle_curl_result(struct slot_results *results);\n>\n>  #ifdef USE_CURL_MULTI\n>  extern void fill_active_slots(void);\n> diff --git a/remote-curl.c b/remote-curl.c\n> index 4281262..aefafd3 100644\n> --- a/remote-curl.c\n> +++ b/remote-curl.c\n> @@ -369,7 +369,7 @@ static int run_slot(struct active_request_slot *slot)\n>         slot->curl_result = curl_easy_perform(slot->curl);\n>         finish_active_slot(slot);\n>\n> -       err = handle_curl_result(slot, &results);\n> +       err = handle_curl_result(&results);\n>         if (err != HTTP_OK && err != HTTP_REAUTH) {\n>                 error(\"RPC failed; result=%d, HTTP code = %ld\",\n>                       results.curl_result, results.http_code);\n> --\n> 1.8.0.rc2.3.g303f317\n"},{"id":"201062","messageId":"CABPQNSYyh-gk8aD9SKLzo0MDBkGECthdGZiT1mB1pA0ajdB8Cw@mail.gmail.com","threadId":"31796","inReplyTo":"CAJa+X0MefHXTu_D7-SN8itQVO4sMyc4O7=bK=-sUP+W4GHOu9g@mail.gmail.com","subject":"Re: git fails: segfault at 0 ip 00000000004076d5 sp 00007fff7806ebc0","fromName":"Erik Faye-Lund","fromEmail":"kusmabite@gmail.com","sentAt":"2012-10-12T16:29:37Z","receivedAt":"2012-10-12T16:29:37Z","isPatch":false,"sender":{"key":"kusmabite@gmail.com","avatar":"https://avatars.githubusercontent.com/u/47073?v=4"},"body":"On Fri, Oct 12, 2012 at 6:58 AM, Brad Hein <linuxbrad@gmail.com> wrote:\n> In Fedora 17\n> With git-1.7.11.7-1.fc17.x86_64 (rpm)\n>\n> I try to clone a particular repository but git just returns, having\n> not cloned the repo. Seems like a bug. Details follow:\n>   $ git clone http://gnuradio.org/git/gnuradio.git\n>\n> While the command fails a message is logged to syslog. Repeated\n> attempts to clone the repo yield the same result:\n>   Oct 11 21:38:25 localhost kernel: [662703.442645]\n> git-remote-http[25796]: segfault at 0 ip 00000000004076d5 sp\n> 00007fff7806ebc0 error 4 in git-remote-http[400000+96000]\n>   Oct 11 21:39:00 localhost kernel: [662737.899829]\n> git-remote-http[25837]: segfault at 0 ip 00000000004076d5 sp\n> 00007fff37c5ef20 error 4 in git-remote-http[400000+96000]\n>   Oct 11 21:39:25 localhost kernel: [662763.341248]\n> git-remote-http[25873]: segfault at 0 ip 00000000004076d5 sp\n> 00007fff6310d470 error 4 in git-remote-http[400000+96000]\n>\n> A tcpdump reveals that the last thing the client does is requests a\n> file that doesn't exist on the server (404). Details are in my post on\n> FedoraForums: http://forums.fedoraforum.org/showthread.php?p=1607891&posted=1#post1607891\n>\n> Problem mitigated by downgrade to \"git-1.7.10.1-1.fc17.x86_64\" or\n> \"git-1.7.11.4-3.fc17.x86_64\" or try to clone a different repository.\n\nThanks for reporting. I gave it a quick go, and the issue seems to\nalso be present in the current 'master'.\n\nThe problem is a NULL-pointer dereferencing introduced in 8809703\n(\"http: factor out http error code handling\"), where the code assume\nthat slot->results still points to http_request::results. This\nassumption seems to be wrong.\n\nThis seems to step around the issue, but I don't know if\nhttp_request::results should be set to NULL in the first place. Jeff?\n\ndiff --git a/http.c b/http.c\nindex 345c171..ac3ab16 100644\n--- a/http.c\n+++ b/http.c\n@@ -745,10 +745,8 @@ char *get_remote_object_url(const char *url,\nconst char *hex,\n \treturn strbuf_detach(&buf, NULL);\n }\n\n-int handle_curl_result(struct active_request_slot *slot)\n+int handle_curl_result(struct active_request_slot *slot, struct\nslot_results *results)\n {\n-\tstruct slot_results *results = slot->results;\n-\n \tif (results->curl_result == CURLE_OK) {\n \t\tcredential_approve(&http_auth);\n \t\treturn HTTP_OK;\n@@ -822,7 +820,7 @@ static int http_request(const char *url, void\n*result, int target, int options)\n\n \tif (start_active_slot(slot)) {\n \t\trun_active_slot(slot);\n-\t\tret = handle_curl_result(slot);\n+\t\tret = handle_curl_result(slot, &results);\n \t} else {\n \t\terror(\"Unable to start HTTP request for %s\", url);\n \t\tret = HTTP_START_FAILED;\ndiff --git a/http.h b/http.h\nindex 12de255..12c27fa 100644\n--- a/http.h\n+++ b/http.h\n@@ -78,7 +78,7 @@ extern int start_active_slot(struct\nactive_request_slot *slot);\n extern void run_active_slot(struct active_request_slot *slot);\n extern void finish_active_slot(struct active_request_slot *slot);\n extern void finish_all_active_slots(void);\n-extern int handle_curl_result(struct active_request_slot *slot);\n+extern int handle_curl_result(struct active_request_slot *slot,\nstruct slot_results *results);\n\n #ifdef USE_CURL_MULTI\n extern void fill_active_slots(void);\ndiff --git a/remote-curl.c b/remote-curl.c\nindex 10fa8f1..42716c5 100644\n--- a/remote-curl.c\n+++ b/remote-curl.c\n@@ -356,7 +356,7 @@ static int run_slot(struct active_request_slot *slot)\n \tslot->curl_result = curl_easy_perform(slot->curl);\n \tfinish_active_slot(slot);\n\n-\terr = handle_curl_result(slot);\n+\terr = handle_curl_result(slot, &results);\n \tif (err != HTTP_OK && err != HTTP_REAUTH) {\n \t\terror(\"RPC failed; result=%d, HTTP code = %ld\",\n \t\t      results.curl_result, results.http_code);\n"},{"id":"201063","messageId":"CABPQNSbRYq43rADP6BaQ_Q2SB+M76rHdX3hOSpGQk2c5UNR5hg@mail.gmail.com","threadId":"31796","inReplyTo":"CABPQNSYyh-gk8aD9SKLzo0MDBkGECthdGZiT1mB1pA0ajdB8Cw@mail.gmail.com","subject":"Re: git fails: segfault at 0 ip 00000000004076d5 sp 00007fff7806ebc0","fromName":"Erik Faye-Lund","fromEmail":"kusmabite@gmail.com","sentAt":"2012-10-12T16:34:30Z","receivedAt":"2012-10-12T16:34:30Z","isPatch":false,"sender":{"key":"kusmabite@gmail.com","avatar":"https://avatars.githubusercontent.com/u/47073?v=4"},"body":"On Fri, Oct 12, 2012 at 6:29 PM, Erik Faye-Lund <kusmabite@gmail.com> wrote:\n> On Fri, Oct 12, 2012 at 6:58 AM, Brad Hein <linuxbrad@gmail.com> wrote:\n>> In Fedora 17\n>> With git-1.7.11.7-1.fc17.x86_64 (rpm)\n>>\n>> I try to clone a particular repository but git just returns, having\n>> not cloned the repo. Seems like a bug. Details follow:\n>>   $ git clone http://gnuradio.org/git/gnuradio.git\n>>\n>> While the command fails a message is logged to syslog. Repeated\n>> attempts to clone the repo yield the same result:\n>>   Oct 11 21:38:25 localhost kernel: [662703.442645]\n>> git-remote-http[25796]: segfault at 0 ip 00000000004076d5 sp\n>> 00007fff7806ebc0 error 4 in git-remote-http[400000+96000]\n>>   Oct 11 21:39:00 localhost kernel: [662737.899829]\n>> git-remote-http[25837]: segfault at 0 ip 00000000004076d5 sp\n>> 00007fff37c5ef20 error 4 in git-remote-http[400000+96000]\n>>   Oct 11 21:39:25 localhost kernel: [662763.341248]\n>> git-remote-http[25873]: segfault at 0 ip 00000000004076d5 sp\n>> 00007fff6310d470 error 4 in git-remote-http[400000+96000]\n>>\n>> A tcpdump reveals that the last thing the client does is requests a\n>> file that doesn't exist on the server (404). Details are in my post on\n>> FedoraForums: http://forums.fedoraforum.org/showthread.php?p=1607891&posted=1#post1607891\n>>\n>> Problem mitigated by downgrade to \"git-1.7.10.1-1.fc17.x86_64\" or\n>> \"git-1.7.11.4-3.fc17.x86_64\" or try to clone a different repository.\n>\n> Thanks for reporting. I gave it a quick go, and the issue seems to\n> also be present in the current 'master'.\n>\n> The problem is a NULL-pointer dereferencing introduced in 8809703\n> (\"http: factor out http error code handling\"), where the code assume\n> that slot->results still points to http_request::results. This\n> assumption seems to be wrong.\n>\n> This seems to step around the issue, but I don't know if\n> http_request::results should be set to NULL in the first place. Jeff?\n\nOK, it seems I jumped the gun, and Jeff already sent out a patch for\nit. Nevermind me, then :)\n"},{"id":"201065","messageId":"7vmwzr4ozm.fsf@alter.siamese.dyndns.org","threadId":"31796","inReplyTo":"20121012062249.GB17026@sigill.intra.peff.net","subject":"Re: [PATCH] http: fix segfault in handle_curl_result","fromName":"Junio C Hamano","fromEmail":"gitster@pobox.com","sentAt":"2012-10-12T16:46:53Z","receivedAt":"2012-10-12T16:46:53Z","isPatch":true,"sender":{"key":"gitster@pobox.com","avatar":"https://avatars.githubusercontent.com/u/54884?v=4"},"body":"Jeff King <peff@peff.net> writes:\n\n> When we create an http active_request_slot, we can set its\n> \"results\" pointer back to local storage. The http code will\n> fill in the details of how the request went, and we can\n> access those details even after the slot has been cleaned\n> up.\n> ...\n> However, I'm leaving that out of this patch.  Commit 8809703 was\n> supposed to be a refactor with zero behavior changes, but it regressed.\n> This fixes the regression by behaving exactly as we did beforehand. We\n> can build the other thing on top.\n\nThanks for a good write-up.\n\nI agree with the fix, and I also agree that it does not make sense\nto pass slot to handle-curl-RESULT when we know the slot may not\nhave anything to do with the result we are dealing with.  If\nanything, we should be passing result (which this patch already\ndoes) and curl handle (the sole reason why slot is passed, after\nthis patch, becomes to let the function access it via slot->curl),\nbut as your analysis shows, with dfa1725 (post 1.7.10.2) it should\nnot be even needed to call init_curl_http_auth() here, which is the\n[2/2] of your follow-up.\n\nHappy I am.  Thanks.\n\n>  http.c        | 7 +++----\n>  http.h        | 3 ++-\n>  remote-curl.c | 2 +-\n>  3 files changed, 6 insertions(+), 6 deletions(-)\n>\n> diff --git a/http.c b/http.c\n> index 7c4a407..9334386 100644\n> --- a/http.c\n> +++ b/http.c\n> @@ -744,10 +744,9 @@ int handle_curl_result(struct active_request_slot *slot)\n>  \treturn strbuf_detach(&buf, NULL);\n>  }\n>  \n> -int handle_curl_result(struct active_request_slot *slot)\n> +int handle_curl_result(struct active_request_slot *slot,\n> +\t\t       struct slot_results *results)\n>  {\n> -\tstruct slot_results *results = slot->results;\n> -\n>  \tif (results->curl_result == CURLE_OK) {\n>  \t\tcredential_approve(&http_auth);\n>  \t\treturn HTTP_OK;\n> @@ -818,7 +817,7 @@ static int http_request(const char *url, void *result, int target, int options)\n>  \n>  \tif (start_active_slot(slot)) {\n>  \t\trun_active_slot(slot);\n> -\t\tret = handle_curl_result(slot);\n> +\t\tret = handle_curl_result(slot, &results);\n>  \t} else {\n>  \t\terror(\"Unable to start HTTP request for %s\", url);\n>  \t\tret = HTTP_START_FAILED;\n> diff --git a/http.h b/http.h\n> index 12de255..0bd1e84 100644\n> --- a/http.h\n> +++ b/http.h\n> @@ -78,7 +78,8 @@ extern void finish_all_active_slots(void);\n>  extern void run_active_slot(struct active_request_slot *slot);\n>  extern void finish_active_slot(struct active_request_slot *slot);\n>  extern void finish_all_active_slots(void);\n> -extern int handle_curl_result(struct active_request_slot *slot);\n> +extern int handle_curl_result(struct active_request_slot *slot,\n> +\t\t\t      struct slot_results *results);\n>  \n>  #ifdef USE_CURL_MULTI\n>  extern void fill_active_slots(void);\n> diff --git a/remote-curl.c b/remote-curl.c\n> index 3ec474f..6054e47 100644\n> --- a/remote-curl.c\n> +++ b/remote-curl.c\n> @@ -369,7 +369,7 @@ static int run_slot(struct active_request_slot *slot)\n>  \tslot->curl_result = curl_easy_perform(slot->curl);\n>  \tfinish_active_slot(slot);\n>  \n> -\terr = handle_curl_result(slot);\n> +\terr = handle_curl_result(slot, &results);\n>  \tif (err != HTTP_OK && err != HTTP_REAUTH) {\n>  \t\terror(\"RPC failed; result=%d, HTTP code = %ld\",\n>  \t\t      results.curl_result, results.http_code);\n"},{"id":"201068","messageId":"20121012171256.GA21409@sigill.intra.peff.net","threadId":"31796","inReplyTo":"CABPQNSbRYq43rADP6BaQ_Q2SB+M76rHdX3hOSpGQk2c5UNR5hg@mail.gmail.com","subject":"Re: git fails: segfault at 0 ip 00000000004076d5 sp 00007fff7806ebc0","fromName":"Jeff King","fromEmail":"peff@peff.net","sentAt":"2012-10-12T17:12:56Z","receivedAt":"2012-10-12T17:12:56Z","isPatch":false,"sender":{"key":"peff@peff.net","avatar":"https://avatars.githubusercontent.com/u/45925?v=4"},"body":"On Fri, Oct 12, 2012 at 06:34:30PM +0200, Erik Faye-Lund wrote:\n\n> > Thanks for reporting. I gave it a quick go, and the issue seems to\n> > also be present in the current 'master'.\n> >\n> > The problem is a NULL-pointer dereferencing introduced in 8809703\n> > (\"http: factor out http error code handling\"), where the code assume\n> > that slot->results still points to http_request::results. This\n> > assumption seems to be wrong.\n> >\n> > This seems to step around the issue, but I don't know if\n> > http_request::results should be set to NULL in the first place. Jeff?\n> \n> OK, it seems I jumped the gun, and Jeff already sent out a patch for\n> it. Nevermind me, then :)\n\nIndependent verification is never a bad thing. :)\n\n-Peff\n"}]}