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
