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

[Mario Kleiner]

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