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

[Emil Velikov]

On 14 July 2016 at 15:23, Eric Engestrom <eric.engestrom at imgtec.com> wrote:
> 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.
>
Indeed, reaching {U,}INT_MAX is extremely unlikely, thus we can avoid
adding extra checks.

> Reviewed-by: Eric Engestrom <eric.engestrom at imgtec.com>
>
I'll add the stable tag and push this in a few minutes (as the fresh
doze of coffee kicks in).

Stefan, I'll double-check about the issue mentioned in the cover
letter and let you know (and/or send patches).

Thanks guys !
Emil