[PATCH 1/3] Fix xf86HandleColormaps() crash at color depth 30.

[Mario Kleiner]

On 12/19/2017 09:58 AM, Michel Dänzer wrote:
> On 2017-12-18 11:36 PM, Mario Kleiner wrote:
>> The size of the X-Server pScreenPriv->PreAllocIndices
>> array allocated within xf86HandleColormaps() is given
>> by the given maxColors argument, but the range of
>> indices by which the PreAllocIndices array is indexed
>> in routines like CMapReinstallMap() seems to be up to
>> 1023 on a 10 bpc / depth 30 screen, leading to an
>> out-of-bounds access and server crash.
>>
>> Raising maxColors to 1024 fixes the crash at server
>> startup with X-Screen color depth 30.
>>
>> Signed-off-by: Mario Kleiner <mario.kleiner.de at gmail.com>
>> ---
>>   src/drmmode_display.c | 3 ++-
>>   1 file changed, 2 insertions(+), 1 deletion(-)
>>
>> diff --git a/src/drmmode_display.c b/src/drmmode_display.c
>> index 7ad3235..67db86e 100644
>> --- a/src/drmmode_display.c
>> +++ b/src/drmmode_display.c
>> @@ -2730,8 +2730,9 @@ Bool drmmode_setup_colormap(ScreenPtr pScreen, ScrnInfoPtr pScrn)
>>   		       "Initializing kms color map\n");
>>   	if (!miCreateDefColormap(pScreen))
>>   	    return FALSE;
>> +
>>   	/* all radeons support 10 bit CLUTs */
>> -	if (!xf86HandleColormaps(pScreen, 256, 10,
>> +	if (!xf86HandleColormaps(pScreen, 1024, 10,
>>   				 NULL, NULL,
>>   				 CMAP_PALETTED_TRUECOLOR
>>   #if 0 /* This option messes up text mode! (eich at suse.de) */
>>
> 
> The hardware CLUT isn't actually used at depth 30, so a better solution
> would be to skip the xf86HandleColormaps call (and probably also set
> xf86CrtcFuncsRec::gamma_set = NULL) for pScrn->depth == 30.
> 

I know. Skipping the xf86HandleColormaps() function was the solution i 
first used, but then i thought we might want to keep it as 
future-proofing in case we'd implement hw gamma tables in the kms 
drivers for depth > 24 by switching to the piecewise linear hw gamma 
tables. Haven't looked yet what DC uses in amdgpu-kms, and what 
therefore makes sense for the amdgpu-ddx?

But no strong opinion, so whatever you prefer?

thanks,
-mario