[PATCH] xf86/modes: Defang stale pointers when copying modes

[Chris Wilson]

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)

Reported-by: Zdenek Kabelac <zdenek.kabelac at gmail.com>
Bugzilla: https://bugs.freedesktop.org/show_bug.cgi?id=36108
Signed-off-by: Chris Wilson <chris at chris-wilson.co.uk>
---
 hw/xfree86/common/xf86.h       |    1 +
 hw/xfree86/modes/xf86Crtc.c    |    6 +++---
 hw/xfree86/modes/xf86Modes.c   |   11 +++++++++++
 hw/xfree86/modes/xf86RandR12.c |    2 +-
 4 files changed, 16 insertions(+), 4 deletions(-)

diff --git a/hw/xfree86/common/xf86.h b/hw/xfree86/common/xf86.h
index b711f05..f83d66f 100644
--- a/hw/xfree86/common/xf86.h
+++ b/hw/xfree86/common/xf86.h
@@ -326,6 +326,7 @@ extern _X_EXPORT double xf86ModeVRefresh(const DisplayModeRec *mode);
 extern _X_EXPORT void xf86SetModeDefaultName(DisplayModePtr mode);
 extern _X_EXPORT void xf86SetModeCrtc(DisplayModePtr p, int adjustFlags);
 extern _X_EXPORT DisplayModePtr xf86DuplicateMode(const DisplayModeRec *pMode);
+extern _X_EXPORT void xf86InternMode(DisplayModeRec *intern, const DisplayModeRec *mode);
 extern _X_EXPORT DisplayModePtr xf86DuplicateModes(ScrnInfoPtr pScrn, DisplayModePtr modeList);
 extern _X_EXPORT Bool xf86ModesEqual(const DisplayModeRec *pMode1,
 		    const DisplayModeRec *pMode2);
diff --git a/hw/xfree86/modes/xf86Crtc.c b/hw/xfree86/modes/xf86Crtc.c
index da9db34..2a94466 100644
--- a/hw/xfree86/modes/xf86Crtc.c
+++ b/hw/xfree86/modes/xf86Crtc.c
@@ -2481,7 +2481,7 @@ xf86InitialConfiguration (ScrnInfoPtr scrn, Bool canGrow)
 
 	if (mode && crtc)
 	{
-	    crtc->desiredMode = *mode;
+	    xf86InternMode(&crtc->desiredMode, mode);
 	    crtc->desiredRotation = output->initial_rotation;
 	    crtc->desiredX = output->initial_x;
 	    crtc->desiredY = output->initial_y;
@@ -2663,7 +2663,7 @@ xf86SetDesiredModes (ScrnInfoPtr scrn)
 
 	    if (!mode)
 		return FALSE;
-	    crtc->desiredMode = *mode;
+	    xf86InternMode(&crtc->desiredMode, mode);
 	    crtc->desiredRotation = RR_Rotate_0;
 	    crtc->desiredTransformPresent = FALSE;
 	    crtc->desiredX = 0;
@@ -2810,7 +2810,7 @@ xf86SetSingleMode (ScrnInfoPtr pScrn, DisplayModePtr desired, Rotation rotation)
 	    ok = FALSE;
 	else
 	{
-	    crtc->desiredMode = *crtc_mode;
+	    xf86InternMode(&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 49cc149..bc2fdf2 100644
--- a/hw/xfree86/modes/xf86Modes.c
+++ b/hw/xfree86/modes/xf86Modes.c
@@ -212,6 +212,17 @@ xf86DuplicateMode(const DisplayModeRec *pMode)
 }
 
 /**
+ * Fills in a copy of mode, removing all stale pointer references.
+ */
+void
+xf86InternMode(DisplayModeRec *intern, const DisplayModeRec *mode)
+{
+    *intern = *mode;
+    intern->prev = intern->next = NULL;
+    intern->name = NULL;
+}
+
+/**
  * Duplicates every mode in the given list and returns a pointer to the first
  * mode.
  *
diff --git a/hw/xfree86/modes/xf86RandR12.c b/hw/xfree86/modes/xf86RandR12.c
index d5031a2..97dc35f 100644
--- a/hw/xfree86/modes/xf86RandR12.c
+++ b/hw/xfree86/modes/xf86RandR12.c
@@ -1236,7 +1236,7 @@ xf86RandR12CrtcSet (ScreenPtr	    pScreen,
 	    /*
 	     * Save the last successful setting for EnterVT
 	     */
-	    crtc->desiredMode = mode;
+	    xf86InternMode(&crtc->desiredMode, &mode);
 	    crtc->desiredRotation = rotation;
 	    if (transform) {
 		crtc->desiredTransform = *transform;
-- 
1.7.9.1