[Date Prev][Date Next] [Thread Prev][Thread Next] [Date Index] [Thread Index]

Bug#572268: xserver-xorg-core: Crash, Segmentation fault, VidModeSetGamma+0x67, xf86ChangeGamma+0x4b



Marcin Szewczyk <debian.bugreport@wodny.org> (04/03/2010):
> I got 2 gdb backtraces - both the same, both with the additional
> function xf86RandR12ChangeGamma.
> 
> I got 2 X traces (associated with the gdb ones - same crashes) -
> both the same, both without the function.

Many thanks for the clarification. :)

> > The patch I've just cherry-picked should help (that's for the
> > extra function that gets called in the “new” trace) getting rid of
> > the crash entirely.
> 
> OK, could You provide me with the source version of the second
> patch? I learn new things here and I'm curious. Everything should
> happen faster now, as I know the procedure and I don't have to do a
> clean rebuild.

Sure, please find it attached.

> I will test:
> - the patch1 + patch2 version,
> - the patch2 only version.

Thanks!

Mraw,
KiBi.
From 2880b2eaac88248cfe68596c01e04c746fa99864 Mon Sep 17 00:00:00 2001
From: Keith Packard <keithp@keithp.com>
Date: Thu, 25 Feb 2010 11:37:05 -0800
Subject: [PATCH] Allow for missing or disabled compat_output

When the compat output is missing (I don't think this is actually
possible), or is disabled (and hence has no crtc), we would like to
avoid dereferencing NULL pointers. This patch creates inline functions
to extract the current compat output, crtc or associated RandR crtc
structure, carefully checking for NULL pointers everywhere.

Reviewed-by: Adam Jackson <ajax@redhat.com>
Signed-off-by: Keith Packard <keithp@keithp.com>
(cherry picked from commit de86a3a3448f0a55c1cd99aee9ea80070a589877)
---
 hw/xfree86/common/xf86cmap.c   |    9 +++------
 hw/xfree86/modes/xf86Crtc.c    |   14 ++++++++------
 hw/xfree86/modes/xf86Crtc.h    |   26 ++++++++++++++++++++++++++
 hw/xfree86/modes/xf86RandR12.c |   14 +++++++++-----
 4 files changed, 46 insertions(+), 17 deletions(-)

