Negative Selection devPrivates refcount?

[Andrew Eikum]

Hello,

I ran into an X server crash triggered by an X module I'm writing, and
eventually tracked the crash down to
'global_keys[PRIVATE_SELECTION].created' underflowing in
_dixFiniPrivates.

What seems to happen is:

1) The module registers a new private key with
dixRegisterPrivateKey(PRIVATE_XSELINUX).

2) Some X client (in my test case, fluxbox) creates a new Selection.
This Selection is allocated with _dixAllocateObjectWithPrivates, which
eventually increments global_keys[PRIVATE_SELECTION].created to 1.

3) I choose the 'Restart' option in fluxbox, which causes Dispatch()
in the main loop in <dix/main.c> to exit, and some teardown occurs.

4) On the next iteration of the loop, dixResetPrivates() is called,
which prints some warnings to the log file about various object
privates still being allocated. More importantly, it resets the
Selection privates 'created' variable to 0.

5) InitSelections() is then called, which iterates over all of the
existing Selections, including the one fluxbox created, in order to
delete them. As part of deleting them, it calls
dixFreeObjectWithPrivates(). This decrements that 'created' variable
to -1.

6) The module's init function re-registers its key with
dixRegisterPrivateKey. This triggers the assert
"assert(!global_keys[t].created);". The X server crashes, as created
is -1.

Assuming my analysis is correct and I'm using the module and
devPrivates APIs correctly, this looks like an xserver bug. Probably
the best way to fix would be to clean up the Selections and such
before calling dixResetPrivates(). This would keep the log file clean,
and should resolve this issue by preventing the refcount from going
negative.  To do this, we'd just move the InitSelections() call to
earlier than the dixResetPrivates() call in the main loop. And, in
fact, this is enough to fix the crash for me.

What worries me about this approach is there are other objects left
allocated: 2 pixmaps, 4 GCs, and 1 cursor on my computer. Each of
these can have privates, and could potentially run into the same issue
depending on when they are cleaned up. Since it no longer crashes for
me after fixing the SELECTION issue, I guess it's not an issue, but
I'm not familiar enough with those parts of the code to be confident.

Another simple option for fixing would be refusing to go negative in
_dixFiniPrivates by checking if it's 0 before decrementing.

I've attached a simple X module to this mail which demonstrates the
problem on my computer with xserver-1.14.4. After building and
enabling the module in xorg.conf, I launch the X server, run fluxbox,
select Restart from the fluxbox menu, and the X server crashes.

Thoughts? Shall I go ahead and send such a patch after the 1.15
release in two weeks?

Thanks,
Andrew
-------------- next part --------------
A non-text attachment was scrubbed...
Name: devpriv.c
Type: text/x-csrc
Size: 793 bytes
Desc: not available
URL: <http://lists.x.org/archives/xorg-devel/attachments/20131210/2536686b/attachment.c>