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

2015-10-27 Thread 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 
mailto:samuel.pitoi...@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 mailto:j.iso...@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)





___
mesa-dev mailing list
mesa-dev@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/mesa-dev

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

2015-10-27 Thread 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 
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 
>> ---
>>   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)
>>
>
>
___
mesa-dev mailing list
mesa-dev@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/mesa-dev

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

2015-10-27 Thread samuel.pitoiset 

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 
---
  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)


___
mesa-dev mailing list
mesa-dev@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/mesa-dev

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

2015-10-27 Thread Julien Isorce 
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 
---
 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)
-- 
1.9.1

___
mesa-dev mailing list
mesa-dev@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/mesa-dev