{"thread":{"id":"52412","subject":"[Outreachy] [PATCH v2] bisect--helper: avoid free-after-use","startedAt":"2019-12-09T10:39:42Z","lastAt":"2019-12-12T19:16:55Z","messageCount":5,"participants":["Miriam Rubio","Junio C Hamano","Miriam R.","Johannes Schindelin"],"isPatch":true,"patchVersion":2,"patchTotal":null},"messages":[{"id":"387744","messageId":"20191209103923.21659-1-mirucam@gmail.com","threadId":"52412","inReplyTo":null,"subject":"[Outreachy] [PATCH v2] bisect--helper: avoid free-after-use","fromName":"Miriam Rubio","fromEmail":"mirucam@gmail.com","sentAt":"2019-12-09T10:39:23Z","receivedAt":"2019-12-09T10:39:42Z","isPatch":true,"sender":{"key":"mirucam@gmail.com","avatar":"https://avatars.githubusercontent.com/u/56339109?v=4"},"body":"From: Tanushree Tumane \n\nIn 5e82c3dd22a (bisect--helper: `bisect_reset` shell function in C,\n2019-01-02), the `git bisect reset` subcommand was ported to C. When the\ncall to `git checkout` failed, an error message was reported to the\nuser.\n\nHowever, this error message used the `strbuf` that had just been\nreleased already. Let's switch that around: first use it, then release\nit.\n\nMentored-by: Johannes Schindelin \nMentored-by: Christian Couder \nSigned-off-by: Tanushree Tumane \nSigned-off-by: Miriam Rubio \n---\nThis patch is a new version of\nhttps://public-inbox.org/git/20191208172813.16518-1-mirucam@gmail.com/\nwhich itself has been sent previously by Tanushree\n(https://public-inbox.org/git/64117cde718f0d56ebfa4c30f4d8fe2155f5cf65.1551003074.git.gitgitgadget@gmail.com/).\n\n builtin/bisect--helper.c | 7 ++++---\n 1 file changed, 4 insertions(+), 3 deletions(-)\n\ndiff --git a/builtin/bisect--helper.c b/builtin/bisect--helper.c\nindex 1fbe156e67..3055b2bb50 100644\n--- a/builtin/bisect--helper.c\n+++ b/builtin/bisect--helper.c\n@@ -169,11 +169,12 @@ static int bisect_reset(const char *commit)\n \n \t\targv_array_pushl(&argv, \"checkout\", branch.buf, \"--\", NULL);\n \t\tif (run_command_v_opt(argv.argv, RUN_GIT_CMD)) {\n+\t\t\terror(_(\"could not check out original\"\n+\t\t\t\t\" HEAD '%s'. Try 'git bisect\"\n+\t\t\t\t\" reset '.\"), branch.buf);\n \t\t\tstrbuf_release(&branch);\n \t\t\targv_array_clear(&argv);\n-\t\t\treturn error(_(\"could not check out original\"\n-\t\t\t\t \" HEAD '%s'. Try 'git bisect\"\n-\t\t\t\t \" reset '.\"), branch.buf);\n+\t\t\treturn -1;\n \t\t}\n \t\targv_array_clear(&argv);\n \t}\n-- \n2.21.0 (Apple Git-122.2)\n\n"},{"id":"387954","messageId":"xmqqo8wepyse.fsf@gitster-ct.c.googlers.com","threadId":"52412","inReplyTo":"20191209103923.21659-1-mirucam@gmail.com","subject":"Re: [Outreachy] [PATCH v2] bisect--helper: avoid free-after-use","fromName":"Junio C Hamano","fromEmail":"gitster@pobox.com","sentAt":"2019-12-11T17:17:21Z","receivedAt":"2019-12-11T17:17:29Z","isPatch":true,"sender":{"key":"gitster@pobox.com","avatar":"https://avatars.githubusercontent.com/u/54884?v=4"},"body":"Miriam Rubio writes:\n\n> From: Tanushree Tumane \n>\n> In 5e82c3dd22a (bisect--helper: `bisect_reset` shell function in C,\n> 2019-01-02), the `git bisect reset` subcommand was ported to C. When the\n> call to `git checkout` failed, an error message was reported to the\n> user.\n>\n> However, this error message used the `strbuf` that had just been\n> released already. Let's switch that around: first use it, then release\n> it.\n>\n> Mentored-by: Johannes Schindelin \n> Mentored-by: Christian Couder \n> Signed-off-by: Tanushree Tumane \n> Signed-off-by: Miriam Rubio \n> ---\n> This patch is a new version of\n> https://public-inbox.org/git/20191208172813.16518-1-mirucam@gmail.com/\n> which itself has been sent previously by Tanushree\n> (https://public-inbox.org/git/64117cde718f0d56ebfa4c30f4d8fe2155f5cf65.1551003074.git.gitgitgadget@gmail.com/).\n>\n> builtin/bisect--helper.c | 7 ++++---\n> 1 file changed, 4 insertions(+), 3 deletions(-)\n>\n> diff --git a/builtin/bisect--helper.c b/builtin/bisect--helper.c\n> index 1fbe156e67..3055b2bb50 100644\n> --- a/builtin/bisect--helper.c\n> +++ b/builtin/bisect--helper.c\n> @@ -169,11 +169,12 @@ static int bisect_reset(const char *commit)\n> \n> \t\targv_array_pushl(&argv, \"checkout\", branch.buf, \"--\", NULL);\n> \t\tif (run_command_v_opt(argv.argv, RUN_GIT_CMD)) {\n> +\t\t\terror(_(\"could not check out original\"\n> +\t\t\t\t\" HEAD '%s'. Try 'git bisect\"\n> +\t\t\t\t\" reset '.\"), branch.buf);\n> \t\t\tstrbuf_release(&branch);\n> \t\t\targv_array_clear(&argv);\n> -\t\t\treturn error(_(\"could not check out original\"\n> -\t\t\t\t \" HEAD '%s'. Try 'git bisect\"\n> -\t\t\t\t \" reset '.\"), branch.buf);\n\nThe original obviously was bad X-<. Will queue. Thanks.\n\n> +\t\t\treturn -1;\n> \t\t}\n> \t\targv_array_clear(&argv);\n> \t}\n"},{"id":"387955","messageId":"xmqqh826pyh4.fsf@gitster-ct.c.googlers.com","threadId":"52412","inReplyTo":"xmqqo8wepyse.fsf@gitster-ct.c.googlers.com","subject":"Re: [Outreachy] [PATCH v2] bisect--helper: avoid free-after-use","fromName":"Junio C Hamano","fromEmail":"gitster@pobox.com","sentAt":"2019-12-11T17:24:07Z","receivedAt":"2019-12-11T17:24:13Z","isPatch":true,"sender":{"key":"gitster@pobox.com","avatar":"https://avatars.githubusercontent.com/u/54884?v=4"},"body":"Junio C Hamano writes:\n\n> Subject: Re: [Outreachy] [PATCH v2] bisect--helper: avoid free-after-use\n\nIt is surprising with multiple mentors, nobody noticed free-after-use\nis perfectly fine---it is use-after-free we would want to avoid.\n\n> Miriam Rubio writes:\n>\n>> From: Tanushree Tumane \n>>\n>> In 5e82c3dd22a (bisect--helper: `bisect_reset` shell function in C,\n>> 2019-01-02), the `git bisect reset` subcommand was ported to C. When the\n>> call to `git checkout` failed, an error message was reported to the\n>> user.\n>>\n>> However, this error message used the `strbuf` that had just been\n>> released already. Let's switch that around: first use it, then release\n>> it.\n>>\n>> Mentored-by: Johannes Schindelin \n>> Mentored-by: Christian Couder \n>> Signed-off-by: Tanushree Tumane \n>> Signed-off-by: Miriam Rubio \n>> ---\n>> This patch is a new version of\n>> https://public-inbox.org/git/20191208172813.16518-1-mirucam@gmail.com/\n>> which itself has been sent previously by Tanushree\n>> (https://public-inbox.org/git/64117cde718f0d56ebfa4c30f4d8fe2155f5cf65.1551003074.git.gitgitgadget@gmail.com/).\n>>\n>> builtin/bisect--helper.c | 7 ++++---\n>> 1 file changed, 4 insertions(+), 3 deletions(-)\n>>\n>> diff --git a/builtin/bisect--helper.c b/builtin/bisect--helper.c\n>> index 1fbe156e67..3055b2bb50 100644\n>> --- a/builtin/bisect--helper.c\n>> +++ b/builtin/bisect--helper.c\n>> @@ -169,11 +169,12 @@ static int bisect_reset(const char *commit)\n>> \n>> \t\targv_array_pushl(&argv, \"checkout\", branch.buf, \"--\", NULL);\n>> \t\tif (run_command_v_opt(argv.argv, RUN_GIT_CMD)) {\n>> +\t\t\terror(_(\"could not check out original\"\n>> +\t\t\t\t\" HEAD '%s'. Try 'git bisect\"\n>> +\t\t\t\t\" reset '.\"), branch.buf);\n>> \t\t\tstrbuf_release(&branch);\n>> \t\t\targv_array_clear(&argv);\n>> -\t\t\treturn error(_(\"could not check out original\"\n>> -\t\t\t\t \" HEAD '%s'. Try 'git bisect\"\n>> -\t\t\t\t \" reset '.\"), branch.buf);\n>\n> The original obviously was bad X-<. Will queue. Thanks.\n>\n>> +\t\t\treturn -1;\n>> \t\t}\n>> \t\targv_array_clear(&argv);\n>> \t}\n"},{"id":"387968","messageId":"CAN7CjDAYo=AZkVg4X_8iLLWTo43h=BBoOY+Ez-ODL0L_eb8xcg@mail.gmail.com","threadId":"52412","inReplyTo":"xmqqh826pyh4.fsf@gitster-ct.c.googlers.com","subject":"Re: [Outreachy] [PATCH v2] bisect--helper: avoid free-after-use","fromName":"Miriam R.","fromEmail":"mirucam@gmail.com","sentAt":"2019-12-11T19:14:03Z","receivedAt":"2019-12-11T19:14:16Z","isPatch":true,"sender":{"key":"mirucam@gmail.com","avatar":"https://avatars.githubusercontent.com/u/56339109?v=4"},"body":"El mié., 11 dic. 2019 a las 18:24, Junio C Hamano\n() escribió:\n>\n> Junio C Hamano writes:\n>\n> > Subject: Re: [Outreachy] [PATCH v2] bisect--helper: avoid free-after-use\n>\n> It is surprising with multiple mentors, nobody noticed free-after-use\n> is perfectly fine---it is use-after-free we would want to avoid.\n>\n\nYes, you are right. I will send another version with the correct title.\n\nThank you,\nMiriam\n\n> > Miriam Rubio writes:\n> >\n> >> From: Tanushree Tumane \n> >>\n> >> In 5e82c3dd22a (bisect--helper: `bisect_reset` shell function in C,\n> >> 2019-01-02), the `git bisect reset` subcommand was ported to C. When the\n> >> call to `git checkout` failed, an error message was reported to the\n> >> user.\n> >>\n> >> However, this error message used the `strbuf` that had just been\n> >> released already. Let's switch that around: first use it, then release\n> >> it.\n> >>\n> >> Mentored-by: Johannes Schindelin \n> >> Mentored-by: Christian Couder \n> >> Signed-off-by: Tanushree Tumane \n> >> Signed-off-by: Miriam Rubio \n> >> ---\n> >> This patch is a new version of\n> >> https://public-inbox.org/git/20191208172813.16518-1-mirucam@gmail.com/\n> >> which itself has been sent previously by Tanushree\n> >> (https://public-inbox.org/git/64117cde718f0d56ebfa4c30f4d8fe2155f5cf65.1551003074.git.gitgitgadget@gmail.com/).\n> >>\n> >> builtin/bisect--helper.c | 7 ++++---\n> >> 1 file changed, 4 insertions(+), 3 deletions(-)\n> >>\n> >> diff --git a/builtin/bisect--helper.c b/builtin/bisect--helper.c\n> >> index 1fbe156e67..3055b2bb50 100644\n> >> --- a/builtin/bisect--helper.c\n> >> +++ b/builtin/bisect--helper.c\n> >> @@ -169,11 +169,12 @@ static int bisect_reset(const char *commit)\n> >>\n> >> argv_array_pushl(&argv, \"checkout\", branch.buf, \"--\", NULL);\n> >> if (run_command_v_opt(argv.argv, RUN_GIT_CMD)) {\n> >> + error(_(\"could not check out original\"\n> >> + \" HEAD '%s'. Try 'git bisect\"\n> >> + \" reset '.\"), branch.buf);\n> >> strbuf_release(&branch);\n> >> argv_array_clear(&argv);\n> >> - return error(_(\"could not check out original\"\n> >> - \" HEAD '%s'. Try 'git bisect\"\n> >> - \" reset '.\"), branch.buf);\n> >\n> > The original obviously was bad X-<. Will queue. Thanks.\n> >\n> >> + return -1;\n> >> }\n> >> argv_array_clear(&argv);\n> >> }\n"},{"id":"388038","messageId":"nycvar.QRO.7.76.6.1912122015200.46@tvgsbejvaqbjf.bet","threadId":"52412","inReplyTo":"xmqqh826pyh4.fsf@gitster-ct.c.googlers.com","subject":"Re: [Outreachy] [PATCH v2] bisect--helper: avoid free-after-use","fromName":"Johannes Schindelin","fromEmail":"johannes.schindelin@gmx.de","sentAt":"2019-12-12T19:16:17Z","receivedAt":"2019-12-12T19:16:55Z","isPatch":true,"sender":{"key":"johannes.schindelin@gmx.de","avatar":"https://avatars.githubusercontent.com/u/127790?v=4"},"body":"Hi Junio,\n\nOn Wed, 11 Dec 2019, Junio C Hamano wrote:\n\n> Junio C Hamano writes:\n>\n> > Subject: Re: [Outreachy] [PATCH v2] bisect--helper: avoid free-after-use\n>\n> It is surprising with multiple mentors, nobody noticed free-after-use\n> is perfectly fine---it is use-after-free we would want to avoid.\n\nWow. It is totally my fault, and the only thing I can blame is the\nmind-numbing work I did on those security fixes. So glad that's over.\n\nSorry for the mistake,\nDscho\n"}]}