[PATCH] libXrender: avoid possible overflow with multiple members
Julien Cristau
jcristau at debian.org
Mon May 27 08:02:09 PDT 2013
Hi Dave,
On Mon, May 27, 2013 at 08:56:34 +1000, Dave Airlie wrote:
> From: Dave Airlie <airlied at redhat.com>
>
> If all of these limits are pushed to their mask, then / 4 won't stop
I assume s/mask/max/
> the malloc from being overflowed.
>
> Signed-off-by: Dave Airlie <airlied at redhat.com>
> ---
> src/Xrender.c | 10 +++++-----
> 1 file changed, 5 insertions(+), 5 deletions(-)
>
> diff --git a/src/Xrender.c b/src/Xrender.c
> index 3102eb2..1c859ea 100644
> --- a/src/Xrender.c
> +++ b/src/Xrender.c
> @@ -459,11 +459,11 @@ XRenderQueryFormats (Display *dpy)
> if (async_state.major_version == 0 && async_state.minor_version < 6)
> rep.numSubpixel = 0;
>
> - if ((rep.numFormats < ((INT_MAX / 4) / sizeof (XRenderPictFormat))) &&
> - (rep.numScreens < ((INT_MAX / 4) / sizeof (XRenderScreen))) &&
> - (rep.numDepths < ((INT_MAX / 4) / sizeof (XRenderDepth))) &&
> - (rep.numVisuals < ((INT_MAX / 4) / sizeof (XRenderVisual))) &&
> - (rep.numSubpixel < ((INT_MAX / 4) / 4)) &&
> + if ((rep.numFormats < ((INT_MAX / 8) / sizeof (XRenderPictFormat))) &&
> + (rep.numScreens < ((INT_MAX / 8) / sizeof (XRenderScreen))) &&
> + (rep.numDepths < ((INT_MAX / 8) / sizeof (XRenderDepth))) &&
> + (rep.numVisuals < ((INT_MAX / 8) / sizeof (XRenderVisual))) &&
> + (rep.numSubpixel < ((INT_MAX / 8) / 4)) &&
> (rep.length < (INT_MAX >> 2)) ) {
> xri = Xmalloc (sizeof (XRenderInfo) +
> (rep.numFormats * sizeof (XRenderPictFormat)) +
I don't see the overflow here.
Worst case is each of these is (INT_MAX / 4) - 1.
The first malloc would get 4 * ((INT_MAX / 4) - 1) + sizeof(XRenderInfo)
which is still < UINT_MAX <= SIZE_MAX.
sizeof(XRenderPictFormat) > sizeof(xPictFormInfo)
sizeof(XRenderScreen) > sizeof(xPictScreen)
sizeof(XRenderDepth) > sizeof(xPictDepth)
sizeof(XRenderVisual) > sizeof (xPictVisual)
So the second malloc gets at most 5 * ((INT_MAX / 4) - 1), which
shouldn't overflow UINT_MAX either?
There's an argument for making this all more obviously correct, in any
case...
Cheers,
Julien
More information about the xorg-devel
mailing list