Bug 3335 - sisusb_driver causes "X -configure" to segfault (problem in xf86MatchDevice)
Summary: sisusb_driver causes "X -configure" to segfault (problem in xf86MatchDevice)
Status: RESOLVED FIXED
Alias: None
Product: xorg
Classification: Unclassified
Component: Server/DDX/Xorg (show other bugs)
Version: 6.8.99.7
Hardware: x86 (IA32) Linux (All)
: high critical
Assignee: Xorg Project Team
QA Contact: Xorg Project Team
URL:
Whiteboard:
Keywords:
Depends on:
Blocks:
 
Reported: 2005-05-19 09:55 UTC by Georgi Georgiev
Modified: 2010-10-01 20:34 UTC (History)
1 user (show)

See Also:
i915 platform:
i915 features:


Attachments

Description Georgi Georgiev 2005-05-19 09:55:24 UTC
Running "X -configure" (xorg 6.8.99.7) aborts with a segfault. Consequently, I
figured out that moving sisusb_driver.o (or .so) out of the way solves the problem.

After compiling a debugging version of xorg with shared libraries, I managed to
properly debug it:

ols-dell xc # gdb X
...
(gdb) run -configure
Starting program: /usr/bin/X -configure
...
Program received signal SIGSEGV, Segmentation fault.
0xb796164e in SISUSBProbe (drv=0x82a7a10, flags=1) at sisusb_driver.c:422
422            if((myminor = SiSUSBFindUSBDongle(devSections[i], minorArray,
numDevSections, &nameptr)) >= 0) {
(gdb) p devSections 
$1 = (GDevPtr *) 0x0

The cause is obvious -- dereferencing a NULL pointer.

Now, after some more debugging:

...
(gdb) break SISUSBProbe 
Breakpoint 1 at 0xb7961561: file sisusb_driver.c, line 361.
(gdb) run -configure
...
Breakpoint 1, SISUSBProbe (drv=0x82a7a10, flags=1) at sisusb_driver.c:361
361         Bool    foundScreen = FALSE;
(gdb) n
386         if((numDevSections = xf86MatchDevice(SISUSB_DRIVER_NAME,
&devSections)) <= 0) {
(gdb) p devSections 
$2 = (GDevPtr *) 0xb798d840
(gdb) step
xf86MatchDevice (drivername=0xb7973868 "sisusb", sectlist=0xbfffcac8) at
xf86Helper.c:1515
1515        GDevPtr       gdp, *pgdp = NULL;
(gdb) n
1519        if (sectlist)
(gdb) 
1520            *sectlist = NULL;
(gdb) 
1522        if (xf86DoProbe) return 1;
(gdb) 
1524        if (xf86DoConfigure && xf86DoConfigurePass1) return 1;
(gdb) p xf86DoConfigure
$3 = 1
(gdb) p xf86DoConfigurePass1 
$4 = 1
(gdb) n
1584    }
(gdb) 
SISUSBProbe (drv=0x82a7a10, flags=1) at sisusb_driver.c:406
406         if(!(minorArray = (int *)xalloc(numDevSections * sizeof(int)))) {
(gdb) p devSections 
$5 = (GDevPtr *) 0x0

# Now, we are back in SISUSBProbe, and xf86MatchDevice has returned 1
# (which is assigned to numDevSections) but it already NULL-ed the devSections
# pointer that was passed to it

(gdb) n
409         for(i = 0; i < numDevSections; i++) minorArray[i] = -1;
(gdb) 
412         if(!(devnameArray = (char **)xalloc(numDevSections * sizeof(char *)))) {
(gdb) 
420         numUsed = 0;
(gdb) 
421         for(i = 0; i < numDevSections; i++) {
(gdb) 
422            if((myminor = SiSUSBFindUSBDongle(devSections[i], minorArray,
numDevSections, &nameptr)) >= 0) {
(gdb) 

Program received signal SIGSEGV, Segmentation fault.
0xb796164e in SISUSBProbe (drv=0x82a7a10, flags=1) at sisusb_driver.c:422
422            if((myminor = SiSUSBFindUSBDongle(devSections[i], minorArray,
numDevSections, &nameptr)) >= 0) {
(gdb)

I don't know much about how xorg works, but from what I see, the xf86MatchDevice
function is a bit confusing:

- it returns 1 as in
    if (xf86DoConfigure && xf86DoConfigurePass1) return 1;
  but *sectlist == NULL at that point. So:
  -- return 1, *sectlist == NULL
- it returns 0 when it finds nothing in the config file, i.e.
  -- return 0, *sectlist == NULL
- it returns 1, if it finds exactly one entry in the config file, i.e.
  -- return 1, *sectlist == { some entry, NULL }
- According to the comments, the function *must* return
  -- 0 if nothing is found
  -- -1 on error
  -- the number of elements in *sectlist otherwise

I am guessing, those "return 1" statements are supposed to be "return -1"
instead? There is no other statement where the function can return -1 otherwise.

This bug is triggered by the sisusb driver, but I think that the driver is not
at fault and therefore I am not assignig it to Driver/SIS.
Comment 1 Thomas Winischhofer 2005-06-21 04:34:41 UTC
I worked-around this now in the sisusb driver by checking the pointer. However,
I assume that this is no real fix. However, I won't touch xf86MatchDevice
because I don't know what other drivers need it and rely on the current behavior.
Comment 2 Daniel Stone 2007-02-27 01:26:45 UTC
Sorry about the phenomenal bug spam, guys.  Adding xorg-team@ to the QA contact so bugs don't get lost in future.
Comment 3 Jesse Adkins 2010-10-01 20:34:54 UTC
(In reply to comment #0)
> 1524        if (xf86DoConfigure && xf86DoConfigurePass1) return 1;
> (gdb) p xf86DoConfigure
> $3 = 1
> (gdb) p xf86DoConfigurePass1 
> $4 = 1

You're right about this being an xserver bug and not a SIS bug. The quoted snippet is the problem. That should have been a -1 instead of 1. All drivers currently check for 0 or negative numbers for failure.

After reading the git log, I found that this issue was fixed by Tiago Vignatti on May 20 2010 by git commit 0ceac6f64f5ad9bc2ac4b19be2dd245ffba78b05.


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.