zsh segfaults with lots of data in one variable

zsh segfaults with lots of data in one variable

[Frank Terbeck]

Hey,

I just wanted to see if lines="$(<&0)" plays nice with binary input
data, when zsh crashed on me.

What I did is this:

[snip]
zsh% slurp() { lines="$(<&0)" }
zsh% slurp < ./a-big-71MiB-mp3-file.mp3
zsh% printf '%s' $lines > foo.mp3
[snap]

This segfaults after the printf. m5sum says a-big-71MiB-mp3-file.mp3
and foo.mp3 are the same.

Of course, that's nothing I would normally do, because there's
cp(1). :-) - Still, zsh shouldn't segfault, I guess.

To verify this still applies, I built the current cvs head and it's
still there - and it is; here is a backtrace from gdb:

[snip]
GNU gdb 6.8-debian
Copyright (C) 2008 Free Software Foundation, Inc.
License GPLv3+: GNU GPL version 3 or later <http://gnu.org/licenses/gpl.html>
This is free software: you are free to change and redistribute it.
There is NO WARRANTY, to the extent permitted by law.  Type "show copying"
and "show warranty" for details.
This GDB was configured as "i486-linux-gnu"...
Reading symbols from /usr/lib/libgdbm.so.3...done.
Loaded symbols for /usr/lib/libgdbm.so.3
Reading symbols from /usr/lib/libpcre.so.3...done.
Loaded symbols for /usr/lib/libpcre.so.3
Reading symbols from /lib/libdl.so.2...done.
Loaded symbols for /lib/libdl.so.2
Reading symbols from /lib/libncursesw.so.5...done.
Loaded symbols for /lib/libncursesw.so.5
Reading symbols from /lib/libm.so.6...done.
Loaded symbols for /lib/libm.so.6
Reading symbols from /lib/libc.so.6...done.
Loaded symbols for /lib/libc.so.6
Reading symbols from /lib/ld-linux.so.2...done.
Loaded symbols for /lib/ld-linux.so.2
Reading symbols from /lib/libnss_compat.so.2...done.
Loaded symbols for /lib/libnss_compat.so.2
Reading symbols from /lib/libnsl.so.1...done.
Loaded symbols for /lib/libnsl.so.1
Reading symbols from /lib/libnss_nis.so.2...done.
Loaded symbols for /lib/libnss_nis.so.2
Reading symbols from /lib/libnss_files.so.2...done.
Loaded symbols for /lib/libnss_files.so.2
Reading symbols from /usr/lib/gconv/ISO8859-15.so...done.
Loaded symbols for /usr/lib/gconv/ISO8859-15.so
Reading symbols from /mnt/extern/builds/zsh/installations/lib/zsh/4.3.9-dev-1/zsh/parameter.so...done.
Loaded symbols for /mnt/extern/builds/zsh/installations/lib/zsh/4.3.9-dev-1/zsh/parameter.so
Reading symbols from /mnt/extern/builds/zsh/installations/lib/zsh/4.3.9-dev-1/zsh/zle.so...done.
Loaded symbols for /mnt/extern/builds/zsh/installations/lib/zsh/4.3.9-dev-1/zsh/zle.so
Reading symbols from /mnt/extern/builds/zsh/installations/lib/zsh/4.3.9-dev-1/zsh/zleparameter.so...done.
Loaded symbols for /mnt/extern/builds/zsh/installations/lib/zsh/4.3.9-dev-1/zsh/zleparameter.so
Reading symbols from /mnt/extern/builds/zsh/installations/lib/zsh/4.3.9-dev-1/zsh/deltochar.so...done.
Loaded symbols for /mnt/extern/builds/zsh/installations/lib/zsh/4.3.9-dev-1/zsh/deltochar.so
Reading symbols from /mnt/extern/builds/zsh/installations/lib/zsh/4.3.9-dev-1/zsh/mathfunc.so...done.
Loaded symbols for /mnt/extern/builds/zsh/installations/lib/zsh/4.3.9-dev-1/zsh/mathfunc.so
Reading symbols from /mnt/extern/builds/zsh/installations/lib/zsh/4.3.9-dev-1/zsh/curses.so...done.
Loaded symbols for /mnt/extern/builds/zsh/installations/lib/zsh/4.3.9-dev-1/zsh/curses.so
Reading symbols from /mnt/extern/builds/zsh/installations/lib/zsh/4.3.9-dev-1/zsh/pcre.so...done.
Loaded symbols for /mnt/extern/builds/zsh/installations/lib/zsh/4.3.9-dev-1/zsh/pcre.so
Reading symbols from /mnt/extern/builds/zsh/installations/lib/zsh/4.3.9-dev-1/zsh/complete.so...done.
Loaded symbols for /mnt/extern/builds/zsh/installations/lib/zsh/4.3.9-dev-1/zsh/complete.so
Reading symbols from /mnt/extern/builds/zsh/installations/lib/zsh/4.3.9-dev-1/zsh/zutil.so...done.
Loaded symbols for /mnt/extern/builds/zsh/installations/lib/zsh/4.3.9-dev-1/zsh/zutil.so
Reading symbols from /mnt/extern/builds/zsh/installations/lib/zsh/4.3.9-dev-1/zsh/computil.so...done.
Loaded symbols for /mnt/extern/builds/zsh/installations/lib/zsh/4.3.9-dev-1/zsh/computil.so
Reading symbols from /mnt/extern/builds/zsh/installations/lib/zsh/4.3.9-dev-1/zsh/complist.so...done.
Loaded symbols for /mnt/extern/builds/zsh/installations/lib/zsh/4.3.9-dev-1/zsh/complist.so
Reading symbols from /mnt/extern/builds/zsh/installations/lib/zsh/4.3.9-dev-1/zsh/rlimits.so...done.
Loaded symbols for /mnt/extern/builds/zsh/installations/lib/zsh/4.3.9-dev-1/zsh/rlimits.so
Reading symbols from /mnt/extern/builds/zsh/installations/lib/zsh/4.3.9-dev-1/zsh/terminfo.so...done.
Loaded symbols for /mnt/extern/builds/zsh/installations/lib/zsh/4.3.9-dev-1/zsh/terminfo.so
Core was generated by `/mnt/extern/builds/zsh/installations/bin/zsh'.
Program terminated with signal 11, Segmentation fault.
[New process 21102]
#0  0x080735ac in runshfunc (prog=0x89c82e8, wrap=0xb7b55c20, name=0xb7f49258 "precmd") at exec.c:4437
4437	    memcpy(ou, underscore, underscoreused);
(gdb) #0  0x080735ac in runshfunc (prog=0x89c82e8, wrap=0xb7b55c20, name=0xb7f49258 "precmd") at exec.c:4437
	cont = 145583904
	ou = 0xbad047d0 <Address 0xbad047d0 out of bounds>
#1  0x08073309 in doshfunc (shfunc=0x89eaec0, doshargs=0x0, noreturnval=1) at exec.c:4352
	tab = (char **) 0x899f018
	x = (char **) 0x4b
	oargv0 = 0x899ca10 "/mnt/extern/builds/zsh/installations/bin/zsh"
	oldzoptind = 1
	oldlastval = 0
	oldoptcind = 0
	oldnumpipestats = 1
	ret = 144345656
	oldpipestats = (int *) 0xb7f49248
	saveopts = "\000\001\000\001\000\001\000\000\001\001\000\000\001\001\001\000\001\001\001\000\000\000\000\001\000\001\001\001\000\001\001\001\001\000\001\001\001\000\000\000\000\000\000\000\001\001\001\000\000\001\001\000\001\001\001\001\000\000\000\000\000\000\001\001\001\000\000\000\000\001\001\001\000\000\000\000\001\001\000\000\000\000\000\000\001\000\000\000\000\000\000\000\001\000\000\000\001\001\000\000\001\000\000\000\000\001\001\001\001\001\001\000\000\001\000\000\000\000\000\000\000\000\000\000\001\000\001\001\001\000\000\001\000\001\000\001\000\000\000\000\000\000\001\000\000\001\000\000\000\000\001\000\000\001\000\000\001\000\001"
	oldscriptname = 0x0
	name = 0x89a8a38 "precmd"
	flags = 0
	fname = 0xb7f49240 "precmd"
	obreaks = 0
	saveemulation = 48
	savesticky_emulation = 0
	restore_sticky = 0
	prog = (Eprog) 0x89c82e8
	fstack = {prev = 0x0, name = 0xb7f49258 "precmd", filename = 0xb7f49290 "/home/hawk/etc/zsh/zshrc.d/zfunct", 
  caller = 0xb7f49260 "/mnt/extern/builds/zsh/installations/bin/zsh", flineno = 12, lineno = 4, tp = 1}
	funcdepth = 1
#2  0x080cdab8 in callhookfunc (name=0x80e48b7 "precmd", lnklst=0x0, arrayp=1, retval=0x0) at utils.c:1170
	shfunc = (Shfunc) 0x89eaec0
	osc = 0
	osm = 0
	stat = 1
	ret = 0
#3  0x080cdd94 in preprompt () at utils.c:1237
	ln = (LinkNode) 0x0
	period = 0
	mailcheck = 60
	lastperiodic = 0
#4  0x08084460 in loop (toplevel=1, justonce=0) at init.c:118
	hstop = 0
	prog = (Eprog) 0xb7f49990
#5  0x0808727d in zsh_main (argc=1, argv=0xbfb75554) at init.c:1405
	t = (char **) 0xbfb75558
	t0 = 158
#6  0x08054b96 in main (argc=) at ./main.c:93
No locals.
(gdb) 
[snap]

So, that's somewhere in the vicinity of 'precmd()'. And indeed, if I
start 'zsh -f' the segfault is gone.

So, let's try this:

[snip]
zsh% /mnt/extern/builds/zsh/installations/bin/zsh -f
zsh% precmd() { print "Hello from precmd()." }
Hello from precmd().
zsh% slurp() { lines="$(<&0)" }
Hello from precmd().
zsh% slurp < ./a-big-71MiB-mp3-file.mp3
Hello from precmd().
zsh% printf '%s' $lines > foo.mp3
[1]    21879 segmentation fault (core dumped)  /mnt/extern/builds/zsh/installations/bin/zsh -f
[snap]

The md5sum of the original file and foo.mp3 is still the same,
so that seems to work.

I tried a ~500KiB text file with which the problem didn't occur.

I hope I narrowed the problem down enough. Let me know if you need to
know anything else.

Regards, Frank

-- 
In protocol design, perfection has been reached not when there is
nothing left to add, but when there is nothing left to take away.
                                                  -- RFC 1925

Re: zsh segfaults with lots of data in one variable

[Peter Stephenson]

On Tue, 3 Mar 2009 22:07:29 +0100
Frank Terbeck <ft@bewatermyfriend.org> wrote:
> I just wanted to see if lines="$(<&0)" plays nice with binary input
> data, when zsh crashed on me.
> 
> What I did is this:
> 
> [snip]
> zsh% slurp() { lines="$(<&0)" }
> zsh% slurp < ./a-big-71MiB-mp3-file.mp3
> zsh% printf '%s' $lines > foo.mp3

This is down to a combination of implementation issues including (I
suspect, depending on your system) how gcc works.

For every command the shell saves the previous last argument in $_.
This happens after expansion, so in that last line it was the complete
value of "$lines".  (This indicates how inefficient it can be to get the
shell to do things that aren't really it's job...)

When executing a shell function, $_ is saved and restored.  If gcc or
some other compiler that supports variable length arrays is around,
we'll use that.  This memory then goes on the stack.  In the case of the
multimegabyte file this caused bad karma.

valgrind gave me an odd message about the stack being switched around
when this happened, so I can't exclude the possibility that the compiler or
the system is getting unnecessarily muddled.  Someone more au fait with
the internals might want to try this with an unpatched version of the
shell.

However, in our case it's easy to fix just using ordindary malloc memory
for saving $_.  It would be much more efficient to mark underscore as
"push old value to stack on write", or something similar, but if I try
that at ten o'clock in the evening I'll get it wrong.

Index: Src/exec.c
===================================================================
RCS file: /cvsroot/zsh/zsh/Src/exec.c,v
retrieving revision 1.163
diff -u -r1.163 exec.c
--- Src/exec.c	11 Feb 2009 20:42:16 -0000	1.163
+++ Src/exec.c	3 Mar 2009 21:54:34 -0000
@@ -4431,10 +4431,12 @@
 mod_export void
 runshfunc(Eprog prog, FuncWrap wrap, char *name)
 {
-    int cont;
-    VARARR(char, ou, underscoreused);
+    int cont, ouu;
+    char *ou;
 
-    memcpy(ou, underscore, underscoreused);
+    ou = zalloc(ouu = underscoreused);
+    if (ou)
+	memcpy(ou, underscore, underscoreused);
 
     while (wrap) {
 	wrap->module->wrapper++;
@@ -4445,13 +4447,19 @@
 	    (wrap->module->node.flags & MOD_UNLOAD))
 	    unload_module(wrap->module);
 
-	if (!cont)
+	if (!cont) {
+	    if (ou)
+		zfree(ou, ouu);
 	    return;
+	}
 	wrap = wrap->next;
     }
     startparamscope();
     execode(prog, 1, 0);
-    setunderscore(ou);
+    if (ou) {
+	setunderscore(ou);
+	zfree(ou, ouu);
+    }
     endparamscope();
 }
 
Index: Src/utils.c
===================================================================
RCS file: /cvsroot/zsh/zsh/Src/utils.c,v
retrieving revision 1.212
diff -u -r1.212 utils.c
--- Src/utils.c	3 Mar 2009 05:22:57 -0000	1.212
+++ Src/utils.c	3 Mar 2009 21:54:35 -0000
@@ -1340,9 +1340,13 @@
 		    fprintf(shout, "You have new mail.\n");
 		    fflush(shout);
 		} else {
-		    VARARR(char, usav, underscoreused);
+		    char *usav;
+		    int uusav = underscoreused;
 
-		    memcpy(usav, underscore, underscoreused);
+		    usav = zalloc(underscoreused);
+
+		    if (usav)
+			memcpy(usav, underscore, underscoreused);
 
 		    setunderscore(*s);
 
@@ -1353,7 +1357,10 @@
 			fputc('\n', shout);
 			fflush(shout);
 		    }
-		    setunderscore(usav);
+		    if (usav) {
+			setunderscore(usav);
+			zfree(usav, uusav);
+		    }
 		}
 	    }
 	    if (isset(MAILWARNING) && st.st_atime > st.st_mtime &&


-- 
Peter Stephenson <p.w.stephenson@ntlworld.com>
Web page now at http://homepage.ntlworld.com/p.w.stephenson/

Re: zsh segfaults with lots of data in one variable

[Peter Stephenson]

[This seemed to disappear the first time I sent it but it may be queued
somewhere and sendmail is lying to me.]

On Tue, 3 Mar 2009 22:07:29 +0100
Frank Terbeck <ft@bewatermyfriend.org> wrote:
> I just wanted to see if lines="$(<&0)" plays nice with binary input
> data, when zsh crashed on me.
> 
> What I did is this:
> 
> [snip]
> zsh% slurp() { lines="$(<&0)" }
> zsh% slurp < ./a-big-71MiB-mp3-file.mp3
> zsh% printf '%s' $lines > foo.mp3  

This is down to a combination of implementation issues including (I
suspect, depending on your system) how gcc works.

For every command the shell saves the previous last argument in $_.
This happens after expansion, so in that last line it was the complete
value of "$lines".  (This indicates how inefficient it can be to get the
shell to do things that aren't really it's job...)

When executing a shell function, $_ is saved and restored.  If gcc or
some other compiler that supports variable length arrays is around,
we'll use that.  This memory then goes on the stack.  In the case of the
multimegabyte file this caused bad karma.

valgrind gave me an odd message about the stack being switched around
when this happened, so I can't exclude the possibility that the compiler or
the system is getting unnecessarily muddled.  Someone more au fait with
the internals might want to try this with an unpatched version of the
shell.

However, in our case it's easy to fix just using ordindary malloc memory
for saving $_.  It would be much more efficient to mark underscore as
"push old value to stack on write", or something similar, but if I try
that at ten o'clock in the evening I'll get it wrong.

Index: Src/exec.c
===================================================================
RCS file: /cvsroot/zsh/zsh/Src/exec.c,v
retrieving revision 1.163
diff -u -r1.163 exec.c
--- Src/exec.c	11 Feb 2009 20:42:16 -0000	1.163
+++ Src/exec.c	3 Mar 2009 21:54:34 -0000
@@ -4431,10 +4431,12 @@
 mod_export void
 runshfunc(Eprog prog, FuncWrap wrap, char *name)
 {
-    int cont;
-    VARARR(char, ou, underscoreused);
+    int cont, ouu;
+    char *ou;
 
-    memcpy(ou, underscore, underscoreused);
+    ou = zalloc(ouu = underscoreused);
+    if (ou)
+	memcpy(ou, underscore, underscoreused);
 
     while (wrap) {
 	wrap->module->wrapper++;
@@ -4445,13 +4447,19 @@
 	    (wrap->module->node.flags & MOD_UNLOAD))
 	    unload_module(wrap->module);
 
-	if (!cont)
+	if (!cont) {
+	    if (ou)
+		zfree(ou, ouu);
 	    return;
+	}
 	wrap = wrap->next;
     }
     startparamscope();
     execode(prog, 1, 0);
-    setunderscore(ou);
+    if (ou) {
+	setunderscore(ou);
+	zfree(ou, ouu);
+    }
     endparamscope();
 }
 
Index: Src/utils.c
===================================================================
RCS file: /cvsroot/zsh/zsh/Src/utils.c,v
retrieving revision 1.212
diff -u -r1.212 utils.c
--- Src/utils.c	3 Mar 2009 05:22:57 -0000	1.212
+++ Src/utils.c	3 Mar 2009 21:54:35 -0000
@@ -1340,9 +1340,13 @@
 		    fprintf(shout, "You have new mail.\n");
 		    fflush(shout);
 		} else {
-		    VARARR(char, usav, underscoreused);
+		    char *usav;
+		    int uusav = underscoreused;
 
-		    memcpy(usav, underscore, underscoreused);
+		    usav = zalloc(underscoreused);
+
+		    if (usav)
+			memcpy(usav, underscore, underscoreused);
 
 		    setunderscore(*s);
 
@@ -1353,7 +1357,10 @@
 			fputc('\n', shout);
 			fflush(shout);
 		    }
-		    setunderscore(usav);
+		    if (usav) {
+			setunderscore(usav);
+			zfree(usav, uusav);
+		    }
 		}
 	    }
 	    if (isset(MAILWARNING) && st.st_atime > st.st_mtime &&


-- 
Peter Stephenson <p.w.stephenson@ntlworld.com>
Web page now at http://homepage.ntlworld.com/p.w.stephenson/

Re: zsh segfaults with lots of data in one variable

[Richard Hartmann]

On Tue, Mar 3, 2009 at 23:03, Peter Stephenson
<p.w.stephenson@ntlworld.com> wrote:

> However, in our case it's easy to fix just using ordindary malloc memory
> for saving $_.  It would be much more efficient to mark underscore as
> "push old value to stack on write", or something similar, but if I try
> that at ten o'clock in the evening I'll get it wrong.

Do you want to revisit this at a later time? If yes, I will mark this as bug
and put it into SF's bug tracker after some time (usually you are simply too
fast for me to actually put anything in there ;)


RIchard

Re: zsh segfaults with lots of data in one variable

[Peter Stephenson]

Richard Hartmann wrote:
> On Tue, Mar 3, 2009 at 23:03, Peter Stephenson
> <p.w.stephenson@ntlworld.com> wrote:
> 
> > However, in our case it's easy to fix just using ordindary malloc memory
> > for saving $_. =C2=A0It would be much more efficient to mark underscore as
> > "push old value to stack on write", or something similar, but if I try
> > that at ten o'clock in the evening I'll get it wrong.
> 
> Do you want to revisit this at a later time? If yes, I will mark this as bug
> and put it into SF's bug tracker after some time (usually you are simply too
> fast for me to actually put anything in there ;)

On the whole, I suspect it's not worth it.  The shell is optimised for
moving around strings of a few dozen bytes or so.  To do it better, we'd
need some sort of a stack; that means an additional allocation.  That's
going to add to the complexity in the vast majority of cases just to
improve the occasional case where $_ is some huge string on entry to a
function.   I don't regard this as a bug, anyway, so I think we can
ignore it for now.

Ta.

-- 
Peter Stephenson <pws@csr.com>                  Software Engineer
CSR PLC, Churchill House, Cambridge Business Park, Cowley Road
Cambridge, CB4 0WZ, UK                          Tel: +44 (0)1223 692070