nouveau: fix screen creation failure paths

The winsys layer would attempt to cleanup the nouveau_device if screen
init failed, however, in most paths the pipe driver would have already
destroyed it, resulting in accesses to freed memory etc.

This commit fixes the problem by allowing the winsys to detect whether
the pipe driver's destroy function needs to be called or not.

Signed-off-by: Ben Skeggs <bskeggs@redhat.com>
Reviewed-by: Emil Velikov <emil.l.velikov@gmail.com>
Reviewed-by: Samuel Pitoiset <samuel.pitoiset@gmail.com>
Tested-by: Samuel Pitoiset <samuel.pitoiset@gmail.com>

diff --git a/src/gallium/drivers/nouveau/nouveau_screen.c b/src/gallium/drivers/nouveau/nouveau_screen.c
index a012579a9bda14dfaf1fe8adb879bc291c976ba1..3cdcc2066a4fafec6661aaae44d211ca1df4a1a6 100644 (file)
--- a/src/gallium/drivers/nouveau/nouveau_screen.c
+++ b/src/gallium/drivers/nouveau/nouveau_screen.c
@@ -147,6 +147,12 @@ nouveau_screen_init(struct nouveau_screen *screen, struct nouveau_device *dev)
    if (nv_dbg)
       nouveau_mesa_debug = atoi(nv_dbg);
 
