Michel Dänzer <mic...@daenzer.net> writes:

> From: Michel Dänzer <michel.daen...@amd.com>
>
> Fixes crash in ScreenInit -> xf86HandleColormaps ->
> xf86RandR12LoadPalette with drivers which don't assign pScrn->pScreen
> in ScreenInit.

That's a terrible interface. This should be fixed by wrapping the driver
ScreenInit and getting pScrn->pScreen set before the 'real' driver
ScreenInit is called; that way pScrn->pScreen will *always* be valid.

We could then eliminate the xf86ScrnToScreen API entirely at some point.

Here's an untested patch which should fix this:

From f96e5432370b74a7b1bc9ed528f0f971d39a5951 Mon Sep 17 00:00:00 2001
From: Keith Packard <kei...@keithp.com>
Date: Fri, 29 Jul 2016 17:45:45 -0700
Subject: [PATCH xserver] xfree86: Set pScrn->pScreen before driver ScreenInit
 is called

Any code called from the driver ScreenInit may want to refer to
pScrn->pScreen. As the function passed to AddScreen is the first place
the DDX sees a new screen, the generic code needs to make sure that
value is set before passing control to the video driver's
initialization code.

This was found by running a driver which didn't bother to set this
value when the initial colormap was installed; xf86RandR12LoadPalette
tried to use pScrn->pScreen and crashed.

Bugzilla: https://bugs.freedesktop.org/show_bug.cgi?id=97124
Signed-off-by: Keith Packard <kei...@keithp.com>
---
 hw/xfree86/common/xf86Init.c | 13 +++++++++++--
 1 file changed, 11 insertions(+), 2 deletions(-)

diff --git a/hw/xfree86/common/xf86Init.c b/hw/xfree86/common/xf86Init.c
index 7a267f8..a544b65 100644
--- a/hw/xfree86/common/xf86Init.c
+++ b/hw/xfree86/common/xf86Init.c
@@ -360,6 +360,15 @@ AddVTAtoms(CallbackListPtr *pcbl, void *data, void *screen)
                    "Failed to register VT properties\n");
 }
 
+static Bool
+xf86ScreenInit(ScreenPtr pScreen, int argc, char **argv)
+{
+    ScrnInfoPtr pScrn = xf86ScreenToScrn(pScreen);
+
+    pScrn->pScreen = pScreen;
+    return pScrn->ScreenInit (pScreen, argc, argv);
+}
+
 /*
  * InitOutput --
  *	Initialize screenInfo for all actually accessible framebuffers.
@@ -791,7 +800,7 @@ InitOutput(ScreenInfo * pScreenInfo, int argc, char **argv)
         pScrn->SetOverscan = NULL;
         pScrn->DriverFunc = NULL;
         pScrn->pScreen = NULL;
-        scr_index = AddGPUScreen(pScrn->ScreenInit, argc, argv);
+        scr_index = AddGPUScreen(xf86ScreenInit, argc, argv);
         xf86VGAarbiterUnlock(pScrn);
         if (scr_index == i) {
             dixSetPrivate(&screenInfo.gpuscreens[scr_index]->devPrivates,
@@ -819,7 +828,7 @@ InitOutput(ScreenInfo * pScreenInfo, int argc, char **argv)
         xf86Screens[i]->SetOverscan = NULL;
         xf86Screens[i]->DriverFunc = NULL;
         xf86Screens[i]->pScreen = NULL;
-        scr_index = AddScreen(xf86Screens[i]->ScreenInit, argc, argv);
+        scr_index = AddScreen(xf86ScreenInit, argc, argv);
         xf86VGAarbiterUnlock(xf86Screens[i]);
         if (scr_index == i) {
             /*
-- 
2.8.1

-- 
-keith

Attachment: signature.asc
Description: PGP signature

_______________________________________________
xorg-devel@lists.x.org: X.Org development
Archives: http://lists.x.org/archives/xorg-devel
Info: https://lists.x.org/mailman/listinfo/xorg-devel

Reply via email to