On Thu, 12 Dec 2002 22:26:23 +0000
Keith Whitwell <[EMAIL PROTECTED]> wrote:
> Felix K�hling wrote:
> > Hi,
> >
> > while I was messing around with my query programme I found this: if I
> > specify an invalid screen as argument to XF86DRIGetClientDriverName the
> > Xserver segfaults. I had a quick look at xc/xc/Xserver/GL/dri/xf86dri.c.
> > stuff->screen is used as array index without checking. I'm not sure
> > though, where would be the right place to fix it.
> >
> > Other functions in xf86dri.c must be affacted, too. They use
> > stuff->screen in the same way.
>
> Those functions should validate any information they receive over the wire, as
> soon as is feasible.
The problem is that the request records are all different. So we can't
check stuff->screen in ProcXF86DRIDispatch. We have to do it in each
request separately.
The attached patch does just that. While I grepped around to see how
other extensions check this I found one more potential problem in
programs/Xserver/GL/glx/glxcmds.c and two suspicious casts of screen
from unsigned to int (screen is always unsigned in the request records).
They are fixed in the attached patch as well.
Regards,
Felix
__\|/__ ___ ___ ___
__Tsch��_______\_6 6_/___/__ \___/__ \___/___\___You can do anything,___
_____Felix_______\�/\ \_____\ \_____\ \______U___just not everything____
[EMAIL PROTECTED] >o<__/ \___/ \___/ at the same time!
Index: dri/xf86dri.c
===================================================================
RCS file: /cvsroot/dri/xc/xc/programs/Xserver/GL/dri/xf86dri.c,v
retrieving revision 1.10
diff -u -r1.10 xf86dri.c
--- dri/xf86dri.c 29 Oct 2002 20:28:57 -0000 1.10
+++ dri/xf86dri.c 13 Dec 2002 15:51:57 -0000
@@ -155,6 +155,11 @@
REQUEST(xXF86DRIQueryDirectRenderingCapableReq);
REQUEST_SIZE_MATCH(xXF86DRIQueryDirectRenderingCapableReq);
+ if (stuff->screen >= screenInfo.numScreens) {
+ client->errorValue = stuff->screen;
+ return BadValue;
+ }
+
rep.type = X_Reply;
rep.length = 0;
rep.sequenceNumber = client->sequence;
@@ -184,6 +189,10 @@
REQUEST(xXF86DRIOpenConnectionReq);
REQUEST_SIZE_MATCH(xXF86DRIOpenConnectionReq);
+ if (stuff->screen >= screenInfo.numScreens) {
+ client->errorValue = stuff->screen;
+ return BadValue;
+ }
if (!DRIOpenConnection( screenInfo.screens[stuff->screen],
&hSAREA,
@@ -221,6 +230,10 @@
REQUEST(xXF86DRIAuthConnectionReq);
REQUEST_SIZE_MATCH(xXF86DRIAuthConnectionReq);
+ if (stuff->screen >= screenInfo.numScreens) {
+ client->errorValue = stuff->screen;
+ return BadValue;
+ }
rep.type = X_Reply;
rep.length = 0;
@@ -242,6 +255,10 @@
{
REQUEST(xXF86DRICloseConnectionReq);
REQUEST_SIZE_MATCH(xXF86DRICloseConnectionReq);
+ if (stuff->screen >= screenInfo.numScreens) {
+ client->errorValue = stuff->screen;
+ return BadValue;
+ }
DRICloseConnection( screenInfo.screens[stuff->screen]);
@@ -258,6 +275,10 @@
REQUEST(xXF86DRIGetClientDriverNameReq);
REQUEST_SIZE_MATCH(xXF86DRIGetClientDriverNameReq);
+ if (stuff->screen >= screenInfo.numScreens) {
+ client->errorValue = stuff->screen;
+ return BadValue;
+ }
DRIGetClientDriverName( screenInfo.screens[stuff->screen],
(int *)&rep.ddxDriverMajorVersion,
@@ -295,6 +316,11 @@
REQUEST(xXF86DRICreateContextReq);
REQUEST_SIZE_MATCH(xXF86DRICreateContextReq);
+ if (stuff->screen >= screenInfo.numScreens) {
+ client->errorValue = stuff->screen;
+ return BadValue;
+ }
+
rep.type = X_Reply;
rep.length = 0;
rep.sequenceNumber = client->sequence;
@@ -329,6 +355,10 @@
{
REQUEST(xXF86DRIDestroyContextReq);
REQUEST_SIZE_MATCH(xXF86DRIDestroyContextReq);
+ if (stuff->screen >= screenInfo.numScreens) {
+ client->errorValue = stuff->screen;
+ return BadValue;
+ }
if (!DRIDestroyContext( screenInfo.screens[stuff->screen],
stuff->context)) {
@@ -348,6 +378,11 @@
REQUEST(xXF86DRICreateDrawableReq);
REQUEST_SIZE_MATCH(xXF86DRICreateDrawableReq);
+ if (stuff->screen >= screenInfo.numScreens) {
+ client->errorValue = stuff->screen;
+ return BadValue;
+ }
+
rep.type = X_Reply;
rep.length = 0;
rep.sequenceNumber = client->sequence;
@@ -378,6 +413,10 @@
REQUEST(xXF86DRIDestroyDrawableReq);
DrawablePtr pDrawable;
REQUEST_SIZE_MATCH(xXF86DRIDestroyDrawableReq);
+ if (stuff->screen >= screenInfo.numScreens) {
+ client->errorValue = stuff->screen;
+ return BadValue;
+ }
if (!(pDrawable = (DrawablePtr)SecurityLookupDrawable(
(Drawable)stuff->drawable,
@@ -409,6 +448,11 @@
REQUEST(xXF86DRIGetDrawableInfoReq);
REQUEST_SIZE_MATCH(xXF86DRIGetDrawableInfoReq);
+ if (stuff->screen >= screenInfo.numScreens) {
+ client->errorValue = stuff->screen;
+ return BadValue;
+ }
+
rep.type = X_Reply;
rep.length = 0;
rep.sequenceNumber = client->sequence;
@@ -483,6 +527,11 @@
REQUEST(xXF86DRIGetDeviceInfoReq);
REQUEST_SIZE_MATCH(xXF86DRIGetDeviceInfoReq);
+ if (stuff->screen >= screenInfo.numScreens) {
+ client->errorValue = stuff->screen;
+ return BadValue;
+ }
+
rep.type = X_Reply;
rep.length = 0;
rep.sequenceNumber = client->sequence;
@@ -528,6 +577,11 @@
DrawablePtr pDrawable;
REQUEST_SIZE_MATCH(xXF86DRIOpenFullScreenReq);
+ if (stuff->screen >= screenInfo.numScreens) {
+ client->errorValue = stuff->screen;
+ return BadValue;
+ }
+
rep.type = X_Reply;
rep.length = 0;
rep.sequenceNumber = client->sequence;
@@ -554,6 +608,11 @@
DrawablePtr pDrawable;
REQUEST_SIZE_MATCH(xXF86DRICloseFullScreenReq);
+ if (stuff->screen >= screenInfo.numScreens) {
+ client->errorValue = stuff->screen;
+ return BadValue;
+ }
+
rep.type = X_Reply;
rep.length = 0;
rep.sequenceNumber = client->sequence;
Index: glx/glxcmds.c
===================================================================
RCS file: /cvsroot/dri/xc/xc/programs/Xserver/GL/glx/glxcmds.c,v
retrieving revision 1.8
diff -u -r1.8 glxcmds.c
--- glx/glxcmds.c 25 Nov 2002 19:58:38 -0000 1.8
+++ glx/glxcmds.c 13 Dec 2002 15:52:01 -0000
@@ -761,7 +761,7 @@
int i, p;
screen = req->screen;
- if (screen > screenInfo.numScreens) {
+ if (screen >= screenInfo.numScreens) {
/* The client library must send a valid screen number. */
client->errorValue = screen;
return BadValue;
@@ -1466,7 +1466,7 @@
ClientPtr client = cl->client;
xGLXQueryExtensionsStringReq *req = (xGLXQueryExtensionsStringReq *) pc;
xGLXQueryExtensionsStringReply reply;
- GLint screen;
+ GLuint screen;
size_t n, length;
const char *ptr;
char *buf;
@@ -1475,7 +1475,7 @@
/*
** Check if screen exists.
*/
- if ((screen < 0) || (screen >= screenInfo.numScreens)) {
+ if (screen >= screenInfo.numScreens) {
client->errorValue = screen;
return BadValue;
}
@@ -1511,7 +1511,7 @@
xGLXQueryServerStringReq *req = (xGLXQueryServerStringReq *) pc;
xGLXQueryServerStringReply reply;
int name;
- GLint screen;
+ GLuint screen;
size_t n, length;
const char *ptr;
char *buf;
@@ -1521,7 +1521,7 @@
/*
** Check if screen exists.
*/
- if ((screen < 0) || (screen >= screenInfo.numScreens)) {
+ if (screen >= screenInfo.numScreens) {
client->errorValue = screen;
return BadValue;
}
