[Mesa-dev] [PATCH glx/glxglvnd] Avoid overflow in 'last' variable of FindGLXFunction(...)

[Eric Engestrom]

On Thu, Jul 14, 2016 at 03:21:20PM +0200, Stefan Dirsch wrote:
> This 'last' variable used in FindGLXFunction(...) may become negative,
> but has been defined as unsigned int resulting in an overflow,
> finally resulting in a segfault when accessing _glXDispatchTableStrings[...].
> Fixed this by definining it as signed int. 'first' variable also needs to be
> defined as signed int. Otherwise condition for while loop fails due to C
> implicitly converting signed to unsigned values before comparison.

Indeed, `last` can become negative is when the name searched for is
alphabetically less than the first entry in the dispatch table.
On the penultimate round, we would have `first = 0` and `last = 1`.
Next iteration of the while loop, middle becomes 0, `strcmp() > 0`
and last = middle - 1, ie. -1.

The same issue exists on the other side (name searched is after last
entry), but until DI_FUNCTION_COUNT reaches UINT_MAX this wouldn't
wrap around.

It's unlikely we'll ever have more than INT_MAX entries in the dispatch
table, so I think this patch is OK. I tried to find a better fix, but
adding checks before updating first and last feels too heavy.

Reviewed-by: Eric Engestrom <eric.engestrom at imgtec.com>

> 
> Signed-off-by: Stefan Dirsch <sndirsch at suse.de>
> ---
>  src/glx/glxglvnd.c | 6 +++---
>  1 file changed, 3 insertions(+), 3 deletions(-)
> 
> diff --git a/src/glx/glxglvnd.c b/src/glx/glxglvnd.c
> index b7252a7..962eda8 100644
> --- a/src/glx/glxglvnd.c
> +++ b/src/glx/glxglvnd.c
> @@ -19,11 +19,11 @@ static void *__glXGLVNDGetProcAddress(const GLubyte *procName)
>  
>  static unsigned FindGLXFunction(const GLubyte *name)
>  {
> -    unsigned first = 0;
> -    unsigned last = DI_FUNCTION_COUNT - 1;
> +    int first = 0;
> +    int last = DI_FUNCTION_COUNT - 1;
>  
>      while (first <= last) {
> -        unsigned middle = (first + last) / 2;
> +        int middle = (first + last) / 2;
>          int comp = strcmp((const char *) name,
>                            __glXDispatchTableStrings[middle]);
>  
> -- 
> 2.6.6