xwininfo segfaults if given a non-existent screen with -display option

Bug #1028274 reported by David Venz
10
This bug affects 1 person
Affects Status Importance Assigned to Milestone
X11 Utils
Fix Released
Medium
x11-utils (Fedora)
Won't Fix
Undecided
x11-utils (Ubuntu)
Fix Released
High
Unassigned
Precise
Fix Released
High
Unassigned
Quantal
Fix Released
High
Unassigned

Bug Description

[Impact]
xwininfo segfaults if called with an out-of-range display 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

[Fix]
The proposed patch works as is and is proposed for both precise and quantal.

[Regression Potential]
The patch is small and only affects an error condition check, so the chance of a regression seems very small. xwininfo is not widely used, so the impact of a regression would be minimal as well.

[Original Report]
I have a dual-head setup that only presents one "screen" to X (subdivided by xinerama).

If I run xwininfo -root -display :0.0 it works as expected, giving the root window information for my single "screen".
If I run xwininfo -root -display :0.1 it segfaults.
I expect it to exit gracefully with an error message. 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.

ProblemType: Bug
DistroRelease: Ubuntu 12.04
Package: x11-utils 7.6+4
ProcVersionSignature: Ubuntu 3.2.0-27.43-generic 3.2.21
Uname: Linux 3.2.0-27-generic x86_64
NonfreeKernelModules: nvidia
.proc.driver.nvidia.gpus.0: Error: [Errno 21] Is a directory: '/proc/driver/nvidia/gpus/0'
.proc.driver.nvidia.registry: Binary: ""
.proc.driver.nvidia.version:
 NVRM version: NVIDIA UNIX x86_64 Kernel Module 295.49 Mon Apr 30 23:46:33 PDT 2012
 GCC version: gcc version 4.6.3 (Ubuntu/Linaro 4.6.3-1ubuntu5)
.tmp.unity.support.test.0:

ApportVersion: 2.0.1-0ubuntu11
Architecture: amd64
CompizPlugins: [core,composite,opengl,compiztoolbox,decor,vpswitch,snap,mousepoll,resize,place,move,wall,grid,regex,imgpng,session,gnomecompat,animation,fade,unitymtgrabhandles,workarounds,scale,expo,ezoom,unityshell]
CompositorRunning: None
Date: Tue Jul 24 16:37:34 2012
DistUpgraded: Fresh install
DistroCodename: precise
DistroVariant: ubuntu
DkmsStatus:
 nvidia-current, 295.40, 3.2.0-23-generic, x86_64: installed
 nvidia-current, 295.40, 3.2.0-26-generic, x86_64: installed
 nvidia-current, 295.40, 3.2.0-27-generic, x86_64: installed
 nvidia-current-updates, 295.49, 3.2.0-27-generic, x86_64: installed
GraphicsCard:
 NVIDIA Corporation G98 [Quadro NVS 295] [10de:06fd] (rev a1) (prog-if 00 [VGA controller])
   Subsystem: NVIDIA Corporation Device [10de:062e]
InstallationMedia: Ubuntu 12.04 LTS "Precise Pangolin" - Release amd64 (20120425)
JockeyStatus:
 xorg:nvidia_current - NVIDIA accelerated graphics driver (Proprietary, Disabled, Not in use)
 xorg:nvidia_current_updates - NVIDIA accelerated graphics driver (post-release updates) (Proprietary, Enabled, In use)
MachineType: Dell Inc. Precision WorkStation T3500
ProcEnviron:
 LANGUAGE=en_AU:en
 TERM=xterm
 PATH=(custom, user)
 LANG=en_AU.UTF-8
 SHELL=/bin/bash
