# [PATCH] bisect: avoid NULL pointer dereference 4 messages from 2018-01-08 to 2018-01-08. Participants: René Scharfe, Johannes Schindelin, Junio C Hamano. Thread: https://gitlist.dev/t/47570 ## René Scharfe, 2018-01-08 20:36 Subject: [PATCH] bisect: avoid NULL pointer dereference Message-ID: <07a8cdae-8f8c-8560-c450-c6089a942c96@web.de> URL: https://gitlist.dev/e/07a8cdae-8f8c-8560-c450-c6089a942c96%40web.de ``` 7c117184d7 (bisect: fix off-by-one error in `best_bisection_sorted()`) fixed an off-by-one error, plugged a memory leak and removed a NULL check. However, the pointer p *is* actually NULL if an empty list is passed to the function. Let's add the check back for safety. Bisecting nothing doesn't make too much sense, but that's no excuse for crashing. Found with GCC's -Wnull-dereference. Signed-off-by: Rene Scharfe --- bisect.c | 6 ++++-- 1 file changed, 4 insertions(+), 2 deletions(-) diff --git a/bisect.c b/bisect.c index 0fca17c02b..2f3008b078 100644 --- a/bisect.c +++ b/bisect.c @@ -229,8 +229,10 @@ static struct commit_list *best_bisection_sorted(struct commit_list *list, int n if (i < cnt - 1) p = p->next; } - free_commit_list(p->next); - p->next = NULL; + if (p) { + free_commit_list(p->next); + p->next = NULL; + } strbuf_release(&buf); free(array); return list; -- 2.15.1 ``` ## Johannes Schindelin, 2018-01-08 20:45 Subject: Re: [PATCH] bisect: avoid NULL pointer dereference Message-ID: URL: https://gitlist.dev/e/nycvar.QRO.7.76.6.1801082145140.31%40ZVAVAG-6OXH6DA.rhebcr.pbec.zvpebfbsg.pbz In-Reply-To: <07a8cdae-8f8c-8560-c450-c6089a942c96@web.de> ``` Hi René, On Mon, 8 Jan 2018, René Scharfe wrote: > 7c117184d7 (bisect: fix off-by-one error in `best_bisection_sorted()`) > fixed an off-by-one error, plugged a memory leak and removed a NULL > check. However, the pointer p *is* actually NULL if an empty list is > passed to the function. Let's add the check back for safety. Bisecting > nothing doesn't make too much sense, but that's no excuse for crashing. > > Found with GCC's -Wnull-dereference. > > Signed-off-by: Rene Scharfe > --- > bisect.c | 6 ++++-- > 1 file changed, 4 insertions(+), 2 deletions(-) > > diff --git a/bisect.c b/bisect.c > index 0fca17c02b..2f3008b078 100644 > --- a/bisect.c > +++ b/bisect.c > @@ -229,8 +229,10 @@ static struct commit_list *best_bisection_sorted(struct commit_list *list, int n > if (i < cnt - 1) > p = p->next; > } > - free_commit_list(p->next); > - p->next = NULL; > + if (p) { > + free_commit_list(p->next); > + p->next = NULL; > + } > strbuf_release(&buf); > free(array); > return list; Isn't this identical to https://public-inbox.org/git/20180103184852.27271-1-avarab@gmail.com/ ? Ciao, Dscho ``` ## René Scharfe, 2018-01-08 21:50 Subject: Re: [PATCH] bisect: avoid NULL pointer dereference Message-ID: <4806374d-144e-5260-b4bf-34c699a980a3@web.de> URL: https://gitlist.dev/e/4806374d-144e-5260-b4bf-34c699a980a3%40web.de In-Reply-To: ``` Hello Dscho, Am 08.01.2018 um 21:45 schrieb Johannes Schindelin: > Isn't this identical to > https://public-inbox.org/git/20180103184852.27271-1-avarab@gmail.com/ ? yes, indeed, thanks. So here's an upvote for Ævar's patch then. :) (I should have sent it earlier, but was not fully convinced it could be triggered in the wild.) René ``` ## Junio C Hamano, 2018-01-08 22:15 Subject: Re: [PATCH] bisect: avoid NULL pointer dereference Message-ID: URL: https://gitlist.dev/e/xmqqr2r06lpp.fsf%40gitster.mtv.corp.google.com In-Reply-To: <07a8cdae-8f8c-8560-c450-c6089a942c96@web.de> ``` René Scharfe writes: > 7c117184d7 (bisect: fix off-by-one error in `best_bisection_sorted()`) > fixed an off-by-one error, plugged a memory leak and removed a NULL > check. However, the pointer p *is* actually NULL if an empty list is > passed to the function. Let's add the check back for safety. Bisecting > nothing doesn't make too much sense, but that's no excuse for crashing. > > Found with GCC's -Wnull-dereference. > > Signed-off-by: Rene Scharfe > --- Thanks. I think this is the same as 2e9fdc79 ("bisect: fix a regression causing a segfault", 2018-01-03) but the log we see here explains what goes wrong much better than the other one ;-) > bisect.c | 6 ++++-- > 1 file changed, 4 insertions(+), 2 deletions(-) > > diff --git a/bisect.c b/bisect.c > index 0fca17c02b..2f3008b078 100644 > --- a/bisect.c > +++ b/bisect.c > @@ -229,8 +229,10 @@ static struct commit_list *best_bisection_sorted(struct commit_list *list, int n > if (i < cnt - 1) > p = p->next; > } > - free_commit_list(p->next); > - p->next = NULL; > + if (p) { > + free_commit_list(p->next); > + p->next = NULL; > + } > strbuf_release(&buf); > free(array); > return list; ```