From deb08658e2a6b1647a7213a316c6f3019bcdce48 Mon Sep 17 00:00:00 2001 From: Maarten Lankhorst Date: Wed, 11 Jul 2012 16:27:46 +0200 Subject: [PATCH] xfree86: Strip dangling pointers from desiredMode 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 Reported-by: Zdenek Kabelac Bugzilla: https://bugs.freedesktop.org/show_bug.cgi?id=36108 Reviewed-by: Chris Wilson Signed-off-by: Keith Packard --- hw/xfree86/common/xf86.h | 2 ++ hw/xfree86/modes/xf86Crtc.c | 6 +++--- hw/xfree86/modes/xf86Modes.c | 15 +++++++++++++++ hw/xfree86/modes/xf86RandR12.c | 2 +- 4 files changed, 21 insertions(+), 4 deletions(-) diff --git a/hw/xfree86/common/xf86.h b/hw/xfree86/common/xf86.h index 8dc7dcccf6..bb2903da0b 100644 --- a/hw/xfree86/common/xf86.h +++ b/hw/xfree86/common/xf86.h @@ -422,6 +422,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 d20152ce69..2628409d2f 100644 --- a/hw/xfree86/modes/xf86Crtc.c +++ b/hw/xfree86/modes/xf86Crtc.c @@ -2453,7 +2453,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; @@ -2637,7 +2637,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; @@ -2776,7 +2776,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 2a6d267568..c4a3eb0a36 100644 --- a/hw/xfree86/modes/xf86Modes.c +++ b/hw/xfree86/modes/xf86Modes.c @@ -190,6 +190,21 @@ xf86SetModeCrtc(DisplayModePtr p, int adjustFlags) p->CrtcVAdjusted = FALSE; } +/** + * 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. */ diff --git a/hw/xfree86/modes/xf86RandR12.c b/hw/xfree86/modes/xf86RandR12.c index b4ed46aebf..4be0ea32f0 100644 --- a/hw/xfree86/modes/xf86RandR12.c +++ b/hw/xfree86/modes/xf86RandR12.c @@ -1219,7 +1219,7 @@ xf86RandR12CrtcSet(ScreenPtr pScreen, /* * Save the last successful setting for EnterVT */ - crtc->desiredMode = mode; + xf86SaveModeContents(&crtc->desiredMode, &mode); crtc->desiredRotation = rotation; crtc->current_scanout = randr_crtc->scanout_pixmap; if (transform) {