Bug 23989 - Segfault when running Xorg --help
Summary: Segfault when running Xorg --help
Status: RESOLVED FIXED
Alias: None
Product: xorg
Classification: Unclassified
Component: Server/General (show other bugs)
Version: unspecified
Hardware: Other All
: medium normal
Assignee: Tiago Vignatti
QA Contact: Xorg Project Team
URL:
Whiteboard:
Keywords:
Depends on:
Blocks: xserver-1.7
  Show dependency treegraph
 
Reported: 2009-09-17 04:15 UTC by Kjartan Maraas
Modified: 2009-09-18 20:29 UTC (History)
1 user (show)

See Also:
i915 platform:
i915 features:


Attachments
0001-xfree86-don-t-clean-up-the-arbiter-if-we-never-initi.patch (1.17 KB, patch)
2009-09-17 21:31 UTC, Peter Hutterer
no flags Details | Splinter Review

Description Kjartan Maraas 2009-09-17 04:15:50 UTC
Program received signal SIGSEGV, Segmentation fault.
0x008ecdbb in pci_device_vgaarb_fini () at common_vgaarb.c:149
149	    close(pci_sys->vgaarb_fd);
Missing separate debuginfos, use: debuginfo-install audit-libs-2.0-3.fc12.i686 hal-libs-0.5.13-7.fc12.i686 keyutils-libs-1.2-6.fc12.i686 krb5-libs-1.7-8.fc12.i686 libXdmcp-1.0.2-11.fc12.i686 libXfont-1.4.0-5.fc12.i686 libattr-2.4.43-4.fc12.i686 libcap-2.16-5.fc12.i686 libcom_err-1.41.9-3.fc12.i686 libfontenc-1.0.5-2.fc12.i686
(gdb) bt
#0  0x008ecdbb in pci_device_vgaarb_fini () at common_vgaarb.c:149
#1  0x080ba9d2 in xf86VGAarbiterFini () at xf86VGAarbiter.c:92
#2  0x080b51a5 in ddxGiveUp () at xf86Init.c:1195
#3  0x080b529e in AbortDDX () at xf86Init.c:1265
#4  0x080ae48e in AbortServer () at log.c:404
#5  0x080aeade in FatalError (f=<value optimized out>) at log.c:529
#6  0x080a99c6 in ProcessCommandLine (argc=<value optimized out>, 
    argv=<value optimized out>) at utils.c:981
#7  0x08062575 in main (argc=<value optimized out>, 
    argv=<value optimized out>, envp=<value optimized out>) at main.c:150
(gdb) bt full
#0  0x008ecdbb in pci_device_vgaarb_fini () at common_vgaarb.c:149
No locals.
#1  0x080ba9d2 in xf86VGAarbiterFini () at xf86VGAarbiter.c:92
No locals.
#2  0x080b51a5 in ddxGiveUp () at xf86Init.c:1195
        i = <value optimized out>
#3  0x080b529e in AbortDDX () at xf86Init.c:1265
        i = <value optimized out>
#4  0x080ae48e in AbortServer () at log.c:404
No locals.
#5  0x080aeade in FatalError (f=<value optimized out>) at log.c:529
        beenhere = 1
#6  0x080a99c6 in ProcessCommandLine (argc=<value optimized out>, 
    argv=<value optimized out>) at utils.c:981
        i = <value optimized out>
        skip = <value optimized out>
#7  0x08062575 in main (argc=<value optimized out>, 
    argv=<value optimized out>, envp=<value optimized out>) at main.c:150
        i = <value optimized out>
        alwaysCheckForInput = {8826048, 8822772}
Comment 1 Julien Cristau 2009-09-17 04:32:47 UTC
This should be fixed by the following commit in libpciaccess, as far as I can tell:
commit 42b879a203c1c16daa9d0c610c6a217ead7a5829
Author: Dave Airlie <airlied@linux.ie>
Date:   Wed Sep 2 19:03:11 2009 +1000

    vgaarb: check pci_sys exists before initing vga arb

Comment 2 Julien Cristau 2009-09-17 04:48:43 UTC
> --- Comment #1 from Julien Cristau <jcristau@debian.org>  2009-09-17 04:32:47 PST ---
> This should be fixed by the following commit in libpciaccess, as far as I can
> tell:

scratch that.  the problem is calling the vgaarb_fini function when
pci_system_init hasn't been called.
Comment 3 Peter Hutterer 2009-09-17 21:31:58 UTC
Created attachment 29652 [details] [review]
0001-xfree86-don-t-clean-up-the-arbiter-if-we-never-initi.patch

something simple like this should do the job.
Comment 4 Tiago Vignatti 2009-09-18 03:00:47 UTC
(In reply to comment #3)
> Created an attachment (id=29652) [details]
> 0001-xfree86-don-t-clean-up-the-arbiter-if-we-never-initi.patch
> 
> something simple like this should do the job.
> 

I just push a patch in the pci library - makes more sense to fix there. Should be fixed now.
Comment 5 Julien Cristau 2009-09-18 03:21:51 UTC
> --- Comment #4 from Tiago Vignatti <tiago.vignatti@nokia.com>  2009-09-18 03:00:47 PST ---
> I just push a patch in the pci library - makes more sense to fix there. Should
> be fixed now.

i'm not sure it makes more sense.  it seems ok to me for libpciaccess to
assume that pci_device_vgaarb_fini won't be called if
pci_device_vgaarb_init wasn't called (and successful).
Comment 6 Tiago Vignatti 2009-09-18 03:56:24 UTC
(In reply to comment #5)
> > --- Comment #4 from Tiago Vignatti <tiago.vignatti@nokia.com>  2009-09-18 03:00:47 PST ---
> > I just push a patch in the pci library - makes more sense to fix there. Should
> > be fixed now.
> 
> i'm not sure it makes more sense.  it seems ok to me for libpciaccess to
> assume that pci_device_vgaarb_fini won't be called if
> pci_device_vgaarb_init wasn't called (and successful).

We usually assume that users (e.g. Xorg or human) can make unpredictable stupid things. So that's why I prefered to protect the library instead the application. 

On the other hand, following your thinking, we would just have to ignore this bug because the reporter was wrongly calling X server's argument, using --help instead -help :)
Comment 7 Julien Cristau 2009-09-18 04:02:21 UTC
On Fri, Sep 18, 2009 at 03:56:24 -0700, bugzilla-daemon@freedesktop.org wrote:

> --- Comment #6 from Tiago Vignatti <tiago.vignatti@nokia.com>  2009-09-18 03:56:24 PST ---
> On the other hand, following your thinking, we would just have to ignore this
> bug because the reporter was wrongly calling X server's argument, using --help
> instead -help :)

i'm not sure what this is following, but certainly not my thinking.
Comment 8 Peter Hutterer 2009-09-18 20:29:51 UTC
(In reply to comment #6)
> We usually assume that users (e.g. Xorg or human) can make unpredictable stupid
> things. So that's why I prefered to protect the library instead the
> application. 

IMO, the correct fix is to make sure we don't do anything stupid (i.e. my patch) and to fail gracefully if it happens nonetheless (libpciaccess commit).
this is the server, not some random user input that we can't validate beforehand.

> On the other hand, following your thinking, we would just have to ignore this
> bug because the reporter was wrongly calling X server's argument, using --help
> instead -help :)

this makes no sense. if you pass in an argument that doesn't parse the server should _not_ segfault. 


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.