Attachment 37760 Details for Bug 18451 – Scope callback data for use by callback.

[patch] Scope callback data for use by callback.

0001-xace-Invalid-reference-to-out-of-scope-data.patch (text/plain), 7.65 KB, created by Chris Wilson on 2010-08-10 01:39:39 UTC

(hide)

Description:

Filename:

MIME Type:

Creator: Chris Wilson

Created: 2010-08-10 01:39:39 UTC

Size: 7.65 KB

patch

obsolete

>From 72b1f171c13060c14cf7ee266e961a17590926fd Mon Sep 17 00:00:00 2001
>From: Chris Wilson <chris@chris-wilson.co.uk>
>Date: Tue, 10 Aug 2010 09:27:53 +0100
>Subject: [PATCH] xace: Invalid reference to out-of-scope data.
>
>The callback data passed by reference to the hook was allocated on stack
>within the scope of the case statement. The compiler is free to reuse
>any of that stack space whilst making the function call so we may end up
>passing garbage into the callback.
>
>References:
>
>  Bug 18451 - Xorg server 1.5.2 SEGV during XFixesGetCursorImage()
>  https://bugs.freedesktop.org/show_bug.cgi?id=18451
>
>Signed-off-by: Chris Wilson <chris@chris-wilson.co.uk>
>---
> Xext/xace.c                 |  170 +++++++++++++++++++------------------------
> hw/xfree86/modes/xf86Crtc.c |    8 ++-
> 2 files changed, 82 insertions(+), 96 deletions(-)
>
>diff --git a/Xext/xace.c b/Xext/xace.c
>index e10d837..c757cad 100644
>--- a/Xext/xace.c
>+++ b/Xext/xace.c
>@@ -87,7 +87,18 @@ void XaceHookAuditEnd(ClientPtr ptr, int result)
>  */
> int XaceHook(int hook, ...)
> {
>-    pointer calldata;	/* data passed to callback */
>+    union {
>+	XaceResourceAccessRec res;
>+	XaceDeviceAccessRec dev;
>+	XaceSendAccessRec send;
>+	XaceReceiveAccessRec recv;
>+	XaceClientAccessRec client;
>+	XaceExtAccessRec ext;
>+	XaceServerAccessRec server;
>+	XaceScreenAccessRec screen;
>+	XaceAuthAvailRec auth;
>+	XaceKeyAvailRec key;
>+    } u;
>     int *prv = NULL;	/* points to return value from callback */
>     va_list ap;		/* argument list */
>     va_start(ap, hook);
>@@ -99,117 +110,86 @@ int XaceHook(int hook, ...)
>      */
>     switch (hook)
>     {
>-	case XACE_RESOURCE_ACCESS: {
>-	    XaceResourceAccessRec rec;
>-	    rec.client = va_arg(ap, ClientPtr);
>-	    rec.id = va_arg(ap, XID);
>-	    rec.rtype = va_arg(ap, RESTYPE);
>-	    rec.res = va_arg(ap, pointer);
>-	    rec.ptype = va_arg(ap, RESTYPE);
>-	    rec.parent = va_arg(ap, pointer);
>-	    rec.access_mode = va_arg(ap, Mask);
>-	    rec.status = Success; /* default allow */
>-	    calldata = &rec;
>-	    prv = &rec.status;
>+	case XACE_RESOURCE_ACCESS:
>+	    u.res.client = va_arg(ap, ClientPtr);
>+	    u.res.id = va_arg(ap, XID);
>+	    u.res.rtype = va_arg(ap, RESTYPE);
>+	    u.res.res = va_arg(ap, pointer);
>+	    u.res.ptype = va_arg(ap, RESTYPE);
>+	    u.res.parent = va_arg(ap, pointer);
>+	    u.res.access_mode = va_arg(ap, Mask);
>+	    u.res.status = Success; /* default allow */
>+	    prv = &u.res.status;
> 	    break;
>-	}
>-	case XACE_DEVICE_ACCESS: {
>-	    XaceDeviceAccessRec rec;
>-	    rec.client = va_arg(ap, ClientPtr);
>-	    rec.dev = va_arg(ap, DeviceIntPtr);
>-	    rec.access_mode = va_arg(ap, Mask);
>-	    rec.status = Success; /* default allow */
>-	    calldata = &rec;
>-	    prv = &rec.status;
>+	case XACE_DEVICE_ACCESS:
>+	    u.dev.client = va_arg(ap, ClientPtr);
>+	    u.dev.dev = va_arg(ap, DeviceIntPtr);
>+	    u.dev.access_mode = va_arg(ap, Mask);
>+	    u.dev.status = Success; /* default allow */
>+	    prv = &u.dev.status;
> 	    break;
>-	}
>-	case XACE_SEND_ACCESS: {
>-	    XaceSendAccessRec rec;
>-	    rec.client = va_arg(ap, ClientPtr);
>-	    rec.dev = va_arg(ap, DeviceIntPtr);
>-	    rec.pWin = va_arg(ap, WindowPtr);
>-	    rec.events = va_arg(ap, xEventPtr);
>-	    rec.count = va_arg(ap, int);
>-	    rec.status = Success; /* default allow */
>-	    calldata = &rec;
>-	    prv = &rec.status;
>+	case XACE_SEND_ACCESS:
>+	    u.send.client = va_arg(ap, ClientPtr);
>+	    u.send.dev = va_arg(ap, DeviceIntPtr);
>+	    u.send.pWin = va_arg(ap, WindowPtr);
>+	    u.send.events = va_arg(ap, xEventPtr);
>+	    u.send.count = va_arg(ap, int);
>+	    u.send.status = Success; /* default allow */
>+	    prv = &u.send.status;
> 	    break;
>-	}
>-	case XACE_RECEIVE_ACCESS: {
>-	    XaceReceiveAccessRec rec;
>-	    rec.client = va_arg(ap, ClientPtr);
>-	    rec.pWin = va_arg(ap, WindowPtr);
>-	    rec.events = va_arg(ap, xEventPtr);
>-	    rec.count = va_arg(ap, int);
>-	    rec.status = Success; /* default allow */
>-	    calldata = &rec;
>-	    prv = &rec.status;
>+	case XACE_RECEIVE_ACCESS:
>+	    u.recv.client = va_arg(ap, ClientPtr);
>+	    u.recv.pWin = va_arg(ap, WindowPtr);
>+	    u.recv.events = va_arg(ap, xEventPtr);
>+	    u.recv.count = va_arg(ap, int);
>+	    u.recv.status = Success; /* default allow */
>+	    prv = &u.recv.status;
> 	    break;
>-	}
>-	case XACE_CLIENT_ACCESS: {
>-	    XaceClientAccessRec rec;
>-	    rec.client = va_arg(ap, ClientPtr);
>-	    rec.target = va_arg(ap, ClientPtr);
>-	    rec.access_mode = va_arg(ap, Mask);
>-	    rec.status = Success; /* default allow */
>-	    calldata = &rec;
>-	    prv = &rec.status;
>+	case XACE_CLIENT_ACCESS:
>+	    u.client.client = va_arg(ap, ClientPtr);
>+	    u.client.target = va_arg(ap, ClientPtr);
>+	    u.client.access_mode = va_arg(ap, Mask);
>+	    u.client.status = Success; /* default allow */
>+	    prv = &u.client.status;
> 	    break;
>-	}
>-	case XACE_EXT_ACCESS: {
>-	    XaceExtAccessRec rec;
>-	    rec.client = va_arg(ap, ClientPtr);
>-	    rec.ext = va_arg(ap, ExtensionEntry*);
>-	    rec.access_mode = DixGetAttrAccess;
>-	    rec.status = Success; /* default allow */
>-	    calldata = &rec;
>-	    prv = &rec.status;
>+	case XACE_EXT_ACCESS:
>+	    u.ext.client = va_arg(ap, ClientPtr);
>+	    u.ext.ext = va_arg(ap, ExtensionEntry*);
>+	    u.ext.access_mode = DixGetAttrAccess;
>+	    u.ext.status = Success; /* default allow */
>+	    prv = &u.ext.status;
> 	    break;
>-	}
>-	case XACE_SERVER_ACCESS: {
>-	    XaceServerAccessRec rec;
>-	    rec.client = va_arg(ap, ClientPtr);
>-	    rec.access_mode = va_arg(ap, Mask);
>-	    rec.status = Success; /* default allow */
>-	    calldata = &rec;
>-	    prv = &rec.status;
>+	case XACE_SERVER_ACCESS:
>+	    u.server.client = va_arg(ap, ClientPtr);
>+	    u.server.access_mode = va_arg(ap, Mask);
>+	    u.server.status = Success; /* default allow */
>+	    prv = &u.server.status;
> 	    break;
>-	}
> 	case XACE_SCREEN_ACCESS:
>-	case XACE_SCREENSAVER_ACCESS: {
>-	    XaceScreenAccessRec rec;
>-	    rec.client = va_arg(ap, ClientPtr);
>-	    rec.screen = va_arg(ap, ScreenPtr);
>-	    rec.access_mode = va_arg(ap, Mask);
>-	    rec.status = Success; /* default allow */
>-	    calldata = &rec;
>-	    prv = &rec.status;
>+	case XACE_SCREENSAVER_ACCESS:
>+	    u.screen.client = va_arg(ap, ClientPtr);
>+	    u.screen.screen = va_arg(ap, ScreenPtr);
>+	    u.screen.access_mode = va_arg(ap, Mask);
>+	    u.screen.status = Success; /* default allow */
>+	    prv = &u.screen.status;
> 	    break;
>-	}
>-	case XACE_AUTH_AVAIL: {
>-	    XaceAuthAvailRec rec;
>-	    rec.client = va_arg(ap, ClientPtr);
>-	    rec.authId = va_arg(ap, XID);
>-	    calldata = &rec;
>+	case XACE_AUTH_AVAIL:
>+	    u.auth.client = va_arg(ap, ClientPtr);
>+	    u.auth.authId = va_arg(ap, XID);
> 	    break;
>-	}
>-	case XACE_KEY_AVAIL: {
>-	    XaceKeyAvailRec rec;
>-	    rec.event = va_arg(ap, xEventPtr);
>-	    rec.keybd = va_arg(ap, DeviceIntPtr);
>-	    rec.count = va_arg(ap, int);
>-	    calldata = &rec;
>+	case XACE_KEY_AVAIL:
>+	    u.key.event = va_arg(ap, xEventPtr);
>+	    u.key.keybd = va_arg(ap, DeviceIntPtr);
>+	    u.key.count = va_arg(ap, int);
> 	    break;
>-	}
>-	default: {
>+	default:
> 	    va_end(ap);
> 	    return 0;	/* unimplemented hook number */
>-	}
>     }
>     va_end(ap);
>  
>     /* call callbacks and return result, if any. */
>-    CallCallbacks(&XaceHooks[hook], calldata);
>+    CallCallbacks(&XaceHooks[hook], &u);
>     return prv ? *prv : Success;
> }
> 
>diff --git a/hw/xfree86/modes/xf86Crtc.c b/hw/xfree86/modes/xf86Crtc.c
>index b2daec7..3cbaa08 100644
>--- a/hw/xfree86/modes/xf86Crtc.c
>+++ b/hw/xfree86/modes/xf86Crtc.c
>@@ -1393,7 +1393,13 @@ xf86ModeCompare (DisplayModePtr a, DisplayModePtr b)
>     if (diff)
> 	return diff;
>     diff = b->Clock - a->Clock;
>-    return diff;
>+    if (diff)
>+	    return diff;
>+    if (a->Private)
>+	    return -1;
>+    if (b->Private)
>+	    return 1;
>+    return 0;
> }
> 
> /**
>-- 
>1.7.1
>

Actions: View | Splinter Review

Attachments on bug 18451: 20159 | 37760

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.