Segmentation fault immediately after 'unset PATH'

Segmentation fault immediately after 'unset PATH'

[Shane Squires]

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

I'm running the most recent version of zsh available on my system (I am not
an admin), which is:

------------------
 % zsh --version
zsh 5.3.1 (x86_64-unknown-linux-gnu)
------------------

The following script, when executed, produces a segmentation fault for me.
This is the most minimal example I can construct.

------------------
File run.zsh:
------------------
#! /usr/bin/zsh

run() {
  typeset -U path=( $path )
  unset PATH
}

run
------------------

------------------
 % ./run.zsh
[1]    13415 segmentation fault  ./run.zsh
------------------

As far as I can tell, the problem has to involve all three elements here:
(1) the type of 'path' has to be modified, (2) 'PATH' has to be unset, and
(3) this has to happen inside a function.

I can imagine that what's going on here is that, for some reason, changing
the type of 'path' from inside a function causes it and 'PATH' to fall "out
of sync" somehow, and this creates problems when 'PATH' is being unset.
But this is extremely vague and speculative, so I won't waste any more time
by discussing it.

Before anyone lectures me on this, I realize that unsetting 'PATH' is not
advisable in general!  It is unfortunately unavoidable in my particular
case.  'PATH' needs to be unset and then immediately set to a new value,
and (because this is being handled in an automated way for many environment
variables) I cannot just immediately re-assign its value.  I will find
another workaround on my end, but I'm reporting this here mainly because it
seems to be a genuine bug in zsh.

Thanks in advance for looking into this--
Shane

Re: Segmentation fault immediately after 'unset PATH'

[Mikael Magnusson]

On 7/10/19, Shane Squires <shane2squires@gmail.com> wrote:
> I'm running the most recent version of zsh available on my system (I am not
> an admin), which is:
>
> ------------------
>  % zsh --version
> zsh 5.3.1 (x86_64-unknown-linux-gnu)
> ------------------
>
> The following script, when executed, produces a segmentation fault for me.
> This is the most minimal example I can construct.
>
> ------------------
> File run.zsh:
> ------------------
> #! /usr/bin/zsh
>
> run() {
>   typeset -U path=( $path )
>   unset PATH
> }
>
> run
> ------------------
>
> ------------------
>  % ./run.zsh
> [1]    13415 segmentation fault  ./run.zsh
> ------------------
>
> As far as I can tell, the problem has to involve all three elements here:
> (1) the type of 'path' has to be modified, (2) 'PATH' has to be unset, and
> (3) this has to happen inside a function.
>
> I can imagine that what's going on here is that, for some reason, changing
> the type of 'path' from inside a function causes it and 'PATH' to fall "out
> of sync" somehow, and this creates problems when 'PATH' is being unset.
> But this is extremely vague and speculative, so I won't waste any more time
> by discussing it.
>
> Before anyone lectures me on this, I realize that unsetting 'PATH' is not
> advisable in general!  It is unfortunately unavoidable in my particular
> case.  'PATH' needs to be unset and then immediately set to a new value,
> and (because this is being handled in an automated way for many environment
> variables) I cannot just immediately re-assign its value.  I will find
> another workaround on my end, but I'm reporting this here mainly because it
> seems to be a genuine bug in zsh.
>
> Thanks in advance for looking into this--
> Shane

This is the backtrace i get with debugging symbols,
Program received signal SIGSEGV, Segmentation fault.
0x00000000004832b8 in unsetparam_pm (pm=0x716a70, altflag=1, exp=1) at
params.c:3614
3614		pm->gsu.s->unsetfn(pm, exp);
(gdb) bt
#0  0x00000000004832b8 in unsetparam_pm (pm=0x716a70, altflag=1,
exp=1) at params.c:3614
#1  0x000000000048338b in unsetparam_pm (pm=0x6e75b0
<special_params+5360>, altflag=0, exp=1)
    at params.c:3634
