Re: [PATCH] bisect: avoid NULL pointer dereference

From: Junio C Hamano <gitster@pobox.com>
Date: Jan 8, 2018, 22:15 UTC
Message-ID: <xmqqr2r06lpp.fsf@gitster.mtv.corp.google.com>
In-Reply-To: <07a8cdae-8f8c-8560-c450-c6089a942c96@web.de>

René Scharfe <l.s.r@web.de> writes:

Show 10 quoted lines

> 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 <l.s.r@web.de>
> ---

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 ;-)

Show 20 quoted lines

>  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;