+   /* These must be set before any failure is possible, as the cleanup
+    * paths assume they're responsible for deleting them.
+    */
+   screen->drm = nouveau_drm(&dev->object);
+   screen->device = dev;
+
    /*
     * this is initialized to 1 in nouveau_drm_screen_create after screen
     * is fully constructed and added to the global screen list.
@@ -175,8 +181,6 @@ nouveau_screen_init(struct nouveau_screen *screen, struct nouveau_device *dev)
                             data, size, &screen->channel);
    if (ret)
       return ret;
-   screen->drm = nouveau_drm(&dev->object);
-   screen->device = dev;
 
    ret = nouveau_client_new(screen->device, &screen->client);
    if (ret)

diff --git a/src/gallium/drivers/nouveau/nv30/nv30_screen.c b/src/gallium/drivers/nouveau/nv30/nv30_screen.c
index ea29811aa46415dcf9f35a91df23a894673bd96f..854f70cf34cfa0022b71f2157a107e8e5f6f1e87 100644 (file)
--- a/src/gallium/drivers/nouveau/nv30/nv30_screen.c
+++ b/src/gallium/drivers/nouveau/nv30/nv30_screen.c
@@ -413,23 +413,20 @@ nv30_screen_destroy(struct pipe_screen *pscreen)
 #define FAIL_SCREEN_INIT(str, err)                    \
    do {                                               \
       NOUVEAU_ERR(str, err);                          \
-      nv30_screen_destroy(pscreen);                   \
-      return NULL;                                    \
+      screen->base.base.context_create = NULL;        \
+      return &screen->base;                           \
    } while(0)
 
 struct nouveau_screen *
 nv30_screen_create(struct nouveau_device *dev)
 {
-   struct nv30_screen *screen = CALLOC_STRUCT(nv30_screen);
+   struct nv30_screen *screen;
    struct pipe_screen *pscreen;
    struct nouveau_pushbuf *push;
    struct nv04_fifo *fifo;
    unsigned oclass = 0;
    int ret, i;
 
-   if (!screen)
-      return NULL;
-
    switch (dev->chipset & 0xf0) {
    case 0x30:
       if (RANKINE_0397_CHIPSET & (1 << (dev->chipset & 0x0f)))
@@ -458,10 +455,16 @@ nv30_screen_create(struct nouveau_device *dev)
 
    if (!oclass) {
       NOUVEAU_ERR("unknown 3d class for 0x%02x\n", dev->chipset);
-      FREE(screen);
       return NULL;
    }
 
+   screen = CALLOC_STRUCT(nv30_screen);
+   if (!screen)
+      return NULL;
+
+   pscreen = &screen->base.base;
+   pscreen->destroy = nv30_screen_destroy;
+
    /*
     * Some modern apps try to use msaa without keeping in mind the
     * restrictions on videomem of older cards. Resulting in dmesg saying:
@@ -479,8 +482,6 @@ nv30_screen_create(struct nouveau_device *dev)
    if (screen->max_sample_count > 4)
       screen->max_sample_count = 4;
 
-   pscreen = &screen->base.base;
-   pscreen->destroy = nv30_screen_destroy;
    pscreen->get_param = nv30_screen_get_param;
    pscreen->get_paramf = nv30_screen_get_paramf;
    pscreen->get_shader_param = nv30_screen_get_shader_param;

diff --git a/src/gallium/drivers/nouveau/nv50/nv50_screen.c b/src/gallium/drivers/nouveau/nv50/nv50_screen.c
index e9e56e707f33f2ce4e1b87e8b554363ef943c8e9..272e1d45bff76fda63097fc72acc97e418a2b60a 100644 (file)
--- a/src/gallium/drivers/nouveau/nv50/nv50_screen.c
+++ b/src/gallium/drivers/nouveau/nv50/nv50_screen.c
@@ -767,6 +767,7 @@ nv50_screen_create(struct nouveau_device *dev)
    if (!screen)
       return NULL;
    pscreen = &screen->base.base;
+   pscreen->destroy = nv50_screen_destroy;
 
    ret = nouveau_screen_init(&screen->base, dev);
    if (ret) {
@@ -787,7 +788,6 @@ nv50_screen_create(struct nouveau_device *dev)
 
    chan = screen->base.channel;
 
-   pscreen->destroy = nv50_screen_destroy;
    pscreen->context_create = nv50_create;
    pscreen->is_format_supported = nv50_screen_is_format_supported;
    pscreen->get_param = nv50_screen_get_param;
@@ -969,8 +969,8 @@ nv50_screen_create(struct nouveau_device *dev)
    return &screen->base;
 
 fail:
-   nv50_screen_destroy(pscreen);
-   return NULL;
+   screen->base.base.context_create = NULL;
+   return &screen->base;
 }
 
 int

diff --git a/src/gallium/drivers/nouveau/nvc0/nvc0_screen.c b/src/gallium/drivers/nouveau/nvc0/nvc0_screen.c
index b573ada4ba6054267abac05d868bf46e5fc57d98..b5d08306596936b6573020dc8b1207222514f40b 100644 (file)
--- a/src/gallium/drivers/nouveau/nvc0/nvc0_screen.c
+++ b/src/gallium/drivers/nouveau/nvc0/nvc0_screen.c
@@ -618,8 +618,7 @@ nvc0_screen_resize_tls_area(struct nvc0_screen *screen,
 #define FAIL_SCREEN_INIT(str, err)                    \
    do {                                               \
       NOUVEAU_ERR(str, err);                          \
-      nvc0_screen_destroy(pscreen);                   \
-      return NULL;                                    \
+      goto fail;                                      \
    } while(0)
 
 struct nouveau_screen *
@@ -651,6 +650,7 @@ nvc0_screen_create(struct nouveau_device *dev)
    if (!screen)
       return NULL;
    pscreen = &screen->base.base;
+   pscreen->destroy = nvc0_screen_destroy;
 
    ret = nouveau_screen_init(&screen->base, dev);
    if (ret) {
@@ -673,7 +673,6 @@ nvc0_screen_create(struct nouveau_device *dev)
       screen->base.vidmem_bindings = 0;
    }
 
-   pscreen->destroy = nvc0_screen_destroy;
    pscreen->context_create = nvc0_create;
    pscreen->is_format_supported = nvc0_screen_is_format_supported;
    pscreen->get_param = nvc0_screen_get_param;
@@ -1066,8 +1065,8 @@ nvc0_screen_create(struct nouveau_device *dev)
    return &screen->base;
 
 fail:
-   nvc0_screen_destroy(pscreen);
-   return NULL;
+   screen->base.base.context_create = NULL;
+   return &screen->base;
 }
 
 int

diff --git a/src/gallium/winsys/nouveau/drm/nouveau_drm_winsys.c b/src/gallium/winsys/nouveau/drm/nouveau_drm_winsys.c
index e117dfcd0095087bfd94bc5f8a3b277c4348476e..456530d7427159726a6345deb46b82a1d8bd3ed6 100644 (file)
--- a/src/gallium/winsys/nouveau/drm/nouveau_drm_winsys.c
+++ b/src/gallium/winsys/nouveau/drm/nouveau_drm_winsys.c
@@ -59,7 +59,7 @@ nouveau_drm_screen_create(int fd)
 {
        struct nouveau_device *dev = NULL;
        struct nouveau_screen *(*init)(struct nouveau_device *);
-       struct nouveau_screen *screen;
+       struct nouveau_screen *screen = NULL;
        int ret, dupfd = -1;
 
        pipe_mutex_lock(nouveau_screen_mutex);
@@ -117,7 +117,7 @@ nouveau_drm_screen_create(int fd)
        }
 
        screen = init(dev);
-       if (!screen)
+       if (!screen || !screen->base.context_create)
                goto err;
 
        /* Use dupfd in hash table, to avoid errors if the original fd gets
@@ -130,10 +130,14 @@ nouveau_drm_screen_create(int fd)
        return &screen->base;
 
 err:
-       if (dev)
-               nouveau_device_del(&dev);
-       else if (dupfd >= 0)
-               close(dupfd);
+       if (screen) {
+               screen->base.destroy(&screen->base);
+       } else {
+               if (dev)
+                       nouveau_device_del(&dev);
+               else if (dupfd >= 0)
+                       close(dupfd);
+       }
        pipe_mutex_unlock(nouveau_screen_mutex);
        return NULL;
 }