diff --git a/hw/xfree86/common/xf86cmap.c b/hw/xfree86/common/xf86cmap.c
index 4769d87..c9d57be 100644
--- a/hw/xfree86/common/xf86cmap.c
+++ b/hw/xfree86/common/xf86cmap.c
@@ -1001,8 +1001,7 @@ xf86ChangeGammaRamp(
     CMapLinkPtr pLink;
 
     if (xf86_crtc_supports_gamma(pScrn)) {
-	xf86CrtcConfigPtr config = XF86_CRTC_CONFIG_PTR(pScrn);
-	RRCrtcPtr crtc = config->output[config->compat_output]->crtc->randr_crtc;
+	RRCrtcPtr crtc = xf86CompatRRCrtc(pScrn);
 
 	if (crtc) {
 	    if (crtc->gammaSize != size)
@@ -1076,8 +1075,7 @@ xf86GetGammaRampSize(ScreenPtr pScreen)
     CMapScreenPtr pScreenPriv;
 
     if (xf86_crtc_supports_gamma(pScrn)) {
-	xf86CrtcConfigPtr config = XF86_CRTC_CONFIG_PTR(pScrn);
-	RRCrtcPtr crtc = config->output[config->compat_output]->crtc->randr_crtc;
+	RRCrtcPtr crtc = xf86CompatRRCrtc(pScrn);
 
 	if (crtc)
 	    return crtc->gammaSize;
@@ -1106,8 +1104,7 @@ xf86GetGammaRamp(
     int shift, sigbits;
 
     if (xf86_crtc_supports_gamma(pScrn)) {
-	xf86CrtcConfigPtr config = XF86_CRTC_CONFIG_PTR(pScrn);
-	RRCrtcPtr crtc = config->output[config->compat_output]->crtc->randr_crtc;
+	RRCrtcPtr crtc = xf86CompatRRCrtc(pScrn);
 
 	if (crtc) {
 	    if (crtc->gammaSize < size)
diff --git a/hw/xfree86/modes/xf86Crtc.c b/hw/xfree86/modes/xf86Crtc.c
index 30b49af..5a96e04 100644
--- a/hw/xfree86/modes/xf86Crtc.c
+++ b/hw/xfree86/modes/xf86Crtc.c
@@ -2572,8 +2572,8 @@ xf86SetDesiredModes (ScrnInfoPtr scrn)
 	if (!crtc->enabled)
 	    continue;
 
-	if (config->output[config->compat_output]->crtc == crtc)
-	    output = config->output[config->compat_output];
+	if (xf86CompatOutput(scrn) && xf86CompatCrtc(scrn) == crtc)
+	    output = xf86CompatOutput(scrn);
 	else
 	{
 	    for (o = 0; o < config->num_output; o++)
@@ -2693,14 +2693,16 @@ xf86SetSingleMode (ScrnInfoPtr pScrn, DisplayModePtr desired, Rotation rotation)
 {
     xf86CrtcConfigPtr	config = XF86_CRTC_CONFIG_PTR(pScrn);
     Bool		ok = TRUE;
-    xf86OutputPtr	compat_output = config->output[config->compat_output];
-    DisplayModePtr	compat_mode;
+    xf86OutputPtr	compat_output;
+    DisplayModePtr	compat_mode = NULL;
     int			c;
 
     /*
      * Let the compat output drive the final mode selection
      */
-    compat_mode = xf86OutputFindClosestMode (compat_output, desired);
+    compat_output = xf86CompatOutput(pScrn);
+    if (compat_output)
+	compat_mode = xf86OutputFindClosestMode (compat_output, desired);
     if (compat_mode)
 	desired = compat_mode;
     
@@ -2896,7 +2898,7 @@ xf86OutputSetEDID (xf86OutputPtr output, xf86MonPtr edid_mon)
     }
 
     /* Set the DDC properties for the 'compat' output */
-    if (output == config->output[config->compat_output])
+    if (output == xf86CompatOutput(scrn))
         xf86SetDDCproperties(scrn, edid_mon);
 
 #ifdef RANDR_12_INTERFACE
diff --git a/hw/xfree86/modes/xf86Crtc.h b/hw/xfree86/modes/xf86Crtc.h
index 9baa956..68a968c 100644
--- a/hw/xfree86/modes/xf86Crtc.h
+++ b/hw/xfree86/modes/xf86Crtc.h
@@ -689,6 +689,32 @@ extern _X_EXPORT int xf86CrtcConfigPrivateIndex;
 
 #define XF86_CRTC_CONFIG_PTR(p)	((xf86CrtcConfigPtr) ((p)->privates[xf86CrtcConfigPrivateIndex].ptr))
 
+static _X_INLINE xf86OutputPtr
+xf86CompatOutput(ScrnInfoPtr pScrn)
+{
+    xf86CrtcConfigPtr config = XF86_CRTC_CONFIG_PTR(pScrn);
+    return config->output[config->compat_output];
+}
+
+static _X_INLINE xf86CrtcPtr
+xf86CompatCrtc(ScrnInfoPtr pScrn)
+{
+    xf86OutputPtr compat_output = xf86CompatOutput(pScrn);
+    if (!compat_output)
+	return NULL;
+    return compat_output->crtc;
+}
+
+static _X_INLINE RRCrtcPtr
+xf86CompatRRCrtc(ScrnInfoPtr pScrn)
+{
+    xf86CrtcPtr	compat_crtc = xf86CompatCrtc(pScrn);
+    if (!compat_crtc)
+	return NULL;
+    return compat_crtc->randr_crtc;
+}
+
+
 /*
  * Initialize xf86CrtcConfig structure
  */
diff --git a/hw/xfree86/modes/xf86RandR12.c b/hw/xfree86/modes/xf86RandR12.c
index 1fc63c4..7ba09b6 100644
--- a/hw/xfree86/modes/xf86RandR12.c
+++ b/hw/xfree86/modes/xf86RandR12.c
@@ -805,9 +805,10 @@ xf86RandR12CreateScreenResources (ScreenPtr pScreen)
 	}
 	else
 	{
-	    xf86OutputPtr   output = config->output[config->compat_output];
+	    xf86OutputPtr   output = xf86CompatOutput(pScrn);
 
-	    if (output->conf_monitor &&
+	    if (output &&
+		output->conf_monitor &&
 		(output->conf_monitor->mon_width  > 0 &&
 		 output->conf_monitor->mon_height > 0))
 	    {
@@ -1719,10 +1720,13 @@ xf86RandR12ChangeGamma(int scrnIndex, Gamma gamma)
 {
     CARD16 *points, *red, *green, *blue;
     ScrnInfoPtr pScrn = xf86Screens[scrnIndex];
-    xf86CrtcConfigPtr config = XF86_CRTC_CONFIG_PTR(pScrn);
-    RRCrtcPtr crtc = config->output[config->compat_output]->crtc->randr_crtc;
-    int size = max(0, crtc->gammaSize);
+    RRCrtcPtr crtc = xf86CompatRRCrtc(pScrn);
+    int size;
 
+    if (!crtc)
+	return Success;
+
+    size = max(0, crtc->gammaSize);
     if (!size)
 	return Success;
 
-- 
1.7.0

Attachment: signature.asc
Description: Digital signature


Reply to: