zsh-workers
 help / color / mirror / code / Atom feed
* [PATCH] {,un}applychange: do not call zle_setline(NULL) if quietgethist() fails
@ 2019-06-26 10:19 Kamil Dudka
       [not found] ` <249696510.1723352.1561545091592@mail2.virginmedia.com>
  2019-06-27 23:01 ` Oliver Kiddle
  0 siblings, 2 replies; 8+ messages in thread
From: Kamil Dudka @ 2019-06-26 10:19 UTC (permalink / raw)
  To: zsh-workers

There is a bug report in Red Hat Bugzilla about zsh crashing on NULL
pointer dereference: https://bugzilla.redhat.com/1722703

I was not able to reproduce the crash myself but the attached patch
should prevent zsh from crashing in this situation.
---
 Src/Zle/zle_utils.c | 8 ++++++--
 1 file changed, 6 insertions(+), 2 deletions(-)

diff --git a/Src/Zle/zle_utils.c b/Src/Zle/zle_utils.c
index 0277d4917..8081d3adc 100644
--- a/Src/Zle/zle_utils.c
+++ b/Src/Zle/zle_utils.c
@@ -1607,7 +1607,9 @@ static int
 unapplychange(struct change *ch)
 {
     if(ch->hist != histline) {
-	zle_setline(quietgethist(ch->hist));
+	Histent he = quietgethist(ch->hist);
+	if(he)
+	    zle_setline(he);
 	zlecs = ch->new_cs;
 	return 0;
     }
@@ -1647,7 +1649,9 @@ static int
 applychange(struct change *ch)
 {
     if(ch->hist != histline) {
-	zle_setline(quietgethist(ch->hist));
+	Histent he = quietgethist(ch->hist);
+	if(he)
+	    zle_setline(he);
 	zlecs = ch->old_cs;
 	return 0;
     }
-- 
2.20.1


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

* Fwd: Re: [PATCH] {,un}applychange: do not call zle_setline(NULL) if quietgethist() fails
       [not found] ` <249696510.1723352.1561545091592@mail2.virginmedia.com>
@ 2019-06-26 11:00   ` Peter Stephenson
  2019-06-26 12:07     ` Kamil Dudka
  0 siblings, 1 reply; 8+ messages in thread
From: Peter Stephenson @ 2019-06-26 11:00 UTC (permalink / raw)
  To: zsh workers

Sorry, this should have gone to the list...

---------- Original Message ----------
From: Peter Stephenson <p.w.stephenson@ntlworld.com>
To: Kamil Dudka <kdudka@redhat.com>
Date: 26 June 2019 at 11:31
Subject: Re: [PATCH] {,un}applychange: do not call zle_setline(NULL) if quietgethist() fails


> On 26 June 2019 at 11:19 Kamil Dudka <kdudka@redhat.com> wrote:
> 
> 
> There is a bug report in Red Hat Bugzilla about zsh crashing on NULL
> pointer dereference: https://bugzilla.redhat.com/1722703
> 
> I was not able to reproduce the crash myself but the attached patch
> should prevent zsh from crashing in this situation.

Hmm... I'm guessing that in the failure case we probably shouldn't
set zlecs either?  It's probably not going to do anything helpful.
Possibly also return 1?

pws

>  Src/Zle/zle_utils.c | 8 ++++++--
>  1 file changed, 6 insertions(+), 2 deletions(-)
> 
> diff --git a/Src/Zle/zle_utils.c b/Src/Zle/zle_utils.c
> index 0277d4917..8081d3adc 100644
> --- a/Src/Zle/zle_utils.c
> +++ b/Src/Zle/zle_utils.c
> @@ -1607,7 +1607,9 @@ static int
>  unapplychange(struct change *ch)
>  {
>      if(ch->hist != histline) {
> -	zle_setline(quietgethist(ch->hist));
> +	Histent he = quietgethist(ch->hist);
> +	if(he)
> +	    zle_setline(he);
>  	zlecs = ch->new_cs;
>  	return 0;
>      }
> @@ -1647,7 +1649,9 @@ static int
>  applychange(struct change *ch)
>  {
>      if(ch->hist != histline) {
> -	zle_setline(quietgethist(ch->hist));
> +	Histent he = quietgethist(ch->hist);
> +	if(he)
> +	    zle_setline(he);
>  	zlecs = ch->old_cs;
>  	return 0;
>      }
> -- 
> 2.20.1
>

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

* Re: Fwd: Re: [PATCH] {,un}applychange: do not call zle_setline(NULL) if quietgethist() fails
  2019-06-26 11:00   ` Fwd: " Peter Stephenson
@ 2019-06-26 12:07     ` Kamil Dudka
  0 siblings, 0 replies; 8+ messages in thread
From: Kamil Dudka @ 2019-06-26 12:07 UTC (permalink / raw)
  To: Peter Stephenson; +Cc: zsh-workers

On Wednesday, June 26, 2019 1:00:44 PM CEST Peter Stephenson wrote:
> > On 26 June 2019 at 11:19 Kamil Dudka <kdudka@redhat.com> wrote:
> > 
> > 
> > There is a bug report in Red Hat Bugzilla about zsh crashing on NULL
> > pointer dereference: https://bugzilla.redhat.com/1722703
> > 
> > I was not able to reproduce the crash myself but the attached patch
> > should prevent zsh from crashing in this situation.
> 
> Hmm... I'm guessing that in the failure case we probably shouldn't
> set zlecs either?  It's probably not going to do anything helpful.
> Possibly also return 1?
> 
> pws

Good point.  zlecs should not be updated when zleline is not updated.
Sorry for missing it in the proposed patch.  Returning 1 might also be
better although I do not really understand why quietgethist() returns
NULL in the mentioned case.

Kamil



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

* Re: [PATCH] {,un}applychange: do not call zle_setline(NULL) if quietgethist() fails
  2019-06-26 10:19 [PATCH] {,un}applychange: do not call zle_setline(NULL) if quietgethist() fails Kamil Dudka
       [not found] ` <249696510.1723352.1561545091592@mail2.virginmedia.com>
@ 2019-06-27 23:01 ` Oliver Kiddle
  2019-06-28  0:20   ` Mikael Magnusson
                     ` (2 more replies)
  1 sibling, 3 replies; 8+ messages in thread
From: Oliver Kiddle @ 2019-06-27 23:01 UTC (permalink / raw)
  To: Kamil Dudka; +Cc: zsh-workers

Kamil Dudka wrote:
> There is a bug report in Red Hat Bugzilla about zsh crashing on NULL
> pointer dereference: https://bugzilla.redhat.com/1722703
>
> I was not able to reproduce the crash myself but the attached patch
> should prevent zsh from crashing in this situation.

It'd perhaps be good to add a DPUTS call here because while it is good
not to crash the condition does perhaps indicate a bug and it'd be
better not to hide it completely. I can't reproduce the crash either but
there must be some plugins or url quoting magic going on here.
More details of the setup used by the user might help find the
underlying problem.

Oliver

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

* Re: [PATCH] {,un}applychange: do not call zle_setline(NULL) if quietgethist() fails
  2019-06-27 23:01 ` Oliver Kiddle
@ 2019-06-28  0:20   ` Mikael Magnusson
  2019-06-28  8:12   ` Kamil Dudka
  2019-07-23 13:45   ` [PATCH v2] " Kamil Dudka
  2 siblings, 0 replies; 8+ messages in thread
From: Mikael Magnusson @ 2019-06-28  0:20 UTC (permalink / raw)
  To: Oliver Kiddle; +Cc: Kamil Dudka, zsh-workers

I can reproduce *a* crash, not necessarily *the* crash, by making a
2x2 window and then maximizing/unmaximizing the window. It seems to
not reproduce if I have run any commands in the shell.

On 6/28/19, Oliver Kiddle <okiddle@yahoo.co.uk> wrote:
> Kamil Dudka wrote:
>> There is a bug report in Red Hat Bugzilla about zsh crashing on NULL
>> pointer dereference: https://bugzilla.redhat.com/1722703
>>
>> I was not able to reproduce the crash myself but the attached patch
>> should prevent zsh from crashing in this situation.
>
> It'd perhaps be good to add a DPUTS call here because while it is good
> not to crash the condition does perhaps indicate a bug and it'd be
> better not to hide it completely. I can't reproduce the crash either but
> there must be some plugins or url quoting magic going on here.
> More details of the setup used by the user might help find the
> underlying problem.
>
> Oliver
>


-- 
Mikael Magnusson

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

* Re: [PATCH] {,un}applychange: do not call zle_setline(NULL) if quietgethist() fails
  2019-06-27 23:01 ` Oliver Kiddle
  2019-06-28  0:20   ` Mikael Magnusson
@ 2019-06-28  8:12   ` Kamil Dudka
  2019-07-23 13:45   ` [PATCH v2] " Kamil Dudka
  2 siblings, 0 replies; 8+ messages in thread
From: Kamil Dudka @ 2019-06-28  8:12 UTC (permalink / raw)
  To: Oliver Kiddle; +Cc: zsh-workers

On Friday, June 28, 2019 1:01:25 AM CEST Oliver Kiddle wrote:
> Kamil Dudka wrote:
> > There is a bug report in Red Hat Bugzilla about zsh crashing on NULL
> > pointer dereference: https://bugzilla.redhat.com/1722703
> > 
> > I was not able to reproduce the crash myself but the attached patch
> > should prevent zsh from crashing in this situation.
> 
> It'd perhaps be good to add a DPUTS call here because while it is good
> not to crash the condition does perhaps indicate a bug and it'd be
> better not to hide it completely. I can't reproduce the crash either but
> there must be some plugins or url quoting magic going on here.
> More details of the setup used by the user might help find the
> underlying problem.
> 
> Oliver

Thank you for analyzing it!  I do not have any more information than
what is available in the mentioned bugzilla.  So I asked the reporter
about their setup.

Kamil



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

* [PATCH v2] {,un}applychange: do not call zle_setline(NULL) if quietgethist() fails
  2019-06-27 23:01 ` Oliver Kiddle
  2019-06-28  0:20   ` Mikael Magnusson
  2019-06-28  8:12   ` Kamil Dudka
@ 2019-07-23 13:45   ` Kamil Dudka
  2019-07-26 15:24     ` Kamil Dudka
  2 siblings, 1 reply; 8+ messages in thread
From: Kamil Dudka @ 2019-07-23 13:45 UTC (permalink / raw)
  To: Oliver Kiddle; +Cc: Peter Stephenson, zsh-workers

There is a bug report in Red Hat Bugzilla about zsh crashing on NULL
pointer dereference: https://bugzilla.redhat.com/1722703

I was not able to reproduce the crash myself but the attached patch
should prevent zsh from crashing in this situation.
---
 Src/Zle/zle_utils.c | 14 ++++++++++++--
 1 file changed, 12 insertions(+), 2 deletions(-)

diff --git a/Src/Zle/zle_utils.c b/Src/Zle/zle_utils.c
index 0277d4917..d549b885b 100644
--- a/Src/Zle/zle_utils.c
+++ b/Src/Zle/zle_utils.c
@@ -1607,7 +1607,12 @@ static int
 unapplychange(struct change *ch)
 {
     if(ch->hist != histline) {
-	zle_setline(quietgethist(ch->hist));
+	Histent he = quietgethist(ch->hist);
+	if(!he) {
+	    dputs(ERRMSG("quietgethist(ch->hist) returned NULL"));
+	    return 1;
+	}
+	zle_setline(he);
 	zlecs = ch->new_cs;
 	return 0;
     }
@@ -1647,7 +1652,12 @@ static int
 applychange(struct change *ch)
 {
     if(ch->hist != histline) {
-	zle_setline(quietgethist(ch->hist));
+	Histent he = quietgethist(ch->hist);
+	if(!he) {
+	    dputs(ERRMSG("quietgethist(ch->hist) returned NULL"));
+	    return 1;
+	}
+	zle_setline(he);
 	zlecs = ch->old_cs;
 	return 0;
     }
-- 
2.20.1


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

* Re: [PATCH v2] {,un}applychange: do not call zle_setline(NULL) if quietgethist() fails
  2019-07-23 13:45   ` [PATCH v2] " Kamil Dudka
@ 2019-07-26 15:24     ` Kamil Dudka
  0 siblings, 0 replies; 8+ messages in thread
From: Kamil Dudka @ 2019-07-26 15:24 UTC (permalink / raw)
  To: Peter Stephenson; +Cc: zsh-workers, Oliver Kiddle

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

Sorry, I used dputs() incorrectly, which caused undefined symbols
in a non-debug build.  The attached patch fixes it.  Sorry for the
troubles!

Kamil


On Tuesday, July 23, 2019 3:45:48 PM CEST Kamil Dudka wrote:
> There is a bug report in Red Hat Bugzilla about zsh crashing on NULL
> pointer dereference: https://bugzilla.redhat.com/1722703
> 
> I was not able to reproduce the crash myself but the attached patch
> should prevent zsh from crashing in this situation.
> ---
>  Src/Zle/zle_utils.c | 14 ++++++++++++--
>  1 file changed, 12 insertions(+), 2 deletions(-)
> 
> diff --git a/Src/Zle/zle_utils.c b/Src/Zle/zle_utils.c
> index 0277d4917..d549b885b 100644
> --- a/Src/Zle/zle_utils.c
> +++ b/Src/Zle/zle_utils.c
> @@ -1607,7 +1607,12 @@ static int
>  unapplychange(struct change *ch)
>  {
>      if(ch->hist != histline) {
> -	zle_setline(quietgethist(ch->hist));
> +	Histent he = quietgethist(ch->hist);
> +	if(!he) {
> +	    dputs(ERRMSG("quietgethist(ch->hist) returned NULL"));
> +	    return 1;
> +	}
> +	zle_setline(he);
>  	zlecs = ch->new_cs;
>  	return 0;
>      }
> @@ -1647,7 +1652,12 @@ static int
>  applychange(struct change *ch)
>  {
>      if(ch->hist != histline) {
> -	zle_setline(quietgethist(ch->hist));
> +	Histent he = quietgethist(ch->hist);
> +	if(!he) {
> +	    dputs(ERRMSG("quietgethist(ch->hist) returned NULL"));
> +	    return 1;
> +	}
> +	zle_setline(he);
>  	zlecs = ch->old_cs;
>  	return 0;
>      }

[-- Attachment #2: 0001-zle_utils-fix-incorrect-use-of-dputs.patch --]
[-- Type: text/x-patch, Size: 1210 bytes --]

From 97c195f6de8c566e88c0bb753aadc8217dc37875 Mon Sep 17 00:00:00 2001
From: Kamil Dudka <kdudka@redhat.com>
Date: Fri, 26 Jul 2019 17:17:40 +0200
Subject: [PATCH] zle_utils: fix incorrect use of dputs()

... that caused undefined symbols in a non-debug build
---
 Src/Zle/zle_utils.c | 10 ++++------
 1 file changed, 4 insertions(+), 6 deletions(-)

diff --git a/Src/Zle/zle_utils.c b/Src/Zle/zle_utils.c
index d549b885b..2b306fdcd 100644
--- a/Src/Zle/zle_utils.c
+++ b/Src/Zle/zle_utils.c
@@ -1608,10 +1608,9 @@ unapplychange(struct change *ch)
 {
     if(ch->hist != histline) {
 	Histent he = quietgethist(ch->hist);
-	if(!he) {
-	    dputs(ERRMSG("quietgethist(ch->hist) returned NULL"));
+	DPUTS(he == NULL, "quietgethist(ch->hist) returned NULL");
+	if(he == NULL)
 	    return 1;
-	}
 	zle_setline(he);
 	zlecs = ch->new_cs;
 	return 0;
@@ -1653,10 +1652,9 @@ applychange(struct change *ch)
 {
     if(ch->hist != histline) {
 	Histent he = quietgethist(ch->hist);
-	if(!he) {
-	    dputs(ERRMSG("quietgethist(ch->hist) returned NULL"));
+	DPUTS(he == NULL, "quietgethist(ch->hist) returned NULL");
+	if(he == NULL)
 	    return 1;
-	}
 	zle_setline(he);
 	zlecs = ch->old_cs;
 	return 0;
-- 
2.20.1


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

end of thread, other threads:[~2019-07-26 15:25 UTC | newest]

Thread overview: 8+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2019-06-26 10:19 [PATCH] {,un}applychange: do not call zle_setline(NULL) if quietgethist() fails Kamil Dudka
     [not found] ` <249696510.1723352.1561545091592@mail2.virginmedia.com>
2019-06-26 11:00   ` Fwd: " Peter Stephenson
2019-06-26 12:07     ` Kamil Dudka
2019-06-27 23:01 ` Oliver Kiddle
2019-06-28  0:20   ` Mikael Magnusson
2019-06-28  8:12   ` Kamil Dudka
2019-07-23 13:45   ` [PATCH v2] " Kamil Dudka
2019-07-26 15:24     ` Kamil Dudka

Code repositories for project(s) associated with this public inbox

	https://git.vuxu.org/mirror/zsh/

This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).