wcmConfig.c use after free segmentation fault causing X server to crash

[omcaif]

Hi.

I think in https://github.com/linuxwacom/xf86-input-wacom/blob/f4241514b725377e03ad55b4dbe8750cd3e4e5f4/src/wcmConfig.c#L416 the condition is reversed. I am currently getting a segmentation fault and a crash as soon as I connect a Wacom tablet, but by inverting that condition (i.e., change to != 0), I no longer get the crash.

Here is a gdb session showing the use after free:

$ gdb /usr/bin/X
GNU gdb (Gentoo 14.2 vanilla) 14.2
Copyright (C) 2023 Free Software Foundation, Inc.
License GPLv3+: GNU GPL version 3 or later <http://gnu.org/licenses/gpl.html>
This is free software: you are free to change and redistribute it.
There is NO WARRANTY, to the extent permitted by law.
Type "show copying" and "show warranty" for details.
This GDB was configured as "x86_64-pc-linux-musl".
Type "show configuration" for configuration details.
For bug reporting instructions, please see:
<https://bugs.gentoo.org/>.
Find the GDB manual and other documentation resources online at:
    <http://www.gnu.org/software/gdb/documentation/>.

For help, type "help".
Type "apropos word" to search for commands related to "word"...
Reading symbols from /usr/bin/X...
Reading symbols from /usr/lib/debug//usr/bin/Xorg.debug...
(gdb) target remote localhost:2000
Remote debugging using localhost:2000
Reading /lib/ld-musl-x86_64.so.1 from remote target...
warning: File transfers from remote targets can be slow. Use "set sysroot" to access files locally instead.
Reading /lib/ld-musl-x86_64.so.1 from remote target...
Reading symbols from target:/lib/ld-musl-x86_64.so.1...
(No debugging symbols found in target:/lib/ld-musl-x86_64.so.1)
Reading /usr/lib/debug/.build-id/76/48ff7a4483eab52ba1460dc115d98b337c8238.debug from remote target...
0x00007ffff7fc44da in _dlstart () from target:/lib/ld-musl-x86_64.so.1
(gdb) set sysroot
Reading symbols from /lib/ld-musl-x86_64.so.1...
(No debugging symbols found in /lib/ld-musl-x86_64.so.1)
(gdb) b wcmConfig.c:983
No source file named wcmConfig.c.
Breakpoint 1 (wcmConfig.c:983) pending.
(gdb) b wcmConfig.c:416
No source file named wcmConfig.c.
Breakpoint 2 (wcmConfig.c:416) pending.
(gdb) b wcmConfig.c:388
No source file named wcmConfig.c.
Breakpoint 3 (wcmConfig.c:388) pending.
(gdb) b wcmConfig.c:627
No source file named wcmConfig.c.
Breakpoint 4 (wcmConfig.c:627) pending.
(gdb) c
Continuing.
[Detaching after fork from child process 2844]
[Detaching after fork from child process 2845]
[New Thread 2837.2838]
[New Thread 2837.2839]
[New Thread 2837.2840]
[New Thread 2837.2841]
[New Thread 2837.2842]
[New Thread 2837.2843]

Thread 1 "X" hit Breakpoint 1, wcmPreInit (priv=0x7fffebf28260) at ../xf86-input-wacom-1.2.1/src/wcmConfig.c:983
983             if (!wcmMatchDevice(priv, &common))
(gdb) c
Continuing.

Thread 1 "X" hit Breakpoint 2, wcmMatchDevice (priv=0x7fffebf28260, common_return=0x7fffffffd7a0)
    at ../xf86-input-wacom-1.2.1/src/wcmConfig.c:416
416             if (wcmForeachDevice(priv, matchDevice, priv) == 0)
(gdb) c
Continuing.

Thread 1 "X" hit Breakpoint 4, wcmInitialToolSize (priv=0x7fffffffda90)
    at ../xf86-input-wacom-1.2.1/src/wcmConfig.c:629
