[PATCH xserver] glamor: restore vfunc handlers on init failure

[Hans de Goede]

Hi,

On 03-11-16 09:59, Olivier Fourdan wrote:
> In glamor_init(), if the minimum requirements are not met, glamor may
> fail after setting up its own CloseScreen() and DestroyPixmap()
> routines, leading to a crash when either of the two routines is called
> if glamor failed to complete its initialization, e.g:
>
>   (EE) Backtrace:
>   (EE) 0:  Xwayland (OsSigHandler+0x29)
>   (EE) 1: /lib64/libpthread.so.0 (__restore_rt+0x0)
>   (EE) 2: Xwayland (glamor_sync_close+0x2a)
>   (EE) 3: Xwayland (glamor_close_screen+0x52)
>   (EE) 4: Xwayland (CursorCloseScreen+0x88)
>   (EE) 5: Xwayland (AnimCurCloseScreen+0xa4)
>   (EE) 6: Xwayland (present_close_screen+0x42)
>   (EE) 7: Xwayland (dix_main+0x4f9)
>   (EE) 8: /lib64/libc.so.6 (__libc_start_main+0xf1)
>   (EE) 9:  Xwayland (_start+0x2a)
>
> Restore the previous CloseScreen() and DestroyPixmap() vfunc handlers in
> case of failure when checking for the minimum requirements, so that if
> any of the requirement is not met we don't leave the CloseScreen() and
> DestroyPixmap() from glamor handlers in place.
>
> Bugzilla: https://bugzilla.redhat.com/show_bug.cgi?id=1390018
>
> Signed-off-by: Olivier Fourdan <ofourdan at redhat.com>

Patch looks good to me:

Reviewed-by: Hans de Goede <hdegoede at redhat.com>

Regards,

Hans

> ---
>  glamor/glamor.c | 11 ++++++++---
>  1 file changed, 8 insertions(+), 3 deletions(-)
>
> diff --git a/glamor/glamor.c b/glamor/glamor.c
> index b771832..c54cf3b 100644
> --- a/glamor/glamor.c
> +++ b/glamor/glamor.c
> @@ -470,7 +470,7 @@ glamor_init(ScreenPtr screen, unsigned int flags)
>          LogMessage(X_WARNING,
>                     "glamor%d: Failed to allocate screen private\n",
>                     screen->myNum);
> -        goto fail;
> +        goto free_glamor_private;
>      }
>
>      glamor_set_screen_private(screen, glamor_priv);
> @@ -480,7 +480,7 @@ glamor_init(ScreenPtr screen, unsigned int flags)
>          LogMessage(X_WARNING,
>                     "glamor%d: Failed to allocate pixmap private\n",
>                     screen->myNum);
> -        goto fail;
> +        goto free_glamor_private;
>      }
>
>      if (!dixRegisterPrivateKey(&glamor_gc_private_key, PRIVATE_GC,
> @@ -488,7 +488,7 @@ glamor_init(ScreenPtr screen, unsigned int flags)
>          LogMessage(X_WARNING,
>                     "glamor%d: Failed to allocate gc private\n",
>                     screen->myNum);
> -        goto fail;
> +        goto free_glamor_private;
>      }
>
>      glamor_priv->saved_procs.close_screen = screen->CloseScreen;
> @@ -731,6 +731,11 @@ glamor_init(ScreenPtr screen, unsigned int flags)
>      return TRUE;
>
>   fail:
> +    /* Restore default CloseScreen and DestroyPixmap handlers */
> +    screen->CloseScreen = glamor_priv->saved_procs.close_screen;
> +    screen->DestroyPixmap = glamor_priv->saved_procs.destroy_pixmap;
> +
> + free_glamor_private:
>      free(glamor_priv);
>      glamor_set_screen_private(screen, NULL);
>      return FALSE;
>