{"thread":{"id":"64895","subject":"[PATCH] fix(clone): segment fault when using --revision and protocol v0/v1","startedAt":"2026-02-01T09:23:42Z","lastAt":"2026-02-08T15:25:56Z","messageCount":4,"participants":["NitroCao via GitGitGadget","Junio C Hamano","Nitro Cao"],"isPatch":true,"patchVersion":1,"patchTotal":null},"messages":[{"id":"534941","messageId":"pull.2185.git.git.1769937818682.gitgitgadget@gmail.com","threadId":"64895","inReplyTo":null,"subject":"[PATCH] fix(clone): segment fault when using --revision and protocol v0/v1","fromName":"NitroCao via GitGitGadget","fromEmail":"gitgitgadget@gmail.com","sentAt":"2026-02-01T09:23:38Z","receivedAt":"2026-02-01T09:23:42Z","isPatch":true,"sender":{"key":"name:NitroCao","avatar":null},"body":"From: Nitro Cao <jaycecao520@gmail.com>\n\ngit clone command would segment fault when satisfying the following\nconditions at the same time:\n  - Use HTTP protocol v0 or v1 to interact with remote servers.\n  - The value of `--revision` doesn't specify the peer reference, like\n    `--revision master` instead of `--revision refs/heads/master:master`\n\nWhen using protocol v2, git client can use `ref-prefix` param of\n`ls-refs` command to fetch wanted references based on `--revision`.\nBut for protocol v0/v1, git client just fetch all references and\ndoesn't filter them.\nIn this case, the value of `remote_head` variable is not NULL,\nwhich leads to the value of `remote_head_points_at` not NULL too.\nBut we don't specify the peer reference in `--revsion`,\n`remote_head_points_at->peer_ref` would be NULL. So git client would\nboom when `update_remote_refs`.\n\nSigned-off-by: Nitro Cao <jaycecao520@gmail.com>\n---\n    fix(clone): segment fault when using --revision and protocol v0/v1\n    \n    git clone command would segment fault when satisfying the following\n    conditions at the same time:\n    \n     * Use HTTP protocol v0 or v1 to interact with remote servers.\n     * The value of --revision doesn't specify the peer reference, like\n       --revision master instead of --revision refs/heads/master:master\n    \n    When using protocol v2, git client can use ref-prefix param of ls-refs\n    command to fetch wanted references based on --revision. But for protocol\n    v0/v1, git client just fetch all references and doesn't filter them. In\n    this case, the value of remote_head variable is not NULL, which leads to\n    the value of remote_head_points_at not NULL too. But we don't specify\n    the peer reference in --revsion, remote_head_points_at->peer_ref would\n    be NULL. So git client would boom when update_remote_refs.\n\nPublished-As: https://github.com/gitgitgadget/git/releases/tag/pr-git-2185%2FNitroCao%2Ffix%2Fsegment-fault-with-revision-param-v1\nFetch-It-Via: git fetch https://github.com/gitgitgadget/git pr-git-2185/NitroCao/fix/segment-fault-with-revision-param-v1\nPull-Request: https://github.com/git/git/pull/2185\n\n builtin/clone.c | 2 +-\n 1 file changed, 1 insertion(+), 1 deletion(-)\n\ndiff --git a/builtin/clone.c b/builtin/clone.c\nindex b40cee5968..ba8de92563 100644\n--- a/builtin/clone.c\n+++ b/builtin/clone.c\n@@ -558,7 +558,7 @@ static void update_remote_refs(const struct ref *refs,\n \t\t\twrite_followtags(refs, msg);\n \t}\n \n-\tif (remote_head_points_at && !option_bare) {\n+\tif (remote_head_points_at && remote_head_points_at->peer_ref && !option_bare) {\n \t\tstruct strbuf head_ref = STRBUF_INIT;\n \t\tstrbuf_addstr(&head_ref, branch_top);\n \t\tstrbuf_addstr(&head_ref, \"HEAD\");\n\nbase-commit: 22584464849815268419fd9d2eba307362360db1\n-- \ngitgitgadget\n"},{"id":"535060","messageId":"pull.2185.v2.git.git.1770119773541.gitgitgadget@gmail.com","threadId":"64895","inReplyTo":"pull.2185.git.git.1769937818682.gitgitgadget@gmail.com","subject":"[PATCH v2] clone: fix segfault when using --revision and v0/v1 protocol","fromName":"NitroCao via GitGitGadget","fromEmail":"gitgitgadget@gmail.com","sentAt":"2026-02-03T11:56:13Z","receivedAt":"2026-02-03T11:56:16Z","isPatch":true,"sender":{"key":"name:NitroCao","avatar":null},"body":"From: Nitro Cao <jaycecao520@gmail.com>\n\nWhen `git clone` is used with `--revision` and the protocol version is\nv0 or v1, the client segfaults if the revision does not specify a peer\nreference (e.g. `--revision master` instead of\n`--revision refs/heads/master:master`).\n\nThis occurs because `update_remote_refs()` assumes that if\n`remote_head_points_at` is set, `remote_head_points_at->peer_ref` is\nalso valid. However, for v0/v1 protocols, all references are fetched\nwithout filtering, and if the revision lacks a peer reference,\n`peer_ref` remains NULL.\n\nAdd a check for `remote_head_points_at->peer_ref` before dereferencing\nit to prevent the segmentation fault.\n\nSigned-off-by: Nitro Cao <jaycecao520@gmail.com>\n---\n    fix(clone): segment fault when using --revision and protocol v0/v1\n    \n    git clone command would segment fault when satisfying the following\n    conditions at the same time:\n    \n     * Use HTTP protocol v0 or v1 to interact with remote servers.\n     * The value of --revision doesn't specify the peer reference, like\n       --revision master instead of --revision refs/heads/master:master\n    \n    When using protocol v2, git client can use ref-prefix param of ls-refs\n    command to fetch wanted references based on --revision. But for protocol\n    v0/v1, git client just fetch all references and doesn't filter them. In\n    this case, the value of remote_head variable is not NULL, which leads to\n    the value of remote_head_points_at not NULL too. But we don't specify\n    the peer reference in --revsion, remote_head_points_at->peer_ref would\n    be NULL. So git client would boom when update_remote_refs.\n\nPublished-As: https://github.com/gitgitgadget/git/releases/tag/pr-git-2185%2FNitroCao%2Ffix%2Fsegment-fault-with-revision-param-v2\nFetch-It-Via: git fetch https://github.com/gitgitgadget/git pr-git-2185/NitroCao/fix/segment-fault-with-revision-param-v2\nPull-Request: https://github.com/git/git/pull/2185\n\nRange-diff vs v1:\n\n 1:  f75b1f7e2e ! 1:  b3ab27f977 fix(clone): segment fault when using --revision and protocol v0/v1\n     @@ Metadata\n      Author: Nitro Cao <jaycecao520@gmail.com>\n      \n       ## Commit message ##\n     -    fix(clone): segment fault when using --revision and protocol v0/v1\n     +    clone: fix segfault when using --revision and v0/v1 protocol\n      \n     -    git clone command would segment fault when satisfying the following\n     -    conditions at the same time:\n     -      - Use HTTP protocol v0 or v1 to interact with remote servers.\n     -      - The value of `--revision` doesn't specify the peer reference, like\n     -        `--revision master` instead of `--revision refs/heads/master:master`\n     +    When `git clone` is used with `--revision` and the protocol version is\n     +    v0 or v1, the client segfaults if the revision does not specify a peer\n     +    reference (e.g. `--revision master` instead of\n     +    `--revision refs/heads/master:master`).\n      \n     -    When using protocol v2, git client can use `ref-prefix` param of\n     -    `ls-refs` command to fetch wanted references based on `--revision`.\n     -    But for protocol v0/v1, git client just fetch all references and\n     -    doesn't filter them.\n     -    In this case, the value of `remote_head` variable is not NULL,\n     -    which leads to the value of `remote_head_points_at` not NULL too.\n     -    But we don't specify the peer reference in `--revsion`,\n     -    `remote_head_points_at->peer_ref` would be NULL. So git client would\n     -    boom when `update_remote_refs`.\n     +    This occurs because `update_remote_refs()` assumes that if\n     +    `remote_head_points_at` is set, `remote_head_points_at->peer_ref` is\n     +    also valid. However, for v0/v1 protocols, all references are fetched\n     +    without filtering, and if the revision lacks a peer reference,\n     +    `peer_ref` remains NULL.\n     +\n     +    Add a check for `remote_head_points_at->peer_ref` before dereferencing\n     +    it to prevent the segmentation fault.\n      \n          Signed-off-by: Nitro Cao <jaycecao520@gmail.com>\n      \n\n\n builtin/clone.c | 2 +-\n 1 file changed, 1 insertion(+), 1 deletion(-)\n\ndiff --git a/builtin/clone.c b/builtin/clone.c\nindex b40cee5968..ba8de92563 100644\n--- a/builtin/clone.c\n+++ b/builtin/clone.c\n@@ -558,7 +558,7 @@ static void update_remote_refs(const struct ref *refs,\n \t\t\twrite_followtags(refs, msg);\n \t}\n \n-\tif (remote_head_points_at && !option_bare) {\n+\tif (remote_head_points_at && remote_head_points_at->peer_ref && !option_bare) {\n \t\tstruct strbuf head_ref = STRBUF_INIT;\n \t\tstrbuf_addstr(&head_ref, branch_top);\n \t\tstrbuf_addstr(&head_ref, \"HEAD\");\n\nbase-commit: 67ad42147a7acc2af6074753ebd03d904476118f\n-- \ngitgitgadget\n"},{"id":"535088","messageId":"xmqqfr7hsk63.fsf@gitster.g","threadId":"64895","inReplyTo":"pull.2185.v2.git.git.1770119773541.gitgitgadget@gmail.com","subject":"Re: [PATCH v2] clone: fix segfault when using --revision and v0/v1 protocol","fromName":"Junio C Hamano","fromEmail":"gitster@pobox.com","sentAt":"2026-02-03T19:26:28Z","receivedAt":"2026-02-03T19:26:30Z","isPatch":true,"sender":{"key":"gitster@pobox.com","avatar":"https://avatars.githubusercontent.com/u/54884?v=4"},"body":"\"NitroCao via GitGitGadget\" <gitgitgadget@gmail.com> writes:\n\n> From: Nitro Cao <jaycecao520@gmail.com>\n>\n> When `git clone` is used with `--revision` and the protocol version is\n> v0 or v1, the client segfaults if the revision does not specify a peer\n> reference (e.g. `--revision master` instead of\n> `--revision refs/heads/master:master`).\n>\n> This occurs because `update_remote_refs()` assumes that if\n> `remote_head_points_at` is set, `remote_head_points_at->peer_ref` is\n> also valid. However, for v0/v1 protocols, all references are fetched\n> without filtering, and if the revision lacks a peer reference,\n> `peer_ref` remains NULL.\n>\n> Add a check for `remote_head_points_at->peer_ref` before dereferencing\n> it to prevent the segmentation fault.\n\nHmph.\n\nWhile your change may skip the code that segfaults, wouldn't it also\nstop noticing a broken case where .peer_ref should have been set but\ndidn't, even when --revision=<rev> parameter is not used in the\ncommand invocation?  IOW, it is better to segfault and draw attention\nby Git developers when a valid input is given by the end user and our\ncode misbehaves (e.g., and fails to to set .peer_ref as it should).\n\nStepping back a bit, \"git clone --help\" says the following on\n\"--revision=<rev>\":\n\n    `--revision=<rev>`::\n            Create a new repository, and fetch the history leading\n            to the given revision _<rev>_ (and nothing else),\n            without making any remote-tracking branch, and without\n            making any local branch, and detach `HEAD` to\n            _<rev>_. The argument can be a ref name\n            (e.g. `refs/heads/main` or `refs/tags/v1.0`) that peels\n            down to a commit, or a hexadecimal object name.  This\n            option is incompatible with `--branch` and `--mirror`.\n\nThe intent of running the command with this option seems to me that\nwe do not want to create any branches, neither remote-tracking nor\nlocal.  Looking at what is done in update_remote_refs(), I think we\nstill want to honor check_connectivity even when we are in this\n\"single revision only, detach the HEAD at that commit\" mode in order\nto ensure the integrity of the data, but we cetainly do not want to\ncall the write_remote_refs() and the write_followtags() helpers.\n\nWouldn't the correct fix be more like the following?\n\n - split out parts from update_remote_refs() that are needed even in\n   option_rev mode into a separate helper function, and call that\n   from cmd_clone().\n\n - make the call to update_remote_refs() conditional---specifically,\n   we shouldn't be calling it when option_rev is in effect.\n\n>  builtin/clone.c | 2 +-\n>  1 file changed, 1 insertion(+), 1 deletion(-)\n\nAlso, isn't this something we can specify the expected behaviour in\ntests?  Not only we want to ensure that nothing segfaults, we would\nwant to make sure that the resulting repository has no refs and HEAD\nis detached at the specified revision.\n\nThanks.\n\n> diff --git a/builtin/clone.c b/builtin/clone.c\n> index b40cee5968..ba8de92563 100644\n> --- a/builtin/clone.c\n> +++ b/builtin/clone.c\n> @@ -558,7 +558,7 @@ static void update_remote_refs(const struct ref *refs,\n>  \t\t\twrite_followtags(refs, msg);\n>  \t}\n>  \n> -\tif (remote_head_points_at && !option_bare) {\n> +\tif (remote_head_points_at && remote_head_points_at->peer_ref && !option_bare) {\n>  \t\tstruct strbuf head_ref = STRBUF_INIT;\n>  \t\tstrbuf_addstr(&head_ref, branch_top);\n>  \t\tstrbuf_addstr(&head_ref, \"HEAD\");\n>\n> base-commit: 67ad42147a7acc2af6074753ebd03d904476118f\n"},{"id":"535478","messageId":"20260208152551.93473-1-jaycecao520@gmail.com","threadId":"64895","inReplyTo":"xmqqfr7hsk63.fsf@gitster.g","subject":"","fromName":"Nitro Cao","fromEmail":"jaycecao520@gmail.com","sentAt":"2026-02-08T15:25:51Z","receivedAt":"2026-02-08T15:25:56Z","isPatch":false,"sender":{"key":"jaycecao520@gmail.com","avatar":null},"body":"Hi Junio and thanks for your time! I'm a newbie, sorry for the last\nreply :(\n\n> While your change may skip the code that segfaults, wouldn't it also\n> stop noticing a broken case where .peer_ref should have been set but\n> didn't, even when --revision=<rev> parameter is not used in the\n> command invocation?  IOW, it is better to segfault and draw attention\n> by Git developers when a valid input is given by the end user and our\n> code misbehaves (e.g., and fails to to set .peer_ref as it should).\n\nI totally agree with you.\n\n\n> Wouldn't the correct fix be more like the following?\n>\n> - split out parts from update_remote_refs() that are needed even in\n>   option_rev mode into a separate helper function, and call that\n>   from cmd_clone().\n>\n> - make the call to update_remote_refs() conditional---specifically,\n>   we shouldn't be calling it when option_rev is in effect.\n\nDo we have another fix to make a conditional call to `find_ref_by_name()`\nwhen `option_rev` is in effect? Because from the doc of `--revision`,\n\"... and detach `HEAD` to_<rev>_. ...\", that means we don't need to\nknow where the real HEAD points to?\n\n> Also, isn't this something we can specify the expected behaviour in\n> tests?  Not only we want to ensure that nothing segfaults, we would\n> want to make sure that the resulting repository has no refs and HEAD\n> is detached at the specified revision.\n\nI'll add several tests to cover the bug after we make sure how to fix it.\n"}]}