629     {
(gdb) c
Continuing.
[New Thread 2837.2846]

Thread 1 "X" hit Breakpoint 1, wcmPreInit (priv=0x7fffec1d52b0) at ../xf86-input-wacom-1.2.1/src/wcmConfig.c:983
983             if (!wcmMatchDevice(priv, &common))
(gdb) p priv
$1 = (WacomDevicePtr) 0x7fffec1d52b0
(gdb) p common
$2 = (WacomCommonPtr) 0x0
(gdb) c
Continuing.

Thread 1 "X" hit Breakpoint 2, wcmMatchDevice (priv=0x7fffec1d52b0, common_return=0x7fffffffd920)
    at ../xf86-input-wacom-1.2.1/src/wcmConfig.c:416
416             if (wcmForeachDevice(priv, matchDevice, priv) == 0)
(gdb) p priv->common
$3 = (WacomCommonPtr) 0x7fffebe5b2c0
(gdb) p *common_return
$4 = (WacomCommonPtr) 0x7fffebe5b2c0
(gdb) c
Continuing.

Thread 1 "X" hit Breakpoint 3, matchDevice (privMatch=0x7fffebf28260, data=0x7fffec1d52b0)
    at ../xf86-input-wacom-1.2.1/src/wcmConfig.c:388
388             wcmFreeCommon(&priv->common);
(gdb) p priv->common
$5 = (WacomCommonPtr) 0x7fffebe5b2c0
(gdb) n
389             priv->common = wcmRefCommon(privMatch->common);
(gdb)
390             priv->next = priv->common->wcmDevices;
(gdb) p priv->common
$6 = (WacomCommonPtr) 0x7fffebf1e1e0
(gdb) n
391             priv->common->wcmDevices = priv;
(gdb)
393             return 0;
(gdb)
394     }
(gdb)
wcmForeachDevice (priv=0x7fffec1d52b0, func=0x7fffebf3c555 <matchDevice>, data=0x7fffec1d52b0)
    at ../xf86-input-wacom-1.2.1/src/x11/xf86Wacom.c:623
623                     if (rc == -ENODEV)
(gdb)
625                     if (rc < 0)
(gdb)
627                     nmatch += 1; /* zero counts as matched */
(gdb)
628                     if (rc == 0)
(gdb)
629                             break;
(gdb)
632             return nmatch;
(gdb)
633     }
(gdb)
wcmMatchDevice (priv=0x7fffec1d52b0, common_return=0x7fffffffd920)
    at ../xf86-input-wacom-1.2.1/src/wcmConfig.c:418
418             return 0;
(gdb) p *common_return
$7 = (WacomCommonPtr) 0x7fffebe5b2c0
(gdb) p **common_return
Cannot access memory at address 0x7fffebe5b2c0
(gdb) c
Continuing.

Thread 1 "X" received signal SIGSEGV, Segmentation fault.
wcmPreInit (priv=0x7fffec1d52b0) at ../xf86-input-wacom-1.2.1/src/wcmConfig.c:987
987             common->debugLevel = wcmOptGetInt(priv, "CommonDBG", common->debugLevel);
(gdb) c
Continuing.

Thread 1 "X" received signal SIGABRT, Aborted.
0x00007ffff7fa4cc7 in ?? () from /lib/ld-musl-x86_64.so.1
(gdb) c
Continuing.

Program terminated with signal SIGABRT, Aborted.
The program no longer exists.
(

Basically, in wcmConfig.c:388, there is a free, and then the number of matches is incremented in x11/xf86Wacom.c:627, which indicates there has been a replacement, and then on the outside, wcmConfig.c:416 is supposed to check for non-zero which indicates replacement, but it currently checks for 0...

[whot]

Nice find, thanks. Should be fixed with #343, can you give that one a test please? Thanks

[omcaif]

I just tested that patch and it seems to work.

[whot]

nice, thanks!