Logging from signal context is unsafe

[Chase Douglas]

The X internal logging functions use signal-unsafe functions. However, the server is logging messages while in signal context. This can create memory and locking corruption.

[Chase Douglas (chasedouglas)]

The patch is derived from the patch series I submitted upstream for this issue:

http://lists.x.org/archives/xorg-devel/2012-April/030285.html

[Bryce Harrington (bryce)]

+1.

I've reviewed both the upstream proposed patchset, and the debdiff with the patch proposed for precise.

The code changes look sane. The proof is in the pudding though: cnd says with these patches the crashes he sees are eliminated.

Let's get a spot of testing done before this goes in, just to be safe. Would be nice to hear upstream's comments too.

[Bryce Harrington (bryce)]

After bypassing the test error, I've built xserver debs, installed, and
booted on the following:

1. dell mini V. Intel 945 w/ touchpad
    + OK

2. fujitsu t4410. Intel 965 w/ touchscreen
    + Crashed with SIGABRT in raise()
    + Booted OK
    + Apport caught the abort and generated a .crash
    + I didn't notice any crashing behavior myself

3. Dell latitude 13. Arrandale, external monitor attached
    + OK

4. Desktop. Sandybridge on KVM
    + OK

5. Desktop. NVIDIA G92 with proprietary -nvidia driver loaded
    + OK

6. Desktop. ATI Radeon RV730XT with -radeon driver loaded
    + OK

As we've been getting random SIGABRTs reported in ubuntu, I'm not going to call that a regression, although I don't recall seeing it on that particular laptop previously. I'll investigate that further, as a distinct unrelated bug. (At least, it dashes hopes that this bug report's patch would eliminate all the SIGABRT bugs...)

So, going to give the patch an OK on basic boot test.

[Chase Douglas (chasedouglas)]

The patch has been refreshed with a fix for a test case. It has been uploaded to ppa:chasedouglas/jupiter for testing.

[Chase Douglas (chasedouglas)]

This new version of the patch fixes issues in the first version and adds many more logging paths that needed fixing.

The cause of a bug that was thought to be fixed by this patch has been found. As such, the only known effect of this patch is to not crash valgrind when attempting to log in a signal handler. Since this is a large patch to be adding this late in the cycle, I am marking this bug as won't fix for precise. We can add it in Q.

[Bryce Harrington (bryce)]

Has this been taken upstream?

If it has, we'll get it in the upstream xserver when we pull that in. We may also want to pull it in sooner, so we're better able to diagnose crashes.

[Chase Douglas (chasedouglas)]

It is being worked on upstream on xorg-devel. It is moving slowly, but working its way in. When it is accepted upstream we can take a look at any backporting for quantal.

[Timo Aaltonen (tjaalton)]

fixed in quantal & raring