diff mbox series

drm/panfrost: Add sanity checks to submit IOCTL

Message ID 20190424131355.62817-1-tomeu.vizoso@collabora.com
State Accepted
Commit 6ff408e6dc2072adc3e27d7687dfba19edb05d60
Headers show
Series drm/panfrost: Add sanity checks to submit IOCTL | expand

Commit Message

Tomeu Vizoso April 24, 2019, 1:13 p.m. UTC
So userspace can get feedback on any error conditions, instead of going
ahead and things breaking later.

Signed-off-by: Tomeu Vizoso <tomeu.vizoso@collabora.com>

---
 drivers/gpu/drm/panfrost/panfrost_drv.c | 35 +++++++++++++++++--------
 1 file changed, 24 insertions(+), 11 deletions(-)

-- 
2.20.1

Comments

Daniel Vetter April 24, 2019, 1:20 p.m. UTC | #1
On Wed, Apr 24, 2019 at 03:13:53PM +0200, Tomeu Vizoso wrote:
> So userspace can get feedback on any error conditions, instead of going

> ahead and things breaking later.

> 

> Signed-off-by: Tomeu Vizoso <tomeu.vizoso@collabora.com>


Bonus: some igts to exercise these corner cases. It helps a lot with uapi
review and testing.
-Daniel

> ---

>  drivers/gpu/drm/panfrost/panfrost_drv.c | 35 +++++++++++++++++--------

>  1 file changed, 24 insertions(+), 11 deletions(-)

> 

> diff --git a/drivers/gpu/drm/panfrost/panfrost_drv.c b/drivers/gpu/drm/panfrost/panfrost_drv.c

> index c06af78ab833..0f2863cb8077 100644

> --- a/drivers/gpu/drm/panfrost/panfrost_drv.c

> +++ b/drivers/gpu/drm/panfrost/panfrost_drv.c

> @@ -172,13 +172,27 @@ static int panfrost_ioctl_submit(struct drm_device *dev, void *data,

>  {

>  	struct panfrost_device *pfdev = dev->dev_private;

>  	struct drm_panfrost_submit *args = data;

> -	struct drm_syncobj *sync_out;

> +	struct drm_syncobj *sync_out = NULL;

>  	struct panfrost_job *job;

>  	int ret = 0;

>  

> +	if (!args->jc)

> +		return -EINVAL;

> +

> +	if (args->requirements && args->requirements != PANFROST_JD_REQ_FS)

> +		return -EINVAL;

> +

> +	if (args->out_sync > 0) {

> +		sync_out = drm_syncobj_find(file, args->out_sync);

> +		if (!sync_out)

> +			return -ENODEV;

> +	}

> +

>  	job = kzalloc(sizeof(*job), GFP_KERNEL);

> -	if (!job)

> -		return -ENOMEM;

> +	if (!job) {

> +		ret = -ENOMEM;

> +		goto fail_out_sync;

> +	}

>  

>  	kref_init(&job->refcount);

>  

> @@ -190,25 +204,24 @@ static int panfrost_ioctl_submit(struct drm_device *dev, void *data,

>  

>  	ret = panfrost_copy_in_sync(dev, file, args, job);

>  	if (ret)

> -		goto fail;

> +		goto fail_job;

>  

>  	ret = panfrost_lookup_bos(dev, file, args, job);

>  	if (ret)

> -		goto fail;

> +		goto fail_job;

>  

>  	ret = panfrost_job_push(job);

>  	if (ret)

> -		goto fail;

> +		goto fail_job;

>  

>  	/* Update the return sync object for the job */

> -	sync_out = drm_syncobj_find(file, args->out_sync);

> -	if (sync_out) {

> +	if (sync_out)

>  		drm_syncobj_replace_fence(sync_out, job->render_done_fence);

> -		drm_syncobj_put(sync_out);

> -	}

>  

> -fail:

> +fail_job:

>  	panfrost_job_put(job);

> +fail_out_sync:

> +	drm_syncobj_put(sync_out);

>  

>  	return ret;

>  }

> -- 

> 2.20.1

> 


-- 
Daniel Vetter
Software Engineer, Intel Corporation
http://blog.ffwll.ch
diff mbox series

Patch

diff --git a/drivers/gpu/drm/panfrost/panfrost_drv.c b/drivers/gpu/drm/panfrost/panfrost_drv.c
index c06af78ab833..0f2863cb8077 100644
--- a/drivers/gpu/drm/panfrost/panfrost_drv.c
+++ b/drivers/gpu/drm/panfrost/panfrost_drv.c
@@ -172,13 +172,27 @@  static int panfrost_ioctl_submit(struct drm_device *dev, void *data,
 {
 	struct panfrost_device *pfdev = dev->dev_private;
 	struct drm_panfrost_submit *args = data;
-	struct drm_syncobj *sync_out;
+	struct drm_syncobj *sync_out = NULL;
 	struct panfrost_job *job;
 	int ret = 0;
 
+	if (!args->jc)
+		return -EINVAL;
+
+	if (args->requirements && args->requirements != PANFROST_JD_REQ_FS)
+		return -EINVAL;
+
+	if (args->out_sync > 0) {
+		sync_out = drm_syncobj_find(file, args->out_sync);
+		if (!sync_out)
+			return -ENODEV;
+	}
+
 	job = kzalloc(sizeof(*job), GFP_KERNEL);
-	if (!job)
-		return -ENOMEM;
+	if (!job) {
+		ret = -ENOMEM;
+		goto fail_out_sync;
+	}
 
 	kref_init(&job->refcount);
 
@@ -190,25 +204,24 @@  static int panfrost_ioctl_submit(struct drm_device *dev, void *data,
 
 	ret = panfrost_copy_in_sync(dev, file, args, job);
 	if (ret)
-		goto fail;
+		goto fail_job;
 
 	ret = panfrost_lookup_bos(dev, file, args, job);
 	if (ret)
-		goto fail;
+		goto fail_job;
 
 	ret = panfrost_job_push(job);
 	if (ret)
-		goto fail;
+		goto fail_job;
 
 	/* Update the return sync object for the job */
-	sync_out = drm_syncobj_find(file, args->out_sync);
-	if (sync_out) {
+	if (sync_out)
 		drm_syncobj_replace_fence(sync_out, job->render_done_fence);
-		drm_syncobj_put(sync_out);
-	}
 
-fail:
+fail_job:
 	panfrost_job_put(job);
+fail_out_sync:
+	drm_syncobj_put(sync_out);
 
 	return ret;
 }