#2  0x000000000041e22a in bin_unset (name=0x7ffff7fe7760 "unset",
argv=0x7ffff7fe77a8,
    ops=0x7fffffffc2c0, func=31) at builtin.c:3739
#3  0x000000000041093c in execbuiltin (args=0x7ffff7fe7770, assigns=0x0,
    bn=0x6e37a0 <builtins+4416>) at builtin.c:507
#4  0x00000000004394d0 in execcmd_exec (state=0x7fffffffcbe0,
eparams=0x7fffffffc800,
    input=0, output=0, how=18, last1=2, close_if_forked=-1) at exec.c:4096
#5  0x0000000000432dfe in execpline2 (state=0x7fffffffcbe0, pcode=67,
how=18, input=0,
    output=0, last1=0) at exec.c:1929
#6  0x0000000000431a44 in execpline (state=0x7fffffffcbe0,
slcode=4098, how=18, last1=0)
    at exec.c:1660
#7  0x0000000000430d2c in execlist (state=0x7fffffffcbe0,
dont_change_job=1, exiting=0)
    at exec.c:1415
#8  0x0000000000430399 in execode (p=0x7ffff7ff2700,
dont_change_job=1, exiting=0,
    context=0x4c5eca "shfunc") at exec.c:1194
#9  0x000000000043e357 in runshfunc (prog=0x7ffff7ff2700, wrap=0x0,
    name=0x7ffff7fe7170 "(anon)") at exec.c:5979
#10 0x000000000043db7d in doshfunc (shfunc=0x715450,
doshargs=0x7ffff7ff2738, noreturnval=0)
    at exec.c:5829
#11 0x000000000043ca64 in execshfunc (shf=0x715450,
args=0x7ffff7ff2738) at exec.c:5398
#12 0x000000000043c4b9 in execfuncdef (state=0x7fffffffd420,
redir_prog=0x0) at exec.c:5264
#13 0x00000000004306a0 in execsimple (state=0x7fffffffd420) at exec.c:1248
#14 0x0000000000430b57 in execlist (state=0x7fffffffd420,
dont_change_job=0, exiting=1)
    at exec.c:1378
#15 0x0000000000430399 in execode (p=0x7ffff7ff2638,
dont_change_job=0, exiting=1,
    context=0x4c8792 "cmdarg") at exec.c:1194
#16 0x0000000000430261 in execstring (
    s=0x7fffffffda8b "() { typeset -U path=( $path ); unset PATH }",
dont_change_job=0,
    exiting=1, context=0x4c8792 "cmdarg") at exec.c:1160
---Type <return> to continue, or q <return> to quit---q
Quit
(gdb) p pm
$1 = (Param) 0x716a70
(gdb) p pm->gsu.s
$2 = (GsuScalar) 0x0
(gdb) fr 1
#1  0x000000000048338b in unsetparam_pm (pm=0x6e75b0
<special_params+5360>, altflag=0, exp=1)
    at params.c:3634
3634		    unsetparam_pm(altpm, 1, exp);
(gdb) list
3629		    if (oldpm && !altpm->level) {
3630			oldpm->old = NULL;
3631			/* fudge things so removenode isn't called */
3632			altpm->level = 1;
3633		    }
3634		    unsetparam_pm(altpm, 1, exp);
3635		}
3636	
3637		zsfree(altremove);
3638	    }
(gdb) p altpm
$3 = (Param) 0x716a70


Adding a check for that particular NULL causes the following backtrace instead:

Program received signal SIGSEGV, Segmentation fault.
0x00000000004868a5 in scanendscope (hn=0x6e7ce0 <special_params+7200>, flags=0)
    at params.c:5621
5621		    pm->old = tpm->old;
(gdb) p pm
$1 = (Param) 0x6e7ce0 <special_params+7200>
(gdb) p pm->old
$2 = (Param) 0x0
(gdb) p pm
$3 = (Param) 0x6e7ce0 <special_params+7200>
(gdb) p tpm
$4 = (Param) 0x0
(gdb) bt
#0  0x00000000004868a5 in scanendscope (hn=0x6e7ce0
<special_params+7200>, flags=0)
    at params.c:5621
