Re: bugzilla #439: bufSize in lib/GL/glx/glxcmds.c can be too large.

2003-07-01 Thread Egbert Eich
Ian Romanick writes:
  
  I looked into the code, and I now understand what's going on.  Alexis 
  made a good catch of a very subtle bug!  The main problem that I had was 
  that it wasn't 100% clear at first glance how bufSize / buf / pc were 
  used.  Some form of - 8 should be applied to bufSize.  I have attached 
  the patch that I plan to apply to the DRI tree.  I suspect that it has 
  only cosmetic and / or commentary differences from your patch.
  
  Some things have moved around in the DRI tree, so this patch probably 
  won't apply to the XFree86 tree.


We can wait until the DRI stuff is merged back again.
The patch indeed is very similar to what has been proposed in #439.

I've also looked at the GLX code. At line 671 in glxext.c
there is :
maxSize = ctx-bufSize - sizeof(xGLXRenderLargeReq);

Wouldn't we have to add sz_xGLXRenderReq there again?
I suppose if the size is to small it is saver as if it is too big.

Would you mind taking bug #439 and close it when the code is 
scheduled for merger with XFree86?

Thanks a lot!

   Egbert.

___
Devel mailing list
[EMAIL PROTECTED]
http://XFree86.Org/mailman/listinfo/devel


bugzilla #439: bufSize in lib/GL/glx/glxcmds.c can be too large.

2003-06-30 Thread Egbert Eich
There is a report in bugzilla (#439) which claims:

the bug is in xc/lib/GL/glx/glxcmds.c 
 int bufSize = XMaxRequestSize(dpy) * 4;
should be 
int bufSize = XMaxRequestSize(dpy) * 4 - 8;
or more cleanly
 int bufSize = XMaxRequestSize(dpy) * 4 - sizeof(xGLXRenderReq);

it happens that you may completely fill your GLX buffer if you 
use variable size command larger than 156 bytes (and smaller than 4096 bytes)
in that case you find yourself with an X command larger than 256Kbytes. This
is very unlikely but possible. It explain why this bug has not shown itself
before in this very old SGI code.

I've briefly looked at the code and it seems to be correct.
However I would like to double check before I commit anything.

Any opinions?


Egbert.
___
Devel mailing list
[EMAIL PROTECTED]
http://XFree86.Org/mailman/listinfo/devel


Re: bugzilla #439: bufSize in lib/GL/glx/glxcmds.c can be too large.

2003-06-30 Thread Ian Romanick
Egbert Eich wrote:
There is a report in bugzilla (#439) which claims:

the bug is in xc/lib/GL/glx/glxcmds.c 
 int bufSize = XMaxRequestSize(dpy) * 4;
should be 
int bufSize = XMaxRequestSize(dpy) * 4 - 8;
or more cleanly
 int bufSize = XMaxRequestSize(dpy) * 4 - sizeof(xGLXRenderReq);

it happens that you may completely fill your GLX buffer if you 
use variable size command larger than 156 bytes (and smaller than 4096 bytes)
in that case you find yourself with an X command larger than 256Kbytes. This
is very unlikely but possible. It explain why this bug has not shown itself
before in this very old SGI code.

I've briefly looked at the code and it seems to be correct.
However I would like to double check before I commit anything.
Any opinions?
I'm not sure this is correct.  bufSize is used to allocate the buffer 
(gc-buf in the code) that will hold the commands, including the 
xGLXRenderReq header.  I've been doing a lot of work lately on the GLX 
code (both client-side  server-side) in the DRI tree lately.  I'll take 
a look at this a bit more and get back to you.

___
Devel mailing list
[EMAIL PROTECTED]
http://XFree86.Org/mailman/listinfo/devel


Re: bugzilla #439: bufSize in lib/GL/glx/glxcmds.c can be too large.

2003-06-30 Thread Ian Romanick
Ian Romanick wrote:
Egbert Eich wrote:

There is a report in bugzilla (#439) which claims:

the bug is in xc/lib/GL/glx/glxcmds.c  int bufSize = 
XMaxRequestSize(dpy) * 4;
should be int bufSize = XMaxRequestSize(dpy) * 4 - 8;
or more cleanly
 int bufSize = XMaxRequestSize(dpy) * 4 - sizeof(xGLXRenderReq);

it happens that you may completely fill your GLX buffer if you use 
variable size command larger than 156 bytes (and smaller than 4096 bytes)
in that case you find yourself with an X command larger than 
256Kbytes. This
is very unlikely but possible. It explain why this bug has not shown 
itself
before in this very old SGI code.

I've briefly looked at the code and it seems to be correct.
However I would like to double check before I commit anything.
Any opinions?
I'm not sure this is correct.  bufSize is used to allocate the buffer 
(gc-buf in the code) that will hold the commands, including the 
xGLXRenderReq header.  I've been doing a lot of work lately on the GLX 
code (both client-side  server-side) in the DRI tree lately.  I'll take 
a look at this a bit more and get back to you.
I looked into the code, and I now understand what's going on.  Alexis 
made a good catch of a very subtle bug!  The main problem that I had was 
that it wasn't 100% clear at first glance how bufSize / buf / pc were 
used.  Some form of - 8 should be applied to bufSize.  I have attached 
the patch that I plan to apply to the DRI tree.  I suspect that it has 
only cosmetic and / or commentary differences from your patch.

Some things have moved around in the DRI tree, so this patch probably 
won't apply to the XFree86 tree.
Index: glxcmds.c
===
RCS file: /cvsroot/dri/xc/xc/lib/GL/glx/glxcmds.c,v
retrieving revision 1.44
diff -u -d -r1.44 glxcmds.c
--- glxcmds.c   25 Jun 2003 00:39:58 -  1.44
+++ glxcmds.c   30 Jun 2003 20:49:15 -
@@ -198,7 +261,7 @@
 GLXContext AllocateGLXContext( Display *dpy )
 {
  GLXContext gc;
- int bufSize = XMaxRequestSize(dpy) * 4;
+ int bufSize;
  CARD8 opcode;
 
 if (!dpy)
@@ -217,7 +280,14 @@
 }
 memset(gc, 0, sizeof(struct __GLXcontextRec));
 
-/* Allocate transport buffer */
+/*
+** Create a temporary buffer to hold GLX rendering commands.  The size
+** of the buffer is selected so that the maximum number of GLX rendering
+** commands can fit in a single X packet and still have room in the X
+** packed to for the GLXRenderReq header.
+*/
+
+bufSize = (XMaxRequestSize(dpy) * 4) - sz_xGLXRenderReq;
 gc-buf = (GLubyte *) Xmalloc(bufSize);
 if (!gc-buf) {
Xfree(gc);