Attachment 62203 Details for Bug 36108 – Don't copy stale pointers

[patch] Don't copy stale pointers

0001-xf86-modes-Defang-stale-pointers-when-copying-modes.patch (text/plain), 5.31 KB, created by Chris Wilson on 2012-05-29 04:19:49 UTC

(hide)

Description:

Filename:

MIME Type:

Creator: Chris Wilson

Created: 2012-05-29 04:19:49 UTC

Size: 5.31 KB

patch

obsolete

>From 264dea9e4b2edcbff4676dbc597b7f80beb8b2eb Mon Sep 17 00:00:00 2001
>From: Chris Wilson <chris@chris-wilson.co.uk>
>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 <zdenek.kabelac@gmail.com>
>Bugzilla: https://bugs.freedesktop.org/show_bug.cgi?id=36108
>Signed-off-by: Chris Wilson <chris@chris-wilson.co.uk>
>---
> 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
>

Actions: View | Splinter Review

Attachments on bug 36108: 58548 | 62203

Use of freedesktop.org services, including Bugzilla, is subject to our Code of Conduct. How we collect and use information is described in our Privacy Policy.