From 264dea9e4b2edcbff4676dbc597b7f80beb8b2eb Mon Sep 17 00:00:00 2001 From: Chris Wilson Date: Fri, 16 Mar 2012 11:24:12 +0000 Subject: [PATCH] xf86/modes: Defang stale pointers when copying modes 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) v2: Rebase upon whitespaces changes. Reported-by: Zdenek Kabelac Bugzilla: https://bugs.freedesktop.org/show_bug.cgi?id=36108 Signed-off-by: Chris Wilson --- hw/xfree86/common/xf86.h | 2 ++ hw/xfree86/modes/xf86Crtc.c | 6 +++--- hw/xfree86/modes/xf86Modes.c | 11 +++++++++++ hw/xfree86/modes/xf86RandR12.c | 2 +- 4 files changed, 17 insertions(+), 4 deletions(-) diff --git a/hw/xfree86/common/xf86.h b/hw/xfree86/common/xf86.h index 0eb000d..6e5e768 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 +xf86InternMode(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 ab6ed5e..c8c2d72 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; + xf86InternMode(&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; + xf86InternMode(&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; + 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 2a6d267..bfdc311 100644 --- a/hw/xfree86/modes/xf86Modes.c +++ b/hw/xfree86/modes/xf86Modes.c @@ -191,6 +191,17 @@ xf86SetModeCrtc(DisplayModePtr p, int adjustFlags) } /** + * Fills in a copy of mode, removing all stale pointer references. + */ +void +xf86InternMode(DisplayModePtr intern, const DisplayModeRec *mode) +{ + *intern = *mode; + intern->prev = intern->next = NULL; + intern->name = 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 e6b2052..0dfca5d 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; + xf86InternMode(&crtc->desiredMode, &mode); crtc->desiredRotation = rotation; if (transform) { crtc->desiredTransform = *transform; -- 1.7.10