Nouveau - Apr 2015 - [PATCH 2/2] pm: fix a potential race condition when creating an engine context

This fixes a crash when multiple PM engine contexts are created.

Signed-off-by: Samuel Pitoiset <samuel.pitoiset at gmail.com>
Reviewed-by: Martin Peres <martin.peres at free.fr>
---
 drm/nouveau/nvkm/engine/pm/base.c | 7 +++++--
 1 file changed, 5 insertions(+), 2 deletions(-)

diff --git a/drm/nouveau/nvkm/engine/pm/base.c
b/drm/nouveau/nvkm/engine/pm/base.c
index 2006c44..274457c 100644
--- a/drm/nouveau/nvkm/engine/pm/base.c
+++ b/drm/nouveau/nvkm/engine/pm/base.c
@@ -332,9 +332,12 @@ static void
 nvkm_perfctx_dtor(struct nvkm_object *object)
 {
 	struct nvkm_pm *ppm = (void *)object->engine;
+	struct nvkm_perfctx *ctx = (void *)object;
+
 	mutex_lock(&nv_subdev(ppm)->mutex);
-	nvkm_engctx_destroy(&ppm->context->base);
-	ppm->context = NULL;
+	nvkm_engctx_destroy(&ctx->base);
+	if (ppm->context == ctx)
+		ppm->context = NULL;
 	mutex_unlock(&nv_subdev(ppm)->mutex);
 }
 
-- 
2.3.5

There is always the possiblity that the ppm->context pointer would get
partially updated and accidentally would equal ctx. This would allow two
contexts to co-exist, which is not acceptable. Moving the test to the
critical section takes care of this problem.

Signed-off-by: Samuel Pitoiset <samuel.pitoiset at gmail.com>
Signed-off-by: Martin Peres <martin.peres at free.fr>
---
 drm/nouveau/nvkm/engine/pm/base.c | 7 +++----
 1 file changed, 3 insertions(+), 4 deletions(-)

diff --git a/drm/nouveau/nvkm/engine/pm/base.c
b/drm/nouveau/nvkm/engine/pm/base.c
index 274457c..4cf36a3 100644
--- a/drm/nouveau/nvkm/engine/pm/base.c
+++ b/drm/nouveau/nvkm/engine/pm/base.c
@@ -358,12 +358,11 @@ nvkm_perfctx_ctor(struct nvkm_object *parent, struct
nvkm_object *engine,
 	mutex_lock(&nv_subdev(ppm)->mutex);
 	if (ppm->context == NULL)
 		ppm->context = ctx;
-	mutex_unlock(&nv_subdev(ppm)->mutex);
-
 	if (ctx != ppm->context)
-		return -EBUSY;
+		ret = -EBUSY;
+	mutex_unlock(&nv_subdev(ppm)->mutex);
 
-	return 0;
+	return ret;
 }
 
 struct nvkm_oclass
-- 
2.3.5

* bump *

Ben, could you take a look at the series ?

On 04/14/2015 10:43 PM, Samuel Pitoiset wrote:
> There is always the possiblity that the ppm->context pointer would get
> partially updated and accidentally would equal ctx. This would allow two
> contexts to co-exist, which is not acceptable. Moving the test to the
> critical section takes care of this problem.
>
> Signed-off-by: Samuel Pitoiset <samuel.pitoiset at gmail.com>
> Signed-off-by: Martin Peres <martin.peres at free.fr>
> ---
>   drm/nouveau/nvkm/engine/pm/base.c | 7 +++----
>   1 file changed, 3 insertions(+), 4 deletions(-)
>
> diff --git a/drm/nouveau/nvkm/engine/pm/base.c
b/drm/nouveau/nvkm/engine/pm/base.c
> index 274457c..4cf36a3 100644
> --- a/drm/nouveau/nvkm/engine/pm/base.c
> +++ b/drm/nouveau/nvkm/engine/pm/base.c
> @@ -358,12 +358,11 @@ nvkm_perfctx_ctor(struct nvkm_object *parent, struct
nvkm_object *engine,
>   	mutex_lock(&nv_subdev(ppm)->mutex);
>   	if (ppm->context == NULL)
>   		ppm->context = ctx;
> -	mutex_unlock(&nv_subdev(ppm)->mutex);
> -
>   	if (ctx != ppm->context)
> -		return -EBUSY;
> +		ret = -EBUSY;
> +	mutex_unlock(&nv_subdev(ppm)->mutex);
>   
> -	return 0;
> +	return ret;
>   }
>   
>   struct nvkm_oclass