Bug 4248

As per Keith and Daniel suggestions, I confirm that running Keith' transparent
xterm or fdclock -t both kill the server right away. So it must have something
to do with using that ARGB visual.

Created attachment 3170 [details] [review]
Fix crash on cmap alloc failure and fix failure with 32 bits visual

This patch fixes the bug, which was in fact two bugs. Here's a copy of the
explanation I posted to the xorg mailing list:
I found why using the 32 bits ARGB visual cause an immediate server
crash on ppc. In fact, the bug is not ppc specific, but we trigger some
undefined result at some point. There are actually 2 different bugs
going on:

 1) The actual cause of the crash is a problem with failure handling in
CMapCreateColormap(). It doesn't initialize the cmap private ptr to NULL
first, thus if any failure happens when calling down the chain or
calling CMapAllocateColormapPrivate(), we will have a dangling value in
there. Later on, due to the failure, CMapDestroyColormap() will be
called, which will try to use that dangling value and crash. The fix is
to initialize the private pointer to NULL:


 static Bool 
 CMapCreateColormap (ColormapPtr pmap)
 {
     ScreenPtr pScreen = pmap->pScreen;
     CMapScreenPtr pScreenPriv = 
	 (CMapScreenPtr)pScreen->devPrivates[CMapScreenIndex].ptr;
     Bool ret = FALSE;

     pScreen->CreateColormap = pScreenPriv->CreateColormap;
+    pmap->devPrivates[CMapColormapIndex].ptr = NULL;
     if((*pScreen->CreateColormap)(pmap)) { 
	 if(CMapAllocateColormapPrivate(pmap)) 
	    ret = TRUE;
     }
     pScreen->CreateColormap = CMapCreateColormap;

     return ret;
 }


 2) The above turns the crash into a simple failure of the application.
Now the reason why it fails is because CMapAllocateColormapPrivate() fails.
The reason why this later fails is because it uses a construct (which is
used all over the xf86cmap.c code) that looks like:

    if((1 << pmap->pVisual->nplanes) > pScreenPriv->maxColors)
	numColors = pmap->pVisual->ColormapEntries;
    else 
	numColors = 1ull << pmap->pVisual->nplanes; 

In our case, pmap->pVisual->nplanes is 32, thus (1 << pmap->pVisual->nplanes)
is undefined, especially on a 32 bits architecture. This cause the subsequent
xalloc() to do random things based on what we have here. On ppc, we are lucky,
it gets a nice 0 and we fail right away. In other circumstances, you might end
up allocating random amount of memory...

This construct is used in many places in xf86cmap.c to calulate the size of
the color map to use.

The "simple" way to fix it, as per the patch below, is to turn 1 into 1ull
(unsigned long long) thus causing the test to properly detect that a 32
planes visual has too many colors. 

If use of long longs is a problem for X, then we should replace that construct
with a function that explicitely test if nplanes is too big for the native
integer size (or simply > 32, do we support platforms where unsigned int is
not 32 bits ?)

As discussed on IRC, X supports colormaps with up to 16 bit components, so it
seems reasonable to limit colormaps to that same depth; a function which
explicitly checks depth against 16 and only then goes on to compute 1 << depth
should work fine.

Created attachment 3171 [details] [review]
Fix colormap private allocation in dix

Ok, as per discussion with Keith on IRC, I broke up the patch in 2. This one
fixes the allocation of colormaps devPrivate array so it's properly NULL'ified

Created attachment 3172 [details] [review]
Fix colormap on 32 bits visuals

And this patch fixes the colormap code for 32 bits visuals by separating the
test into a function that only does 1 << nplanes when nplanes <= 16

both patches committed to HEAD