git@vger.kernel.org list mirror (unofficial, one of many)
 help / color / mirror / code / Atom feed
* [PATCH] bisect: avoid NULL pointer dereference
@ 2018-01-08 20:36 René Scharfe
  2018-01-08 20:45 ` Johannes Schindelin
  2018-01-08 22:15 ` Junio C Hamano
  0 siblings, 2 replies; 4+ messages in thread
From: René Scharfe @ 2018-01-08 20:36 UTC (permalink / raw)
  To: Git List; +Cc: Martin Ågren, Junio C Hamano

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

^ permalink raw reply	[flat|nested] 4+ messages in thread

* Re: [PATCH] bisect: avoid NULL pointer dereference
  2018-01-08 20:36 [PATCH] bisect: avoid NULL pointer dereference René Scharfe
@ 2018-01-08 20:45 ` Johannes Schindelin
  2018-01-08 21:50   ` René Scharfe
  2018-01-08 22:15 ` Junio C Hamano
  1 sibling, 1 reply; 4+ messages in thread
From: Johannes Schindelin @ 2018-01-08 20:45 UTC (permalink / raw)
  To: René Scharfe; +Cc: Git List, Martin Ågren, Junio C Hamano

[-- Attachment #1: Type: text/plain, Size: 1179 bytes --]

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

^ permalink raw reply	[flat|nested] 4+ messages in thread

* Re: [PATCH] bisect: avoid NULL pointer dereference
  2018-01-08 20:45 ` Johannes Schindelin
@ 2018-01-08 21:50   ` René Scharfe
  0 siblings, 0 replies; 4+ messages in thread
From: René Scharfe @ 2018-01-08 21:50 UTC (permalink / raw)
  To: Johannes Schindelin; +Cc: Git List, Martin Ågren, Junio C Hamano

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é

^ permalink raw reply	[flat|nested] 4+ messages in thread

* Re: [PATCH] bisect: avoid NULL pointer dereference
  2018-01-08 20:36 [PATCH] bisect: avoid NULL pointer dereference René Scharfe
  2018-01-08 20:45 ` Johannes Schindelin
@ 2018-01-08 22:15 ` Junio C Hamano
  1 sibling, 0 replies; 4+ messages in thread
From: Junio C Hamano @ 2018-01-08 22:15 UTC (permalink / raw)
  To: René Scharfe
  Cc: Git List, Martin Ågren, Ævar Arnfjörð Bjarmason

René Scharfe <l.s.r@web.de> 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 <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 ;-)

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

^ permalink raw reply	[flat|nested] 4+ messages in thread

end of thread, other threads:[~2018-01-08 22:15 UTC | newest]

Thread overview: 4+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2018-01-08 20:36 [PATCH] bisect: avoid NULL pointer dereference René Scharfe
2018-01-08 20:45 ` Johannes Schindelin
2018-01-08 21:50   ` René Scharfe
2018-01-08 22:15 ` Junio C Hamano

git@vger.kernel.org list mirror (unofficial, one of many)

This inbox may be cloned and mirrored by anyone:

	git clone --mirror https://public-inbox.org/git
	git clone --mirror http://ou63pmih66umazou.onion/git
	git clone --mirror http://czquwvybam4bgbro.onion/git
	git clone --mirror http://hjrcffqmbrq6wope.onion/git

	# If you have public-inbox 1.1+ installed, you may
	# initialize and index your mirror using the following commands:
	public-inbox-init -V1 git git/ https://public-inbox.org/git \
		git@vger.kernel.org
	public-inbox-index git

Example config snippet for mirrors.
Newsgroups are available over NNTP:
	nntp://news.public-inbox.org/inbox.comp.version-control.git
	nntp://7fh6tueqddpjyxjmgtdiueylzoqt6pt7hec3pukyptlmohoowvhde4yd.onion/inbox.comp.version-control.git
	nntp://ie5yzdi7fg72h7s4sdcztq5evakq23rdt33mfyfcddc5u3ndnw24ogqd.onion/inbox.comp.version-control.git
	nntp://4uok3hntl7oi7b4uf4rtfwefqeexfzil2w6kgk2jn5z2f764irre7byd.onion/inbox.comp.version-control.git
	nntp://news.gmane.io/gmane.comp.version-control.git
 note: .onion URLs require Tor: https://www.torproject.org/

code repositories for project(s) associated with this inbox:

	https://80x24.org/mirrors/git.git

AGPL code for this site: git clone https://public-inbox.org/public-inbox.git