Bug 1695 - XScreenSaverUnsetAttributes crashes xserver
Summary: XScreenSaverUnsetAttributes crashes xserver
Status: RESOLVED FIXED
Alias: None
Product: xorg
Classification: Unclassified
Component: Lib/other (show other bugs)
Version: 6.7.0
Hardware: x86 (IA32) Linux (All)
: low critical
Assignee: Roland Mainz
QA Contact:
URL:
Whiteboard:
Keywords:
Depends on:
Blocks:
 
Reported: 2004-10-23 00:14 UTC by Josip Deanovic
Modified: 2004-12-12 00:43 UTC (History)
2 users (show)

See Also:
i915 platform:
i915 features:


Attachments
1659-crash.patch (485 bytes, patch)
2004-11-04 17:43 UTC, Adam Jackson
no flags Details | Splinter Review
[FIXED_X11R68x] 1695-crash.patch (697 bytes, patch)
2004-11-07 06:41 UTC, Adam Jackson
roland.mainz: 6.8-branch+
Details | Splinter Review
Patch incl. Changelog comment for 2004-12-12-trunk (same as attachment #1230) (3.24 KB, patch)
2004-12-12 19:41 UTC, Roland Mainz
no flags Details | Splinter Review

Description Josip Deanovic 2004-10-23 00:14:41 UTC
To reproduce the crashing of xserver one should:
1. use XScreenSaverSetAttributes() to set attributes for xscreensaver
2. use XScreenSaverUnsetAttributes() to unset previously set attributes
3. use XCloseDisplay to close the connection to display

When program comes to the point where display should be closed, it's 
xserver will exit with signal 11.

Here's the source of xcrash program which should crash X server upon
execution.

/* 
 * Compilation: 
 * gcc -g -O2 -Wall -Wstrict-prototypes -o xcrash xcrash.c \ 
 * -I/usr/X11R6/include -L/usr/X11R6/lib -lX11 -lXext -lXss 
 */ 
 
#include <stdio.h> 
#include <stdlib.h> 
#include <unistd.h> 
#include <getopt.h> 
#include <X11/Xlib.h> 
#include <X11/extensions/scrnsaver.h> 
 
#define PROGNAME "xcrash" 
 
 
int main (int argc, char *argv[]) 
{ 
char *disp = NULL; 
int parameter = 0; 
Display *display; 
Visual *visual; 
Colormap colormap; 
int screen; 
int screen_width; 
int screen_height; 
int depth; 
Window root_window; 
GC gc; 
int event_base; 
int error_base; 
XScreenSaverInfo *ssinfo = NULL; 
XSetWindowAttributes attributes; 
 
 
/* Getting arguments using getopt_long */ 
while (1) 
      { 
      int option_index = 0; 
      static struct option long_options[] = { 
                                            {"display", 1, 0, 'd'}, 
                                            {0, 0, 0, 0} 
                                            }; 
 
      parameter = getopt_long (argc, argv, "d:", long_options, &option_index); 
      if (parameter == -1) 
         break; 
 
      if  (parameter == 'd') 
          disp = optarg; 
      } 
 
if (optind < argc)
   { 
   exit (EXIT_FAILURE); 
   } 
 
 
/* Connecting to a server */ 
if ((display = XOpenDisplay (disp)) == NULL) 
   { 
   fprintf (stderr, "%s: cannot connect to X server %s\n", PROGNAME,
XDisplayName (disp)); 
   exit (EXIT_FAILURE); 
   } 
 
/* Let's use sync mode for debugging purposes. */ 
XSynchronize (display, True); 
 
 
/* Using macros provided by Xlib to obtain info about display */ 
screen = DefaultScreen (display); 
screen_width = DisplayWidth (display, screen); 
screen_height = DisplayHeight (display, screen); 
root_window = DefaultRootWindow (display); 
depth = DefaultDepth (display, screen); 
visual = DefaultVisual (display, screen); 
colormap = XDefaultColormap (display, screen); 
gc = DefaultGC (display, screen); 
 
 
/* Testing the existence of Xscreensaver extension */ 
if (XScreenSaverQueryExtension (display, &event_base, &error_base) == False) 
   { 
   fprintf (stderr, "%s: XScreenSaverQueryExtension - screensaver extension not
supported\n", PROGNAME); 
   exit (EXIT_FAILURE); 
   } 
 
/* Allocating an XScreenSaverInfo structure */ 
if ((ssinfo = XScreenSaverAllocInfo()) == NULL) 
   { 
   fprintf (stderr, "%s: XScreenSaverAllocInfo - insufficient memory\n", PROGNAME); 
   exit (EXIT_FAILURE); 
   } 
 
/* 
 * If some other client already used XScreenSaverSetAttributes we should not 
 * try to do the same. It means that external screensaver will be used and 
 * ssinfo->kind will be 2. 
 */ 
if (XScreenSaverQueryInfo (display, root_window, ssinfo) == 0) 
   { 
   fprintf (stderr, "%s: XScreenSaverQueryInfo - extension not supported\n",
PROGNAME); 
   exit (EXIT_FAILURE); 
   } 
 
if (ssinfo->kind == 2)
   { 
   fprintf (stderr, "%s: some other client has already set screensaver
attributes.\n", PROGNAME); 
   exit (EXIT_FAILURE); 
   } 
 
/* Some window (screensaver) attributes */ 
attributes.background_pixel = BlackPixel (display, screen); 
attributes.border_pixel = BlackPixel (display, screen); 
 
 
/* Normally, we would use window with override_redirect attribute set. */ 
XScreenSaverSetAttributes (display, root_window, 0, 0, screen_width, 
                           screen_height, 0, depth, InputOutput, visual, 
                           CWBackPixel | CWBorderPixel, &attributes); 
 
if (XScreenSaverQueryInfo (display, root_window, ssinfo) == 0) 
   { 
   fprintf (stderr, "%s: XScreenSaverQueryInfo - extension not supported\n",
PROGNAME); 
   exit (EXIT_FAILURE); 
   } 
 
XScreenSaverUnsetAttributes (display, root_window); 
 
fprintf (stderr, "To dye or not to dye... It is not question any more.\n"); 
 
XCloseDisplay (display); 
 
exit (EXIT_SUCCESS); 
}


I have reported this problem to xfree86 team and they have already
solved this problem.
Some friends of mine reported the same problem with xorg-6.7.0 so,
I decided to report this bug to xorg, too.

Thanks in advance.
Comment 1 Adam Jackson 2004-11-04 17:43:47 UTC
Created attachment 1225 [details] [review]
1659-crash.patch

this fixes it for me, should catch any other similar bugs in the xss code.
Comment 2 Adam Jackson 2004-11-07 06:41:37 UTC
Created attachment 1230 [details] [review]
[FIXED_X11R68x] 1695-crash.patch

that one was a naive fix, here's a better one.
Comment 3 Adam Jackson 2004-11-07 06:43:28 UTC
Comment on attachment 1230 [details] [review]
[FIXED_X11R68x] 1695-crash.patch

nominating for 6.8.2.

fixes a bug in the screensaver code that can cause any authenticated connection
to crash the server.
Comment 4 Roland Mainz 2004-11-19 07:30:50 UTC
Comment on attachment 1230 [details] [review]
[FIXED_X11R68x] 1695-crash.patch

Approved for the X11R6.8.x branch in the 2004-11-17 release-wranglers phone
call.
Please don't commit it yourself, I'll handle that once the CVS service is
available again.
Comment 5 Roland Mainz 2004-12-12 19:35:41 UTC
Comment on attachment 1230 [details] [review]
[FIXED_X11R68x] 1695-crash.patch

Patch checked-in into the X11R6.8.x stable branch:

/cvs/xorg/xc/ChangeLog,v  <--  ChangeLog
new revision: 1.365.2.33; previous revision: 1.365.2.32
cvs commit: Using deprecated info format strings.  Convert your scripts to use
the new argument format and remove '1's from your info file format strings.
/cvs/xorg/xc/programs/Xserver/Xext/saver.c,v  <--  saver.c
new revision: 1.2.4.2; previous revision: 1.2.4.1
cvs commit: Using deprecated info format strings.  Convert your scripts to use
the new argument format and remove '1's from your info file format strings.
Mailing the commit message to xorg-commit@lists.freedesktop.org...
Comment 6 Roland Mainz 2004-12-12 19:41:17 UTC
Created attachment 1533 [details] [review]
Patch incl. Changelog comment for 2004-12-12-trunk (same as attachment #1230 [details] [review])
Comment 7 Roland Mainz 2004-12-12 19:43:37 UTC
attachment #1533 [details] [review] checked-in into Xorg trunk:

/cvs/xorg/xc/ChangeLog,v  <--  ChangeLog
new revision: 1.603; previous revision: 1.602
/cvs/xorg/xc/programs/Xserver/Xext/saver.c,v  <--  saver.c
new revision: 1.5; previous revision: 1.4
Mailing the commit message to xorg-commit@lists.freedesktop.org...

Marking bug as FIXED.


Use of freedesktop.org services, including Bugzilla, is subject to our Code of Conduct. How we collect and use information is described in our Privacy Policy.