{"thread":{"id":"47570","subject":"[PATCH] bisect: avoid NULL pointer dereference","startedAt":"2018-01-08T20:37:07Z","lastAt":"2018-01-08T22:15:53Z","messageCount":4,"participants":["René Scharfe","Johannes Schindelin","Junio C Hamano"],"isPatch":true,"patchVersion":1,"patchTotal":null},"messages":[{"id":"336191","messageId":"07a8cdae-8f8c-8560-c450-c6089a942c96@web.de","threadId":"47570","inReplyTo":null,"subject":"[PATCH] bisect: avoid NULL pointer dereference","fromName":"René Scharfe","fromEmail":"l.s.r@web.de","sentAt":"2018-01-08T20:36:55Z","receivedAt":"2018-01-08T20:37:07Z","isPatch":true,"sender":{"key":"l.s.r@web.de","avatar":"https://avatars.githubusercontent.com/u/26122331?v=4"},"body":"7c117184d7 (bisect: fix off-by-one error in `best_bisection_sorted()`)\nfixed an off-by-one error, plugged a memory leak and removed a NULL\ncheck. However, the pointer p *is* actually NULL if an empty list is\npassed to the function. Let's add the check back for safety. Bisecting\nnothing doesn't make too much sense, but that's no excuse for crashing.\n\nFound with GCC's -Wnull-dereference.\n\nSigned-off-by: Rene Scharfe \n---\n bisect.c | 6 ++++--\n 1 file changed, 4 insertions(+), 2 deletions(-)\n\ndiff --git a/bisect.c b/bisect.c\nindex 0fca17c02b..2f3008b078 100644\n--- a/bisect.c\n+++ b/bisect.c\n@@ -229,8 +229,10 @@ static struct commit_list *best_bisection_sorted(struct commit_list *list, int n\n \t\tif (i < cnt - 1)\n \t\t\tp = p->next;\n \t}\n-\tfree_commit_list(p->next);\n-\tp->next = NULL;\n+\tif (p) {\n+\t\tfree_commit_list(p->next);\n+\t\tp->next = NULL;\n+\t}\n \tstrbuf_release(&buf);\n \tfree(array);\n \treturn list;\n-- \n2.15.1\n"},{"id":"336195","messageId":"nycvar.QRO.7.76.6.1801082145140.31@ZVAVAG-6OXH6DA.rhebcr.pbec.zvpebfbsg.pbz","threadId":"47570","inReplyTo":"07a8cdae-8f8c-8560-c450-c6089a942c96@web.de","subject":"Re: [PATCH] bisect: avoid NULL pointer dereference","fromName":"Johannes Schindelin","fromEmail":"johannes.schindelin@gmx.de","sentAt":"2018-01-08T20:45:37Z","receivedAt":"2018-01-08T20:45:48Z","isPatch":true,"sender":{"key":"johannes.schindelin@gmx.de","avatar":"https://avatars.githubusercontent.com/u/127790?v=4"},"body":"Hi René,\n\nOn Mon, 8 Jan 2018, René Scharfe wrote:\n\n> 7c117184d7 (bisect: fix off-by-one error in `best_bisection_sorted()`)\n> fixed an off-by-one error, plugged a memory leak and removed a NULL\n> check. However, the pointer p *is* actually NULL if an empty list is\n> passed to the function. Let's add the check back for safety. Bisecting\n> nothing doesn't make too much sense, but that's no excuse for crashing.\n> \n> Found with GCC's -Wnull-dereference.\n> \n> Signed-off-by: Rene Scharfe \n> ---\n> bisect.c | 6 ++++--\n> 1 file changed, 4 insertions(+), 2 deletions(-)\n> \n> diff --git a/bisect.c b/bisect.c\n> index 0fca17c02b..2f3008b078 100644\n> --- a/bisect.c\n> +++ b/bisect.c\n> @@ -229,8 +229,10 @@ static struct commit_list *best_bisection_sorted(struct commit_list *list, int n\n> \t\tif (i < cnt - 1)\n> \t\t\tp = p->next;\n> \t}\n> -\tfree_commit_list(p->next);\n> -\tp->next = NULL;\n> +\tif (p) {\n> +\t\tfree_commit_list(p->next);\n> +\t\tp->next = NULL;\n> +\t}\n> \tstrbuf_release(&buf);\n> \tfree(array);\n> \treturn list;\n\nIsn't this identical to\nhttps://public-inbox.org/git/20180103184852.27271-1-avarab@gmail.com/ ?\n\nCiao,\nDscho"},{"id":"336203","messageId":"4806374d-144e-5260-b4bf-34c699a980a3@web.de","threadId":"47570","inReplyTo":"nycvar.QRO.7.76.6.1801082145140.31@ZVAVAG-6OXH6DA.rhebcr.pbec.zvpebfbsg.pbz","subject":"Re: [PATCH] bisect: avoid NULL pointer dereference","fromName":"René Scharfe","fromEmail":"l.s.r@web.de","sentAt":"2018-01-08T21:50:25Z","receivedAt":"2018-01-08T21:50:39Z","isPatch":true,"sender":{"key":"l.s.r@web.de","avatar":"https://avatars.githubusercontent.com/u/26122331?v=4"},"body":"Hello Dscho,\n\nAm 08.01.2018 um 21:45 schrieb Johannes Schindelin:\n> Isn't this identical to\n> https://public-inbox.org/git/20180103184852.27271-1-avarab@gmail.com/ ?\n\nyes, indeed, thanks. So here's an upvote for Ævar's patch then. :)\n\n(I should have sent it earlier, but was not fully convinced it could be\ntriggered in the wild.)\n\nRené\n"},{"id":"336209","messageId":"xmqqr2r06lpp.fsf@gitster.mtv.corp.google.com","threadId":"47570","inReplyTo":"07a8cdae-8f8c-8560-c450-c6089a942c96@web.de","subject":"Re: [PATCH] bisect: avoid NULL pointer dereference","fromName":"Junio C Hamano","fromEmail":"gitster@pobox.com","sentAt":"2018-01-08T22:15:46Z","receivedAt":"2018-01-08T22:15:53Z","isPatch":true,"sender":{"key":"gitster@pobox.com","avatar":"https://avatars.githubusercontent.com/u/54884?v=4"},"body":"René Scharfe writes:\n\n> 7c117184d7 (bisect: fix off-by-one error in `best_bisection_sorted()`)\n> fixed an off-by-one error, plugged a memory leak and removed a NULL\n> check. However, the pointer p *is* actually NULL if an empty list is\n> passed to the function. Let's add the check back for safety. Bisecting\n> nothing doesn't make too much sense, but that's no excuse for crashing.\n>\n> Found with GCC's -Wnull-dereference.\n>\n> Signed-off-by: Rene Scharfe \n> ---\n\nThanks. I think this is the same as 2e9fdc79 (\"bisect: fix a\nregression causing a segfault\", 2018-01-03) but the log we see here\nexplains what goes wrong much better than the other one ;-)\n\n> bisect.c | 6 ++++--\n> 1 file changed, 4 insertions(+), 2 deletions(-)\n>\n> diff --git a/bisect.c b/bisect.c\n> index 0fca17c02b..2f3008b078 100644\n> --- a/bisect.c\n> +++ b/bisect.c\n> @@ -229,8 +229,10 @@ static struct commit_list *best_bisection_sorted(struct commit_list *list, int n\n> \t\tif (i < cnt - 1)\n> \t\t\tp = p->next;\n> \t}\n> -\tfree_commit_list(p->next);\n> -\tp->next = NULL;\n> +\tif (p) {\n> +\t\tfree_commit_list(p->next);\n> +\t\tp->next = NULL;\n> +\t}\n> \tstrbuf_release(&buf);\n> \tfree(array);\n> \treturn list;\n"}]}