[Mesa-dev] [PATCH] nouveau: Use dup fd as key in drm-winsys hash table to fix ZaphodHeads.

[Ilia Mirkin]

This scheme is copied from radeon, does it need a similar fix? I'm away
from computers for another week or so, will be able to look then.
On Jun 5, 2015 4:37 PM, "Mario Kleiner" <mario.kleiner.de at gmail.com> wrote:

> The dup'ed fd owned by the nouveau_screen for a device node
> must also be used as key for the winsys hash table, instead
> of using the original fd passed in for a screen, to make
> multi-x-screen ZaphodHeads configurations work on nouveau.
>
> This prevents the following crash scenario that was observed
> when a dynamically loaded rendering plugin used OpenGL on a
> ZaphodHeads setup, e.g., on a dual x-screen setup. At first
> load the plugin worked, but after unloading and reloading it,
> the next rendering operation crashed:
>
> 1. Client, e.g., a plugin, calls glXQueryVersion.
>
> 2. DRI screens for the x-screens 0 and 1 are created, one shared
>    nouveau_screen is created for the shared device node of both
>    screens, but the original fd of x-screen 0 is used as identifying
>    key in the hash table, instead of the dup()ed fd of x-screen 0
>    which is owned by the nouveau_screen. nouveau_screen's refcount
>    is now 2.
>
> 3. Regular rendering happens by the client plugin, then the plugin
>    gets unloaded.
>
> 4. XCloseDisplay(). x-screen 0 gets its DRI screen destroyed,
>    nouveau_drm_screen_unref() drops the refcount to 1, calling mesa
>    code then closes the fd of x-screen 0, so now the fd which is
>    used as key in the hash table is invalid. x-screen 1 gets
>    destroyed, nouveau_drm_screen_unref() drops the refcount to 0,
>    the nouveau_screen gets destroyed, but removal of its entry
>    in the hash table fails, because the invalid fd in the hash
>    table no longer matches anything (fstat() on the fd is used
>    for hashing and key comparison, but fstat() on an already closed
>    fd fails and returns bogus results). x-screen 1 closes its fd.
>
>    Now all fd's are closed, the nouveau_screen destroyed, but
>    there is a dangling reference to the nouveau_screen in the
>    hash table.
>
> 5. Some OpenGL client plugin gets loaded again and calls
>    glXQueryVersion. Step 2 above repeats, but because a
>    dangling reference with a matching fd is found in the winsys
>    hash table, no new nouveau_screen is created this time. Instead
>    the invalid pointer to the old nouveau_screen is recycled,
>    which points to nirvana -> Crash.
>
> This problem is avoided by use of the dup()ed fd which is
> owned by the nouveau_screen and has the same lifetime as
> the nouveau_screen itself.
>
> Cc: "10.3 10.4 10.5 10.6" <mesa-stable at lists.freedesktop.org>
>
> Signed-off-by: Mario Kleiner <mario.kleiner.de at gmail.com>
> Cc: Ilia Mirkin <imirkin at alum.mit.edu>
> ---
>  src/gallium/winsys/nouveau/drm/nouveau_drm_winsys.c | 3 ++-
>  1 file changed, 2 insertions(+), 1 deletion(-)
>
> diff --git a/src/gallium/winsys/nouveau/drm/nouveau_drm_winsys.c
> b/src/gallium/winsys/nouveau/drm/nouveau_drm_winsys.c
> index 0635246..dbc3cae 100644
> --- a/src/gallium/winsys/nouveau/drm/nouveau_drm_winsys.c
> +++ b/src/gallium/winsys/nouveau/drm/nouveau_drm_winsys.c
> @@ -120,7 +120,8 @@ nouveau_drm_screen_create(int fd)
>         if (!screen)
>                 goto err;
>
> -       util_hash_table_set(fd_tab, intptr_to_pointer(fd), screen);
> +       /* Use dupfd in hash table, to avoid crashes in ZaphodHeads
> configs */
> +       util_hash_table_set(fd_tab, intptr_to_pointer(dupfd), screen);
>         screen->refcount = 1;
>         pipe_mutex_unlock(nouveau_screen_mutex);
>         return &screen->base;
> --
> 2.1.4
>
>
-------------- next part --------------
An HTML attachment was scrubbed...
URL: <http://lists.freedesktop.org/archives/mesa-dev/attachments/20150605/1759a948/attachment.html>