[Mesa-dev] [PATCH v2] nouveau: fix double free when screen_create fails

[samuel.pitoiset]

On 27/10/2015 11:08, Julien Isorce wrote:
> Looks good though what happened to the del around if (!oclass)  in 
> nv30_screen_create ?

Yeah, good catch. I think we could use FAIL_SCREEN_INIT() there.
I'll send a new patch with this fix.

> Also it seems to fix the other problem around else if (dupfd) which 
> was not closed on failure.

Yes, it fixes both issues.

>
> On 27 October 2015 at 09:10, samuel.pitoiset 
> <samuel.pitoiset at gmail.com <mailto:samuel.pitoiset at gmail.com>> wrote:
>
>     What about this one http://hastebin.com/uboruxicof.coffee ?
>
>     This patch is loosely based on your first attempt, except that I
>     removed the call
>     to nouveau_device_del() in nouveau_drm_screen_create().
>
>
>     On 27/10/2015 09:52, Julien Isorce wrote:
>
>         This patch prevents to call nouveau_device_del twice on the
>         same device.
>
>         Encountered this case when nvc0_screen_create fails with:
>         nvc0_screen_create:717 - Error allocating PGRAPH context for
>         M2MF: -16
>
>         https://bugs.freedesktop.org/show_bug.cgi?id=70354
>
>         Signed-off-by: Julien Isorce <j.isorce at samsung.com
>         <mailto:j.isorce at samsung.com>>
>         ---
>           src/gallium/drivers/nouveau/nouveau_screen.c   | 8 ++++++--
>           src/gallium/drivers/nouveau/nv30/nv30_screen.c | 1 +
>           src/gallium/drivers/nouveau/nv50/nv50_screen.c | 1 +
>           src/gallium/drivers/nouveau/nvc0/nvc0_screen.c | 1 +
>           4 files changed, 9 insertions(+), 2 deletions(-)
>
>         diff --git a/src/gallium/drivers/nouveau/nouveau_screen.c
>         b/src/gallium/drivers/nouveau/nouveau_screen.c
>         index b2290e7..2bd6d4f 100644
>         --- a/src/gallium/drivers/nouveau/nouveau_screen.c
>         +++ b/src/gallium/drivers/nouveau/nouveau_screen.c
>         @@ -177,13 +177,17 @@ nouveau_screen_init(struct
>         nouveau_screen *screen, struct nouveau_device *dev)
>                 screen->device = dev;
>                 ret = nouveau_client_new(screen->device, &screen->client);
>         -       if (ret)
>         +       if (ret) {
>         +               screen->device = 0;
>                         return ret;
>         +       }
>                 ret = nouveau_pushbuf_new(screen->client, screen->channel,
>                                           4, 512 * 1024, 1,
>         &screen->pushbuf);
>         -       if (ret)
>         +       if (ret) {
>         +               screen->device = 0;
>                         return ret;
>         +       }
>                     /* getting CPU time first appears to be more
>         accurate */
>                   screen->cpu_gpu_time_delta = os_time_get();
>         diff --git a/src/gallium/drivers/nouveau/nv30/nv30_screen.c
>         b/src/gallium/drivers/nouveau/nv30/nv30_screen.c
>         index 0330164..54af655 100644
>         --- a/src/gallium/drivers/nouveau/nv30/nv30_screen.c
>         +++ b/src/gallium/drivers/nouveau/nv30/nv30_screen.c
>         @@ -411,6 +411,7 @@ nv30_screen_destroy(struct pipe_screen
>         *pscreen)
>           #define FAIL_SCREEN_INIT(str, err)   \
>              do {  \
>                 NOUVEAU_ERR(str, err);   \
>         +      screen->base.device = 0;     \
>                 nv30_screen_destroy(pscreen);  \
>                 return NULL;   \
>              } while(0)
>         diff --git a/src/gallium/drivers/nouveau/nv50/nv50_screen.c
>         b/src/gallium/drivers/nouveau/nv50/nv50_screen.c
>         index ec51d00..a1fad42 100644
>         --- a/src/gallium/drivers/nouveau/nv50/nv50_screen.c
>         +++ b/src/gallium/drivers/nouveau/nv50/nv50_screen.c
>         @@ -905,6 +905,7 @@ nv50_screen_create(struct nouveau_device *dev)
>              return pscreen;
>             fail:
>         +   screen->base.device = 0;
>              nv50_screen_destroy(pscreen);
>              return NULL;
>           }
>         diff --git a/src/gallium/drivers/nouveau/nvc0/nvc0_screen.c
>         b/src/gallium/drivers/nouveau/nvc0/nvc0_screen.c
>         index af8e5f7..28fee35 100644
>         --- a/src/gallium/drivers/nouveau/nvc0/nvc0_screen.c
>         +++ b/src/gallium/drivers/nouveau/nvc0/nvc0_screen.c
>         @@ -609,6 +609,7 @@ nvc0_screen_resize_tls_area(struct
>         nvc0_screen *screen,
>           #define FAIL_SCREEN_INIT(str, err)   \
>              do {  \
>                 NOUVEAU_ERR(str, err);   \
>         +      screen->base.device = 0;     \
>                 nvc0_screen_destroy(pscreen);  \
>                 return NULL;   \
>              } while(0)
>
>
>

-------------- next part --------------
An HTML attachment was scrubbed...
URL: <http://lists.freedesktop.org/archives/mesa-dev/attachments/20151027/cad7ab98/attachment-0001.html>