Bug 53242 - xwininfo segfaults on invalid screen
Summary: xwininfo segfaults on invalid screen
Status: RESOLVED FIXED
Alias: None
Product: xorg
Classification: Unclassified
Component: App/xwininfo (show other bugs)
Version: 7.6 (2010.12)
Hardware: x86-64 (AMD64) Linux (All)
: medium normal
Assignee: Xorg Project Team
QA Contact: Xorg Project Team
URL:
Whiteboard:
Keywords: patch
Depends on:
Blocks:
 
Reported: 2012-08-08 12:26 UTC by david.venz
Modified: 2018-06-17 20:01 UTC (History)
2 users (show)

See Also:
i915 platform:
i915 features:


Attachments
Workaround - Patch against Ubuntu 12.04 source (7.6) (682 bytes, text/plain)
2012-08-08 12:26 UTC, david.venz
no flags Details
0001-Return-connection-failure-if-display-string-specifie.patch (2.13 KB, patch)
2012-08-25 19:37 UTC, Jeremy Huddleston Sequoia
no flags Details | Splinter Review
libxcb patch, introducing XCB_CONN_CLOSED_INVALID_SCREEN (2.13 KB, patch)
2012-08-25 19:50 UTC, Jeremy Huddleston Sequoia
no flags Details | Splinter Review
Yet another revision of libxcb patch (2.15 KB, patch)
2012-08-25 19:59 UTC, Alan Coopersmith
no flags Details | Splinter Review

Description david.venz 2012-08-08 12:26:42 UTC
Created attachment 65279 [details]
Workaround - Patch against Ubuntu 12.04 source (7.6)

This is https://bugs.launchpad.net/ubuntu/+source/x11-utils/+bug/1028274 and https://bugzilla.redhat.com/show_bug.cgi?id=808561

[Impact]
xwininfo segfaults if called with an out-of-range screen number.

[Test Case]
1. xwininfo -root -display :0.0
2. xwininfo -root -display :0.1

Actual Behavior:
 * First command passes
 * Second command segfaults
Expected Behavior:
 * First command passes
 * Second command returns an error message without segfaulting

---------------

On Ubuntu 8.04 this error message used to be "cannot open display :0.1".

The problem could lie in libxcb (I'm no expert) xcb_connection_has_error() no longer returning true in the -display :0.1 case, but I've attached a patch for xwininfo anyway (sorry about the downstream format etc, not ready to jump into git).  My lingering concern is over how many other bits of code use libxcb in this way, and may have had their assumptions broken.  I.e. is the "right" place to fix this problem longer term in libxcb?
Comment 1 Jeremy Huddleston Sequoia 2012-08-25 19:12:29 UTC
With current master (5037f79e8f6a36d3c524a2dd8150cf96c31b7106):

Program received signal EXC_BAD_ACCESS, Could not access memory.
Reason: KERN_INVALID_ADDRESS at address: 0x0000000000000001
0x00000001000016e1 in Select_Window (dpy=0x10081bc00, screen=0x100822bd4, descend=1) at dsimple.c:180
180	    if (grab_reply->status != XCB_GRAB_STATUS_SUCCESS)
(gdb) bt
#0  0x00000001000016e1 in Select_Window (dpy=0x10081bc00, screen=0x100822bd4, descend=1) at dsimple.c:180
#1  0x00000001000031bf in main (argc=4, argv=0x7fff5fbff950) at xwininfo.c:573
Current language:  auto; currently minimal
(gdb) print grab_reply
$1 = (xcb_grab_pointer_reply_t *) 0x0
(gdb)
Comment 2 Alan Coopersmith 2012-08-25 19:16:20 UTC
Took the question of where to check, libxcb or callers like xwininfo,
to the xcb mailing list for discussion:
http://lists.freedesktop.org/archives/xcb/2012-August/007840.html
Comment 3 Jeremy Huddleston Sequoia 2012-08-25 19:37:25 UTC
Created attachment 66113 [details] [review]
0001-Return-connection-failure-if-display-string-specifie.patch

Here's a patch for libxcb based on alanc's libxcb patch, but it introduces a new error code to allow callers to tell the difference between the two errors.
Comment 4 Jeremy Huddleston Sequoia 2012-08-25 19:50:49 UTC
Created attachment 66114 [details] [review]
libxcb patch, introducing XCB_CONN_CLOSED_INVALID_SCREEN

Addresses a couple issues in the original patch
Comment 5 Alan Coopersmith 2012-08-25 19:59:06 UTC
Created attachment 66115 [details] [review]
Yet another revision of libxcb patch

Merged my fixes with Jeremy's improvements to the previous libxcb patches.
Yay for remote pair programming via bugzilla/irc!  (Sorry for the rapid
fire noise for everyone else.)
Comment 6 Alan Coopersmith 2012-08-30 05:05:26 UTC
Fixes pushed to git master for next releases:

To ssh://git.freedesktop.org/git/xorg/app/xwininfo
   53564df..aedc2ec  master -> master

To ssh://git.freedesktop.org/git/xcb/libxcb
   ed93a6a..ff53285  master -> master


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.