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

[Julien Isorce]

Looks good though what happened to the del around if (!oclass)  in
nv30_screen_create ?
Also it seems to fix the other problem around else if (dupfd) which was not
closed on failure.

On 27 October 2015 at 09:10, samuel.pitoiset <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>
>> ---
>>   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/d7fd379f/attachment.html>