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...@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-sta...@lists.freedesktop.org> > > Signed-off-by: Mario Kleiner <mario.kleiner...@gmail.com> > Cc: Ilia Mirkin <imir...@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 > >
_______________________________________________ mesa-dev mailing list mesa-dev@lists.freedesktop.org http://lists.freedesktop.org/mailman/listinfo/mesa-dev