Segfault on really long paths

Segfault on really long paths

[Richard Hartmann]

Hi all,

richih@adamantium ~ % for i in {1..1000}; do mkdir 0123456789; cd
0123456789; done
[1]    24398 segmentation fault  zsh
richih@adamantium ~ %

It would be better to throw an error than to segfault.


Richard

Re: Segfault on really long paths

[Andrey Borzenkov]

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

On Вторник 30 декабря 2008 03:19:51 Richard Hartmann wrote:
> Hi all,
>
> richih@adamantium ~ % for i in {1..1000}; do mkdir 0123456789; cd
> 0123456789; done
> [1]    24398 segmentation fault  zsh
> richih@adamantium ~ %
>
> It would be better to throw an error than to segfault.
>

Does not do here (Mandriva cooker):

cooker% /bin/pwd > /tmp/foo
cooker% wc /tmp/foo
    1     1 11018 /tmp/foo
cooker% echo $ZSH_VERSION
4.3.6-dev-0

glibc-2.8-1.20080520.5mnb2

Are you sure it is zsh and not (g)libc segfaulting?

[-- Attachment #2: This is a digitally signed message part. --]
[-- Type: application/pgp-signature, Size: 197 bytes --]

Re: Segfault on really long paths

[Michael Prokop]

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

* Andrey Borzenkov <arvidjaar@newmail.ru> [20081230 05:06]:
> On Вторник 30 декабря 2008 03:19:51 Richard Hartmann wrote:

> > richih@adamantium ~ % for i in {1..1000}; do mkdir 0123456789; cd
> > 0123456789; done
> > [1]    24398 segmentation fault  zsh
> > richih@adamantium ~ %

> > It would be better to throw an error than to segfault.

> Does not do here (Mandriva cooker):

> cooker% /bin/pwd > /tmp/foo
> cooker% wc /tmp/foo
>     1     1 11018 /tmp/foo
> cooker% echo $ZSH_VERSION
> 4.3.6-dev-0

> glibc-2.8-1.20080520.5mnb2

> Are you sure it is zsh and not (g)libc segfaulting?

At least Bash (3.2.39) doesn't segfault. I can see several:

| cd: error retrieving current directory: getcwd: cannot access parent directories: File name too long

messages while running but bash doesn't segfault.

Zsh (version 4.3.6 here) receives several EINVAL on readlink()
(using libc 2.7 of Debian/unstable). Then I can see lots of
(uncatched?) ENAMETOOLONG (not mentioning chdir() and stat64())
which seem to cause the segfault in zsh.

regards,
-mika-
-- 
 ,'"`.         http://michael-prokop.at/
(  grml.org -» Linux Live-CD for texttool-users and sysadmins
 `._,'         http://grml.org/

[-- Attachment #2: Digital signature --]
[-- Type: application/pgp-signature, Size: 189 bytes --]

Re: Segfault on really long paths

[Peter Stephenson]

On Tue, 30 Dec 2008 01:19:51 +0100
"Richard Hartmann" <richih.mailinglist@gmail.com> wrote:
> richih@adamantium ~ % for i in {1..1000}; do mkdir 0123456789; cd
> 0123456789; done
> [1]    24398 segmentation fault  zsh
> richih@adamantium ~ %
> 
> It would be better to throw an error than to segfault.

This is one major culprit, there may be others.  If we've *really* run
out of memory there are far too many cases to trap, but there was
an arbitrary limit here that I really don't like.  This is reasonably well
optimized in the case of no metafied characters (unless I've screwed up).

Index: Src/utils.c
===================================================================
RCS file: /cvsroot/zsh/zsh/Src/utils.c,v
retrieving revision 1.207
diff -u -r1.207 utils.c
--- Src/utils.c	27 Nov 2008 17:05:42 -0000	1.207
+++ Src/utils.c	5 Jan 2009 21:50:46 -0000
@@ -3704,26 +3704,67 @@
     return mlen;
 }
 
-/* This function converts a zsh internal string to a form which can be *
- * passed to a system call as a filename.  The result is stored in a   *
- * single static area.  NULL returned if the result is longer than     *
- * 4 * PATH_MAX.                                                       */
+/*
+ * This function converts a zsh internal string to a form which can be
+ * passed to a system call as a filename.  The result is stored in a
+ * single static area, sized to fit.  If there is no Meta character
+ * the original string is returned.
+ */
 
 /**/
 mod_export char *
 unmeta(const char *file_name)
 {
-    static char fn[4 * PATH_MAX];
+    static char *fn;
+    static int sz;
     char *p;
     const char *t;
+    int newsz, meta;
+    
+    meta = 0;
+    for (t = file_name; *t; t++) {
+	if (*t == Meta)
+	    meta = 1;
+    }
+    if (!meta) {
+	/*
+	 * don't need allocation... free if it's long, see below
+	 */
+	if (sz > 4 * PATH_MAX) {
+	    zfree(fn, sz);
+	    fn = NULL;
+	    sz = 0;
+	}
+	return (char *) file_name;
+    }
+
+    newsz = (t - file_name) + 1;
+    /*
+     * Optimisation: don't resize if we don't have to.
+     * We need a new allocation if
+     * - nothing was allocated before
+     * - the new string is larger than the old one
+     * - the old string was larger than an arbitrary limit but the
+     *   new string isn't so that we free up significant space by resizing.
+     */
+    if (!fn || newsz > sz || (sz > 4 * PATH_MAX && newsz <= 4 * PATH_MAX))
+    {
+	if (fn)
+	    zfree(fn, sz);
+	sz = newsz;
+	fn = (char *)zalloc(sz);
+	if (!fn) {
+	    sz = 0;
+	    /*
+	     * will quite likely crash in the caller anyway...
+	     */
+	    return NULL;
+	}
+    }
 
-    for (t = file_name, p = fn; *t && p < fn + 4 * PATH_MAX - 1; p++)
+    for (t = file_name, p = fn; *t; p++)
 	if ((*p = *t++) == Meta)
 	    *p = *t++ ^ 32;
-    if (*t)
-	return NULL;
-    if (p - fn == t - file_name)
-	return (char *) file_name;
     *p = '\0';
     return fn;
 }


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