#1  0x000000000044ab7f in scanmatchtable (ht=0x7069c0, pprog=0x0,
sorted=0, flags1=0,
    flags2=0, scanfunc=0x4867af <scanendscope>, scanflags=0) at hashtable.c:428
#2  0x000000000044ac09 in scanhashtable (ht=0x7069c0, sorted=0,
flags1=0, flags2=0,
    scanfunc=0x4867af <scanendscope>, scanflags=0) at hashtable.c:444
#3  0x0000000000486667 in endparamscope () at params.c:5587
#4  0x000000000043e38c in runshfunc (prog=0x7ffff7ff2700, wrap=0x0,
    name=0x7ffff7fe7170 "(anon)") at exec.c:5984
#5  0x000000000043db7d in doshfunc (shfunc=0x715450,
doshargs=0x7ffff7ff2738, noreturnval=0)
    at exec.c:5829
#6  0x000000000043ca64 in execshfunc (shf=0x715450,
args=0x7ffff7ff2738) at exec.c:5398
#7  0x000000000043c4b9 in execfuncdef (state=0x7fffffffd420,
redir_prog=0x0) at exec.c:5264
#8  0x00000000004306a0 in execsimple (state=0x7fffffffd420) at exec.c:1248
#9  0x0000000000430b57 in execlist (state=0x7fffffffd420,
dont_change_job=0, exiting=1)
    at exec.c:1378
#10 0x0000000000430399 in execode (p=0x7ffff7ff2638,
dont_change_job=0, exiting=1,
    context=0x4c8792 "cmdarg") at exec.c:1194
#11 0x0000000000430261 in execstring (
    s=0x7fffffffda8b "() { typeset -U path=( $path ); unset PATH }",
dont_change_job=0,
    exiting=1, context=0x4c8792 "cmdarg") at exec.c:1160
#12 0x0000000000458f20 in init_misc (
    cmd=0x7fffffffda8b "() { typeset -U path=( $path ); unset PATH }",
    zsh_name=0x7fffffffda84 "zsh") at init.c:1374
#13 0x000000000045a309 in zsh_main (argc=3, argv=0x7fffffffd608) at init.c:1758
#14 0x000000000040fa46 in main (argc=3, argv=0x7fffffffd608) at ./main.c:93


-- 
Mikael Magnusson

Re: Segmentation fault immediately after 'unset PATH'

[Peter Stephenson]

On Wed, 2019-07-10 at 23:52 +0200, Mikael Magnusson wrote:
> On 7/10/19, Shane Squires <shane2squires@gmail.com> wrote:
> > The following script, when executed, produces a segmentation fault for me.
> > This is the most minimal example I can construct.
> > 
> > ------------------
> > File run.zsh:
> > ------------------
> > #! /usr/bin/zsh
> > 
> > run() {
> >   typeset -U path=( $path )
> >   unset PATH
> > }
> > 
> > run
>
> This is the backtrace i get with debugging symbols,
> Program received signal SIGSEGV, Segmentation fault.
> 0x00000000004832b8 in unsetparam_pm (pm=0x716a70, altflag=1, exp=1) at
> params.c:3614
> 3614		pm->gsu.s->unsetfn(pm, exp);
> (gdb) bt
> #0  0x00000000004832b8 in unsetparam_pm (pm=0x716a70, altflag=1,
> exp=1) at params.c:3614

The gsu element (get/set/unset handlers) of that parameter element,
which is the special one for path (N.B. not PATH which is linked), are
null.  That shouldn't ever happen, so it's already broken.  Presumably
this is somehow associated with the typeset.

The parameter structure looks otherwise reasonable, except the PM_UNIQUE
flag has gone by this point.  I'm seeing
PM_RESTRICTED|PM_DONTIMPORT|PM_SPECIAL|PM_TIED|PM_ARRAY
all of which make sense.