ProcKernelCmdLine: BOOT_IMAGE=/boot/vmlinuz-3.2.0-27-generic root=UUID=2769af30-df94-44f1-9846-7241763e2193 ro quiet splash vt.handoff=7
SourcePackage: x11-utils
UpgradeStatus: No upgrade log present (probably fresh install)
dmi.bios.date: 04/12/2010
dmi.bios.vendor: Dell Inc.
dmi.bios.version: A07
dmi.board.name: 09KPNV
dmi.board.vendor: Dell Inc.
dmi.board.version: A00
dmi.chassis.type: 7
dmi.chassis.vendor: Dell Inc.
dmi.modalias: dmi:bvnDellInc.:bvrA07:bd04/12/2010:svnDellInc.:pnPrecisionWorkStationT3500:pvr:rvnDellInc.:rn09KPNV:rvrA00:cvnDellInc.:ct7:cvr:
dmi.product.name: Precision WorkStation T3500
dmi.sys.vendor: Dell Inc.
version.compiz: compiz 1:0.9.7.8-0ubuntu1.2
version.ia32-libs: ia32-libs 20090808ubuntu36
version.libdrm2: libdrm2 2.4.32-1ubuntu1
version.libgl1-mesa-dri: libgl1-mesa-dri 8.0.2-0ubuntu3.1
version.libgl1-mesa-dri-experimental: libgl1-mesa-dri-experimental N/A
version.libgl1-mesa-glx: libgl1-mesa-glx 8.0.2-0ubuntu3.1
version.nvidia-graphics-drivers: nvidia-graphics-drivers N/A
version.xserver-xorg-core: xserver-xorg-core 2:1.11.4-0ubuntu10.6
version.xserver-xorg-input-evdev: xserver-xorg-input-evdev 1:2.7.0-0ubuntu1.2
version.xserver-xorg-video-ati: xserver-xorg-video-ati 1:6.14.99~git20111219.aacbd629-0ubuntu2
version.xserver-xorg-video-intel: xserver-xorg-video-intel 2:2.17.0-1ubuntu4
version.xserver-xorg-video-nouveau: xserver-xorg-video-nouveau 1:0.0.16+git20111201+b5534a1-1build2

Revision history for this message
David Venz (david-venz) wrote :
Revision history for this message
Ubuntu Foundations Team Bug Bot (crichton) wrote :

The attachment "Patch against x11-utils-7.6+4" of this bug report has been identified as being a patch. The ubuntu-reviewers team has been subscribed to the bug report so that they can review the patch. In the event that this is in fact not a patch you can resolve this situation by removing the tag 'patch' from the bug report and editing the attachment so that it is not flagged as a patch. Additionally, if you are member of the ubuntu-reviewers team please also unsubscribe the team from this bug report.

[This is an automated message performed by a Launchpad user owned by Brian Murray. Please contact him regarding any issues with the action taken in this bug report.]

Bryce Harrington (bryce)
Changed in x11-utils (Ubuntu):
status: New → Triaged
importance: Undecided → High
Bryce Harrington (bryce)
Changed in x11-utils (Ubuntu Precise):
status: New → Triaged
importance: Undecided → High
Revision history for this message
Bryce Harrington (bryce) wrote :

I've verified the bug on precise. Probably affects quantal too. The suggested patch looks sensible enough, and I can confirm it does make it stop segfaulting.

Revision history for this message
Launchpad Janitor (janitor) wrote :

This bug was fixed in the package x11-utils - 7.7~1ubuntu1

---------------
x11-utils (7.7~1ubuntu1) quantal; urgency=low

  * Add 100-xwininfo-bad-screen.patch: Fix wininfo segfault when called
    with out-of-range display number.
    (LP: #1028274)
 -- Bryce Harrington <email address hidden> Mon, 06 Aug 2012 10:12:11 -0700

Changed in x11-utils (Ubuntu Quantal):
status: Triaged → Fix Released
Bryce Harrington (bryce)
description: updated
Revision history for this message
Bryce Harrington (bryce) wrote :

David, finally, thanks for contributing this patch to Ubuntu. I don't know how widely used xwininfo is, but it's always good to make things not crash!

Could I ask you to do one last thing. Please send your patch to the upstream developers. You can do this either by filing a bug report at bugs.freedesktop.org, or by posting it to the xorg-devel@ mailing list. If you submit it to the mailing list, there are some directions on how to format the patch so it can be picked up: http://wiki.x.org/wiki/Development/Documentation/SubmittingPatches

Revision history for this message
David Venz (david-venz) wrote :

No probs, I'll look into an upstream submit.
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? I guess I should let upstream decide.

What's the likelihood of an SRU (or whatever the correct term is) to precise? Is there anything I should do to ensure that happens?

Revision history for this message
In , David Venz (david-venz) wrote :

Created attachment 65279
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?

Changed in x11-utils:
importance: Unknown → Medium
status: Unknown → Confirmed
Revision history for this message
In , Jeremy Sequoia (jeremyhu) wrote :

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)

