[resend with fixed changelog entry PATCH] xfree86: Strip dangling pointers from desiredMode

[Maarten Lankhorst]

Based on the original patch by Chris Wilson, which was a better fix than mine.

We stash a copy of the desiredMode on the crtc so that we can restore it
after a vt switch. This copy is a simple memcpy and so also stashes a
references to the pointers contained within the desiredMode. Those
pointers are freed the next time the outputs are probed and mode list
rebuilt, resulting in us chasing those dangling pointers on the next
mode switch.

==22787== Invalid read of size 1
==22787==    at 0x40293C2: __GI_strlen (in
/usr/lib/valgrind/vgpreload_memcheck-amd64-linux.so)
==22787==    by 0x668F875: strdup (strdup.c:42)
==22787==    by 0x5DBA00: XNFstrdup (utils.c:1124)
==22787==    by 0x4D72ED: xf86DuplicateMode (xf86Modes.c:209)
==22787==    by 0x4CA848: xf86CrtcSetModeTransform (xf86Crtc.c:276)
==22787==    by 0x4D05B4: xf86SetDesiredModes (xf86Crtc.c:2677)
==22787==    by 0xA7479D0: sna_create_screen_resources
(sna_driver.c:220)
==22787==    by 0x4CB914: xf86CrtcCreateScreenResources (xf86Crtc.c:725)
==22787==    by 0x425498: main (main.c:216)
==22787==  Address 0x72c60e0 is 0 bytes inside a block of size 9 free'd
==22787==    at 0x4027AAE: free (in
/usr/lib/valgrind/vgpreload_memcheck-amd64-linux.so)
==22787==    by 0x4A547E: xf86DeleteMode (xf86Mode.c:1984)
==22787==    by 0x4CD84F: xf86ProbeOutputModes (xf86Crtc.c:1578)
==22787==    by 0x4DC405: xf86RandR12GetInfo12 (xf86RandR12.c:1537)
==22787==    by 0x518119: RRGetInfo (rrinfo.c:202)
==22787==    by 0x51D997: rrGetScreenResources (rrscreen.c:335)
==22787==    by 0x51E0D0: ProcRRGetScreenResources (rrscreen.c:475)
==22787==    by 0x513852: ProcRRDispatch (randr.c:493)
==22787==    by 0x4346DB: Dispatch (dispatch.c:439)
==22787==    by 0x4256E4: main (main.c:287)

Signed-off-by: Maarten Lankhorst <maarten.lankhorst at canonical.com>
Reported-by: Zdenek Kabelac <zdenek.kabelac at gmail.com>
Bugzilla: https://bugs.freedesktop.org/show_bug.cgi?id=36108
Reviewed-by: Chris Wilson <chris at chris-wilson.co.uk>

---

diff --git a/hw/xfree86/common/xf86.h b/hw/xfree86/common/xf86.h
index 129660d..0a90060 100644
--- a/hw/xfree86/common/xf86.h
+++ b/hw/xfree86/common/xf86.h
@@ -417,6 +417,8 @@ extern _X_EXPORT void
 xf86SetModeCrtc(DisplayModePtr p, int adjustFlags);
 extern _X_EXPORT DisplayModePtr
 xf86DuplicateMode(const DisplayModeRec * pMode);
+extern _X_EXPORT void
+xf86SaveModeContents(DisplayModePtr intern, const DisplayModeRec * pMode);
 extern _X_EXPORT DisplayModePtr
 xf86DuplicateModes(ScrnInfoPtr pScrn, DisplayModePtr modeList);
 extern _X_EXPORT Bool
diff --git a/hw/xfree86/modes/xf86Crtc.c b/hw/xfree86/modes/xf86Crtc.c
index 2c8878f..fb3eda5 100644
--- a/hw/xfree86/modes/xf86Crtc.c
+++ b/hw/xfree86/modes/xf86Crtc.c
@@ -2436,7 +2436,7 @@ xf86InitialConfiguration(ScrnInfoPtr scrn, Bool canGrow)
         xf86CrtcPtr crtc = crtcs[o];
 
         if (mode && crtc) {
-            crtc->desiredMode = *mode;
+            xf86SaveModeContents(&crtc->desiredMode, mode);
             crtc->desiredRotation = output->initial_rotation;
             crtc->desiredX = output->initial_x;
             crtc->desiredY = output->initial_y;
@@ -2620,7 +2620,7 @@ xf86SetDesiredModes(ScrnInfoPtr scrn)
 
             if (!mode)
                 return FALSE;
-            crtc->desiredMode = *mode;
+            xf86SaveModeContents(&crtc->desiredMode, mode);
             crtc->desiredRotation = RR_Rotate_0;
             crtc->desiredTransformPresent = FALSE;
             crtc->desiredX = 0;
@@ -2759,7 +2759,7 @@ xf86SetSingleMode(ScrnInfoPtr pScrn, DisplayModePtr desired, Rotation rotation)
         if (!xf86CrtcSetModeTransform(crtc, crtc_mode, rotation, NULL, 0, 0))
             ok = FALSE;
         else {
-            crtc->desiredMode = *crtc_mode;
+            xf86SaveModeContents(&crtc->desiredMode, crtc_mode);
             crtc->desiredRotation = rotation;
             crtc->desiredTransformPresent = FALSE;
             crtc->desiredX = 0;
diff --git a/hw/xfree86/modes/xf86Modes.c b/hw/xfree86/modes/xf86Modes.c
index 2a6d267..c4a3eb0 100644
--- a/hw/xfree86/modes/xf86Modes.c
+++ b/hw/xfree86/modes/xf86Modes.c
@@ -191,6 +191,21 @@ xf86SetModeCrtc(DisplayModePtr p, int adjustFlags)
 }
 
 /**
+ * Fills in a copy of mode, removing all stale pointer references.
+ * xf86ModesEqual will return true when comparing with original mode.
+ */
+void
+xf86SaveModeContents(DisplayModePtr intern, const DisplayModeRec *mode)
+{
+    *intern = *mode;
+    intern->prev = intern->next = NULL;
+    intern->name = NULL;
+    intern->PrivSize = 0;
+    intern->PrivFlags = 0;
+    intern->Private = NULL;
+}
+
+/**
  * Allocates and returns a copy of pMode, including pointers within pMode.
  */
 DisplayModePtr
diff --git a/hw/xfree86/modes/xf86RandR12.c b/hw/xfree86/modes/xf86RandR12.c
index 59b6f82..8bce801 100644
--- a/hw/xfree86/modes/xf86RandR12.c
+++ b/hw/xfree86/modes/xf86RandR12.c
@@ -1216,7 +1216,7 @@ xf86RandR12CrtcSet(ScreenPtr pScreen,
             /*
              * Save the last successful setting for EnterVT
              */
-            crtc->desiredMode = mode;
+            xf86SaveModeContents(&crtc->desiredMode, &mode);
             crtc->desiredRotation = rotation;
             if (transform) {
                 crtc->desiredTransform = *transform;


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