Hi, Thanks for the feedback!
On Mon, Jul 18, 2011 at 10:06:24AM -0700, Jamey Sharp wrote: > I have a few comments, I guess. > > - The indentation in the GetReqExtra definition didn't work out. Looks > like it should be tabs and tabs only, there. Oh, hrm, sorry about that. I will fix it up. > - This patch turns a buffer overrun into a null-pointer dereference if > there's any caller that hasn't been updated. (While you've hit all the > cases I can find in X.Org sources, keep in mind that there are > closed-source Xlib extensions out there.) I think that's an > improvement, but it's worth calling out in the commit message. True, I will improve the commit message. > - I guess this new behavior should be documented in > specs/libX11/AppC.xml. Okay, noted. > - Your mention of 16K in the commit message confused me because I wasn't > sure where you were getting that limit from. Note that Xlib's buffer > size is actually runtime-configurable, down to a minimum of 2K. Could > you explain it in terms of Xlib's output buffer size, please? That was, perhaps, a bit obscure. I will rephrase this in terms of the Xlib buffer size. > - Changing the implementation of GetReqExtra and the rest of Xlibint.h > is only acceptable if code compiled using the old implementation is > ABI compatible with code compiled using the new implementation. I > believe you're good on that count, but I wanted to point it out. Right -- I tried to be careful with this. > On the whole, I believe this is an improvement. With the above > corrections, I'd be happy to commit this. Thanks, I'll send a v2 shortly. -Kees -- Kees Cook Ubuntu Security Team _______________________________________________ [email protected]: X.Org development Archives: http://lists.x.org/archives/xorg-devel Info: http://lists.x.org/mailman/listinfo/xorg-devel