Revision history for this message
In , Alan Coopersmith (alan-coopersmith) wrote :

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

Revision history for this message
In , Jeremy Sequoia (jeremyhu) wrote :

Created attachment 66113
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.

Revision history for this message
In , Jeremy Sequoia (jeremyhu) wrote :

Created attachment 66114
libxcb patch, introducing XCB_CONN_CLOSED_INVALID_SCREEN

Addresses a couple issues in the original patch

Revision history for this message
In , Alan Coopersmith (alan-coopersmith) wrote :

Created attachment 66115
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.)

Revision history for this message
Chris Halse Rogers (raof) wrote : Please test proposed package

Hello David, or anyone else affected,

Accepted x11-utils into precise-proposed. The package will build now and be available at http://launchpad.net/ubuntu/+source/x11-utils/7.6+4ubuntu0.1 in a few hours, and then in the -proposed repository.

Please help us by testing this new package. See https://wiki.ubuntu.com/Testing/EnableProposed for documentation how to enable and use -proposed. Your feedback will aid us getting this update out to other Ubuntu users.

If this package fixes the bug for you, please change the bug tag from verification-needed to verification-done. If it does not, change the tag to verification-failed. In either case, details of your testing will help us make a better decision.

Further information regarding the verification process can be found at https://wiki.ubuntu.com/QATeam/PerformingSRUVerification . Thank you in advance!

Changed in x11-utils (Ubuntu Precise):
status: Triaged → Fix Committed
tags: added: verification-needed
Changed in x11-utils:
status: Confirmed → In Progress
Revision history for this message
In , Alan Coopersmith (alan-coopersmith) wrote :

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

Changed in x11-utils:
status: In Progress → Fix Released
Revision history for this message
Bartosz Kosiorek (gang65) wrote :

After install package from proposed, there is no longer segfault.

After set out-of-range display number it just displays:

xwininfo -root -display :0.1
xwininfo: error: unable to access screen 1, max is 0

Verification done.

Changed in x11-utils (Ubuntu Precise):
assignee: nobody → Bartosz Kosiorek (gang65)
tags: added: verification-done
removed: verification-needed
Revision history for this message
Colin Watson (cjwatson) wrote : Update Released

The verification of this Stable Release Update has completed successfully and the package has now been released to -updates. Subsequently, the Ubuntu Stable Release Updates Team is being unsubscribed and will not receive messages about this bug report. In the event that you encounter a regression using the package from -updates please report a new bug using ubuntu-bug and tag the bug report regression-update so we can easily find any regresssions.

Changed in x11-utils (Ubuntu Precise):
assignee: Bartosz Kosiorek (gang65) → nobody
Revision history for this message
Launchpad Janitor (janitor) wrote :

This bug was fixed in the package x11-utils - 7.6+4ubuntu0.1

---------------
x11-utils (7.6+4ubuntu0.1) precise-proposed; urgency=low

  * Add 100-xwininfo-bad-screen.patch: Fix wininfo segfault when called
    with out-of-range display number.
    (LP: #1028274)
 -- Bryce Harrington <email address hidden> Mon, 06 Aug 2012 10:12:11 -0700

Changed in x11-utils (Ubuntu Precise):
status: Fix Committed → Fix Released
Changed in x11-utils (Fedora):
importance: Unknown → Undecided
status: Unknown → Won't Fix
To post a comment you must log in.
This report contains Public information  
Everyone can see this information.

Other bug subscribers

Remote bug watches

Bug watches keep track of this bug in other bug trackers.