[PATCH 1/3] Fix xf86HandleColormaps() crash at color depth 30.
michel at daenzer.net
Fri Dec 22 08:57:24 UTC 2017
On 2017-12-22 02:49 AM, Mario Kleiner wrote:
> 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,
>>> #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
We can cross that bridge when we get there.
> Haven't looked yet what DC uses in amdgpu-kms, and what therefore makes
> sense for the amdgpu-ddx?
The legacy/non-atomic KMS gamma ramp has no effect at depth 30 with DC
as well, so the plan for xf86-video-amdgpu is what I described above.
Let's not diverge on this without good reason.
Earthling Michel Dänzer | http://www.amd.com
Libre software enthusiast | Mesa and X developer
More information about the amd-gfx