Bug 165270

Created attachment 117508 [details]
backtrace output from xscreensaver

(as the map list in the traceback shows, this is actually xscreensaver-demo,
spawned from xscreensaver to update the preferences file to indicate the last
saver run, when changing from one saver to another)

I first noticed this because I opened the "properties" page from the gnome
xscreensaver button on the top panel, intending to find out what the last hack
displayed had been, and it was showing a GL hack selected that I had disabled a
few days previously and so should not have been running; looking in the output
log I create (by running startx -- -layout huge >X.log 2>&1) showed the
backtrace I attached.

I'm now getting a crash of xscreensaver-demo every time I try to enable or
disable any hack, and the backtrace is very similar (somewhere under
write_init_file).

Created attachment 117569 [details]
Another backtrace

Can you reproduce this under valgrind (and preferably when compiled with -g) so that I can tell where this 
is happening?  I can't reproduce it on my FC4 machine.

It may be a glibc bug. Watch with me as I run the unstripped version under gdb:

Program received signal SIGABRT, Aborted.
0x009c9402 in __kernel_vsyscall ()
(gdb) up
#1  0x00c44858 in raise () from /lib/libc.so.6
(gdb) up
#2  0x00c45fc8 in abort () from /lib/libc.so.6
(gdb) up
#3  0x00c7996a in __libc_message () from /lib/libc.so.6
(gdb) up
#4  0x00cfa3d5 in __chk_fail () from /lib/libc.so.6
(gdb) up
#5  0x00cfaa00 in __realpath_chk () from /lib/libc.so.6
(gdb) up
#6  0x0804e566 in chase_symlinks (file=0x961c860 "/home/bill/.xscreensaver")
    at /usr/include/bits/stdlib.h:35
35          return __realpath_chk (__name, __resolved, __bos (__resolved));

I'll attach the valgrind output anyway, later, when bugzilla stops giving me
"Internal server error" every time I try :)

Created attachment 117669 [details]
valgrind output for xscreensaver-demo

shows same backtrace, and an amusing bit of output from the bowels of Xt

Beats me... I can't tell what's going on from what you attached.  I don't get that on my FC4 machine.

The Xt stuff is typical, though, and almost certainly not the problem.

Created attachment 117899 [details]
backtrace with xscreensaver-/glibc-/gtk2-debuginfo

I get the same on a recent ppc rawhide installation with
xscreensaver-base-4.22-7 and 166299 looks like another duplicate.

It's something in __bowels_of_glibc.  driver/prefs.c`write_init_file calls
ditto`chase_symlinks which calls realpath() and then there's a crash ... I've
just cut out the code in chase_symlinks (except for the "return 0;") so that the
damn thing works, for now.  There's a 2K buffer being passed in, and I know
there aren't any symlinks in the path to .xscreensaver on here.

> /usr/lib/libgtk-x11-2.0.so.0[0x88b706]
> /usr/lib/libgtk-x11-2.0.so.0(gtk_tree_selection_select_path+0xa9)[0x88bca9]
> /usr/lib/libgtk-x11-2.0.so.0(gtk_tree_selection_select_iter+0x71)[0x88bd54]

This might be related to bug 166299. 

Do you still see the problem with xscreensaver-base-4.22-8 ?

Yes (so I've modified the source now).

It fails during the call to realpath().

I'm gonna take a guess: the buffer passed is on the stack, and the "buffer
overflow" check in glibc is just seeing an address above the bottom of the stack
and punting?

This started happening after one of the glibc updates in Rawhide.

xscreensaver-base-4.22-8 still fails.  glibc-2.3.90-9 here, since it looks like
it might be a glibc issue.  I don't know how long it's been broken, sorry.

I think this should be fixed in xscreensaver-base-4.22-9 in tomorrow's rawhide.
 The problem is realpath expects a buffer to be passed to it of size PATH_MAX
and xscreensaver was passing a buffer of size 2048.  since xscreensaver was just
strdup'ing the result anyway, I changed it to pass NULL for realpath which will
cause it to automatically allocate enough space for the file path.

Is that portable behavior for realpath()?  It's not documented in the man page on either FC4 or OSX.  In 
fact, the FC4 man page says:

       EINVAL Either path or resolved_path is NULL. (In libc5 this would just
              cause a segfault.)

I imagine this is a more portable fix:

--- prefs.c	18 Apr 2005 01:05:10 -0000	1.42
+++ prefs.c	25 Aug 2005 02:08:04 -0000
@@ -86,7 +86,14 @@
 # ifdef HAVE_REALPATH
   if (file)
     {
-      char buf [2048];
+# ifndef PATH_MAX
+#  ifdef MAXPATHLEN
+#   define PATH_MAX MAXPATHLEN
+#  else
+#   define PATH_MAX 2048
+#  endif
+# endif
+      char buf[PATH_MAX];
       if (realpath (file, buf))
         return strdup (buf);
 

Hi Jaime,

realpath() is just unportable in general.

Your way is more portable than mine, but some platforms require doing something
like:

long max_path_length = pathconf (file, _PC_PATH_MAX);

to get the maximum path length.  Furthermore, pathconf can return -1 to mean "no
max path limit" or it can return an absurdly high number that you wouldn't want
to pass to malloc.  In either of those two case there is no safely sized buffer
you can pass to realpath.  

Passing NULL is a glibc extension.  I don't know what other c libraries support it.

Using a fallback size at all is risky, but if you're going to use one I'd say
pick 4096 instead of 2048, because 2048 is what caused this bug to be opened in
the first place.

Check for canonicalize_file_name() (it's "equivalent," it says, "to the call
realpath(path, NULL)."  If that exists, problem solved.

Otherwise, if realpath() exists and PATH_MAX is defined, use that.

Else, punt, and skip it (or write one, but that seems a little pointless).

You could call pathconf() and test the result to be sure it's somewhere between
0 and, say, 64K.