pws

Re: Segmentation fault immediately after 'unset PATH'

[Peter Stephenson]

hu, 2019-07-11 at 09:44 +0100, Peter Stephenson wrote:
 Wed, 2019-07-10 at 23:52 +0200, Mikael Magnusson wrote:

On 7/10/19, Shane Squires <shane2squires@gmail.com> wrote:
> 
> The following script, when executed, produces a segmentation fault for me.
> This is the most minimal example I can construct.
>  
> ------------------
> File run.zsh:
> ------------------
> #! /usr/bin/zsh
>  
> run() {
>   typeset -U path=( $path )
>   unset PATH
> }
>  
> run

I think the issue is that to be able to save and restore path, which is
special, we stick the old values on the end but keep the special
parameter structure in front.  The value to be saved for the environment
is associated with path, so when we try to unset that we throw away the
real special parameter, which we should simply mark as unset.

It's straightforward to fix the actual crash.

The other business here is that the "unset" unsets the global PATH, as
there isn't a local parameter, just the local path which refers to the
same data but has a separate parameter interface.  So PATH remains unset
until you explicitly set it again.

It's deliberate that you can unset one of the variables and still leave
the other visible.  Given that, forcing them both to be local at the
same time is a bit murky, and I can more or less guarantee a bug-prone
mess if we try.

pws

diff --git a/Src/params.c b/Src/params.c
index 1859c7c12..95181f533 100644
--- a/Src/params.c
+++ b/Src/params.c
@@ -3617,10 +3617,18 @@ unsetparam_pm(Param pm, int altflag, int exp)
 	altpm = (Param) paramtab->getnode(paramtab, altremove);
 	/* tied parameters are at the same local level as each other */
 	oldpm = NULL;
-	while (altpm && altpm->level > pm->level) {
-	    /* param under alternate name hidden by a local */
-	    oldpm = altpm;
-	    altpm = altpm->old;
+	/*
+	 * Look for param under alternate name hidden by a local.
+	 * If this parameter is special, however, the visible
+	 * parameter is the special and the hidden one is keeping
+	 * and old value --- we just mark the visible one as unset.
+	 */
+	if (altpm && !(altpm->node.flags & PM_SPECIAL))
+	{
+	    while (altpm && altpm->level > pm->level) {
+		oldpm = altpm;
+		altpm = altpm->old;
+	    }
 	}
 	if (altpm) {
 	    if (oldpm && !altpm->level) {

Re: Segmentation fault immediately after 'unset PATH'

[Peter Stephenson]

On Thu, 2019-07-11 at 11:48 +0100, Peter Stephenson wrote:
> diff --git a/Src/params.c b/Src/params.c
> index 1859c7c12..95181f533 100644
> --- a/Src/params.c
> +++ b/Src/params.c
> @@ -3617,10 +3617,18 @@ unsetparam_pm(Param pm, int altflag, int exp)
>  	altpm = (Param) paramtab->getnode(paramtab, altremove);
>  	/* tied parameters are at the same local level as each other */
>  	oldpm = NULL;
> -	while (altpm && altpm->level > pm->level) {
> -	    /* param under alternate name hidden by a local */
> -	    oldpm = altpm;
> -	    altpm = altpm->old;
> +	/*
> +	 * Look for param under alternate name hidden by a local.
> +	 * If this parameter is special, however, the visible
> +	 * parameter is the special and the hidden one is keeping
> +	 * and old value --- we just mark the visible one as unset.
> +	 */
> +	if (altpm && !(altpm->node.flags & PM_SPECIAL))
> +	{
> +	    while (altpm && altpm->level > pm->level) {
> +		oldpm = altpm;
> +		altpm = altpm->old;
> +	    }
>  	}
>  	if (altpm) {
>  	    if (oldpm && !altpm->level) {

Hmm... can that loop I've "if"ed out ever be right?  It's meaning we're
unsetting the saved version of a parameter, not the visible one.  That
looks pretty fishy at any time.

pws