[Mesa-dev] [Mesa-stable] [PATCH] glx: fix crash with bad fbconfig

[Emil Velikov]

Hi gents,

On 30 May 2016 at 10:13, Tapani Pälli <tapani.palli at intel.com> wrote:
> From: Daniel Czarnowski <daniel.czarnowski at intel.com>
>
> GLX documentation states:
>         glXCreateNewContext can generate the following errors: (...)
>         GLXBadFBConfig if config is not a valid GLXFBConfig
>
> Function checks if the given config is a valid config and sets proper
> error code.
>
> Fixes currently crashing glx-fbconfig-bad Piglit test.
>
> Signed-off-by: Matt Roper <matthew.d.roper at intel.com>
> Signed-off-by: Tapani Pälli <tapani.palli at intel.com>
> Cc: "11.2" <mesa-stable at lists.freedesktop.org>
> ---
>  src/glx/glxcmds.c | 27 +++++++++++++++++++++++++++
>  1 file changed, 27 insertions(+)
>
> diff --git a/src/glx/glxcmds.c b/src/glx/glxcmds.c
> index bff01d2..4bc7fc4 100644
> --- a/src/glx/glxcmds.c
> +++ b/src/glx/glxcmds.c
> @@ -1629,8 +1629,35 @@ _X_EXPORT GLXContext
>  glXCreateNewContext(Display * dpy, GLXFBConfig fbconfig,
>                      int renderType, GLXContext shareList, Bool allowDirect)
>  {
> +   int list_size;
>     struct glx_config *config = (struct glx_config *) fbconfig;
>
> +   if (!config)
> +   {
Existing coding style is to put the opening bracket trailing on the
previous line. Same goes for the rest of the patch.

> +       __glXSendError(dpy, GLXBadFBConfig, 0, X_GLXCreateNewContext, false);
> +       return NULL;
> +   }
> +
> +   int screen = XDefaultScreen(dpy);
Use the DefaultScreen macro instead ?

> +   struct glx_config **config_list = (struct glx_config **)
> +      glXGetFBConfigs(dpy, screen, &list_size);
> +
Worth checking (& bail) that list_size is negative ?

> +   int i;
unsigned i...

> +   for (i = 0; i < list_size; i++)
... and cast list_size to unsigned ?

> +   {
> +       if (config_list[i] == config)
> +       {
> +           break;
> +       }
> +   }
Coding style: drop the brackets if there's only a single nested
statement. I.e. the above two pairs can go.

> +   free(config_list);
> +
> +   if (i == list_size)
> +   {
> +       __glXSendError(dpy, GLXBadFBConfig, 0, X_GLXCreateNewContext, false);
> +       return NULL;
> +   }
> +

In general, please don't mix variable declarations and code.

Thanks
Emil