in glx/x11/indirect.c (implementation of glAreTexturesResident) line 5112 __glXReadReply is called with reply_is_always_array set to GL_TRUE. This seems to make __glXReadReply to read 4*reply.length bytes of data to the dest pointer. But the residences pointer in glAreTexturesResident is to GLboolean data (single byte) and it can actually point to a single byte of data. At least on my application this manages to corrupt the stack at the level where I have the corresponding variable defined. (and because of stack corruption, this can take some time to debug) Eero
Nice find. The code in question has been as-is for many years so this goes way back. The problem is when the GLX reply is an array, the number of bytes returned is always a multiple of four. If the 'residences' array isn't a multiple of four in size, out-of-bounds writes could occur. The solution is to allocate a temporary buffer which is a multiple of four bytes when necessary. I'm about to commit a fix to Mesa for this. Unfortunately, it involves special-casing the indirect AreTextureResident() function whereas it was auto-generated before.
BTW, I suggest that you code around this problem in your application. This bug is probably in every libGL in existance and it'll be a while before this fix is propagated. Specifically, always use a GLboolean 'residences' array that's a multiple of four in size.
(In reply to comment #1) > Nice find. The code in question has been as-is for many years so this goes way > back. > > The problem is when the GLX reply is an array, the number of bytes returned is > always a multiple of four. If the 'residences' array isn't a multiple of four > in size, out-of-bounds writes could occur. > > The solution is to allocate a temporary buffer which is a multiple of four > bytes when necessary. > > I'm about to commit a fix to Mesa for this. Unfortunately, it involves > special-casing the indirect AreTextureResident() function whereas it was > auto-generated before. > I think I disagree here. I think that the real/main problem here is that if __glXReadReply is given the the last argument "true", it will try to read the reply data using size 4 * reply.length ^^^ (indirect.c line 77). So using your patch wouldn't this cause an overrun of the malloc:ed buffer. (Or does _XRead limit this based on available data or something). I would actually suggest fixing this problem by using the the original code, but just changing the last argument to GL_FALSE for __glXReadReply. (I don't really know if this is ok, but it does not crash, and similar code exists for example in glAreProgramsResidentNV ) (I have done my tests using the Mesa-7.2 source code, Need to find out where dri2tokens.h is before I can compile the git version....) I am taking the liberty to marking this with "Reopen bug", Eero
(In reply to comment #3) > (In reply to comment #1) > I am taking the liberty to marking this with "Reopen bug", > Maybe this time... Eero
Unfortunately, changing reply_is_always_array from GL_TRUE to GL_FALSE doesn't give the right results either. I tried that yesterday. If you write a test program that calls glAreTexturesResident() with n=3, then set a breakpoint in __glXReadReply() you'll see that bytes=1 and extra=3. That's because reply looks like this: {type = 1 '\001', unused = 0 '\0', sequenceNumber = 43, length = 1, retval = 0, size = 1, pad3 = 16, pad4 = 0, pad5 = 0, pad6 = 0} and size=1. So bytes = 1*1. So, changing reply_is_always_array to GL_TRUE will work if n=1, but will not work for n>1.
Err, replace GL_TRUE w/ GL_FALSE in my last sentence there.
(In reply to comment #5) > Unfortunately, changing reply_is_always_array from GL_TRUE to GL_FALSE doesn't > give the right results either. I tried that yesterday. > > If you write a test program that calls glAreTexturesResident() with n=3, then > set a breakpoint in __glXReadReply() you'll see that bytes=1 and extra=3. > > That's because reply looks like this: > > {type = 1 '\001', unused = 0 '\0', sequenceNumber = 43, length = 1, retval = 0, > size = 1, pad3 = 16, pad4 = 0, pad5 = 0, pad6 = 0} and size=1. So bytes = > 1*1. > I am getting even more lost.. (gdb) up #1 0xb7eac675 in __indirect_glAreTexturesResident (n=3, textures=0xbfff5dfc, residences=0xbfff5e00 "") at indirect.c:5112 (gdb) down #0 __glXReadReply (dpy=0xddb3bd8, size=1, dest=0xbfff5e00, reply_is_always_array=0 '\0') at indirect.c:81 (gdb) p bytes $7 = 3 (gdb) p extra $8 = 1 (gdb) p reply $9 = {type = 1 '\001', unused = 0 '\0', sequenceNumber = 47906, length = 1, retval = 0, size = 3, pad3 = 143228600, pad4 = 0, pad5 = 0, pad6 = 0} This is on Mesa-7.2 released code, with the reply_is_always_array change Mesa built for linux-dri-debug Eero
In any case our results agree on the reply.length field, and that is the field used by the code path of your fix. So I won't complain about it any more :-) Eero
Mass version move, cvs -> git
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.