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

[Samuel Pitoiset]

On 10/22/2015 01:16 AM, Julien Isorce wrote:
> The real fix is in nouveau_drm_winsys.c by setting dev to 0.
> Which means dev's ownership has been passed to previous call.
> Other changes are there to be consistent with what the
> screen_create functions already do on errors.

This actually happens because nouveau_device_del() is (sometimes) called 
twice
when nvXX_screen_create() fails.

I don't really like this solution but I don't have a better one for now, 
I'll think about
that in the next few days. :)

Note that you forgot to call nouveau_device_del() in nvc0_screen_create().

>
> Encountered this crash because nvc0_screen_create sometimes fails with:
> nvc0_screen_create:717 - Error allocating PGRAPH context for M2MF: -16
> Also see: https://bugs.freedesktop.org/show_bug.cgi?id=70354
>
> Signed-off-by: Julien Isorce <j.isorce at samsung.com>
> ---
>   src/gallium/drivers/nouveau/nv30/nv30_screen.c      | 5 ++++-
>   src/gallium/drivers/nouveau/nv50/nv50_screen.c      | 4 +++-
>   src/gallium/winsys/nouveau/drm/nouveau_drm_winsys.c | 2 ++
>   3 files changed, 9 insertions(+), 2 deletions(-)
>
> diff --git a/src/gallium/drivers/nouveau/nv30/nv30_screen.c b/src/gallium/drivers/nouveau/nv30/nv30_screen.c
> index 0330164..9b8ddac 100644
> --- a/src/gallium/drivers/nouveau/nv30/nv30_screen.c
> +++ b/src/gallium/drivers/nouveau/nv30/nv30_screen.c
> @@ -425,8 +425,10 @@ nv30_screen_create(struct nouveau_device *dev)
>      unsigned oclass = 0;
>      int ret, i;
>   
> -   if (!screen)
> +   if (!screen) {
> +      nouveau_device_del(&dev);
>         return NULL;
> +   }
>   
>      switch (dev->chipset & 0xf0) {
>      case 0x30:
> @@ -456,6 +458,7 @@ nv30_screen_create(struct nouveau_device *dev)
>   
>      if (!oclass) {
>         NOUVEAU_ERR("unknown 3d class for 0x%02x\n", dev->chipset);
> +      nouveau_device_del(&dev);
>         FREE(screen);
>         return NULL;
>      }
> diff --git a/src/gallium/drivers/nouveau/nv50/nv50_screen.c b/src/gallium/drivers/nouveau/nv50/nv50_screen.c
> index ec51d00..e9604d5 100644
> --- a/src/gallium/drivers/nouveau/nv50/nv50_screen.c
> +++ b/src/gallium/drivers/nouveau/nv50/nv50_screen.c
> @@ -711,8 +711,10 @@ nv50_screen_create(struct nouveau_device *dev)
>      int ret;
>   
>      screen = CALLOC_STRUCT(nv50_screen);
> -   if (!screen)
> +   if (!screen) {
> +      nouveau_device_del(&dev);
>         return NULL;
> +   }
>      pscreen = &screen->base.base;
>   
>      ret = nouveau_screen_init(&screen->base, dev);
> diff --git a/src/gallium/winsys/nouveau/drm/nouveau_drm_winsys.c b/src/gallium/winsys/nouveau/drm/nouveau_drm_winsys.c
> index c6603e3..bd1d761 100644
> --- a/src/gallium/winsys/nouveau/drm/nouveau_drm_winsys.c
> +++ b/src/gallium/winsys/nouveau/drm/nouveau_drm_winsys.c
> @@ -117,6 +117,8 @@ nouveau_drm_screen_create(int fd)
>   	}
>   
>   	screen = (struct nouveau_screen*)init(dev);
> +	/* Previous init func took ownership of dev */
> +	dev = 0;
>   	if (!screen)
>   		goto err;
>