I was experiencing this in Hardy Heron, and it was very annoying, so I
decided to do something about it and try my hand at debugging it. So I
installed the source package, compiled it and ran it through GDB. Sure
enough it segfaulted after changing a few wallpapers. Output was very
similar to the above, but the line that caught my attention was:
#6 0x0804ee7a in f_set_rand_wallpaper (button=0x82b0768, user_data=0x0)
at wp_tray_util.c:371
Looking at line 371 of wp_tray_util.c:
free(sz_randfile);
So it appears that freeing this memory is causing the segfault. That pointer
contains the full path of the randomly selected wallpaper, and the memory gets
allocated in a function (in the same source file) called f_get_dir_entry.
It took me a while to figure it out, because it seemed to be happening
randomly, but eventually I discovered that the
bug is definitely in the f_get_dir_entry function. The problem resides in this
block of code:
// found the target yet?^M
if(*p_dir_trgt == 0)^M
{^M
// malloc a new string^M
*sz_dir_trgt = (gchar *)malloc(100);^M
^M
getcwd(*sz_dir_trgt, 100);^M
^M
// copy the target string^M
strncat(*sz_dir_trgt, "/", 100);^M
strncat(*sz_dir_trgt, entry->d_name, 100);^M
}// end if^M
There are 2 problems with this code that I see. The first is a buffer
overflow, the cause of the segfault: only 100 characters are being
allocated for the file name. That might be fine if it were just the file
name, but this is the full path. So if the full path is longer than 100
characters, there is a problem. The directory I am using is almost 90
characters long by itself!!!Sure enough, it crashed every time the
randomly selected filename was over 100 characters, but worked fine when
the path is under 100 characters. Altering the code above and replacing
100 wherever it occurs with 1024 fixes the problem for me.
The second problem is the way the author is using strncat. Using strncat to try
and prevent a buffer overflow was a good idea, but he is calling it multiple
times. The 3rd parameter to strncat is the max bytes to copy, not the max size
of the buffer. So he allocates 100 bytes, the fills some of those bytes with
the current directory and a trailing slash, and then fills some more with the
actual file name. However, if the path + trailing slash is 99 characters long
(for example), strncat will still happily append up to 100 characters on the
end of the string when he copies the filename. Basically, in the last strncat ,
the max bytes shouldn't be a static value, but should subtract the current
length of the string from the number of allocated bytes.
I'd be interested to know if this fixes the problems for others. I can never
tell if my problem is the exact same as the one other people are describing. To
repeat my fix on your computer (assumes you have build-essentials installed):
apt-get build-dep wallpaper-tray
apt-get source wallpaper-tray
cd wallpaper-tray-0.4.6/src/
Edit the "wp_tray_util.c" file with your favorite text editor. Find
the lines of code I mentioned above (right around line 125), and replace
every "100" with "1024". Save and exit the editor.
Then change to the main source directory ( wallpaper-tray-0.4.6 ), compile:
./configure
make
This won't install the compiled files though. You can run the newly
compiled version of the program from the command line to test it out.
Its under the "src" directory, its called "wp_tray" .
If anyone knows how to to get this information upstream, please do. I
would provide a diff, but I don't have time right now to do it properly
( I made a lot of changes to my source file to print out debugging
messages, so I'd have to redo it all on my end to get a diff with just
the relevant changes.)
I hope this helps somebody, enjoy :)
--
wallpapertray crashes when changing wallpaper
https://bugs.launchpad.net/bugs/95917
You received this bug notification because you are a member of Ubuntu
Bugs, which is subscribed to Ubuntu.
--
ubuntu-bugs mailing list
ubuntu-bugs@lists.ubuntu.com
https://lists.ubuntu.com/mailman/listinfo/ubuntu-bugs
