Groups | Search | Server Info | Keyboard shortcuts | Login | Register [http] [https] [nntp] [nntps]

Groups > linux.kernel > #1540576 > unrolled thread

[PATCH] drm/msm: return fence_fd = -1 if gem_submit fails

Back to article view | Back to linux.kernel

Contents

  [PATCH] drm/msm: return fence_fd = -1 if gem_submit fails Gustavo Padovan <gustavo@padovan.org> - 2016-12-12 20:50 +0100
    Re: [PATCH] drm/msm: return fence_fd = -1 if gem_submit fails Chris Wilson <chris@chris-wilson.co.uk> - 2016-12-12 21:50 +0100
      Re: [PATCH] drm/msm: return fence_fd = -1 if gem_submit fails Gustavo Padovan <gustavo.padovan@collabora.com> - 2016-12-12 22:30 +0100

#1540576 — [PATCH] drm/msm: return fence_fd = -1 if gem_submit fails

From: Gustavo Padovan <gustavo.padovan@collabora.co.uk>

Previously we were returning garbage here, fix it by setting it to -1
before the first possible point of failure.

Signed-off-by: Gustavo Padovan <gustavo.padovan@collabora.co.uk>
---
 drivers/gpu/drm/msm/msm_gem_submit.c | 5 ++++-
 1 file changed, 4 insertions(+), 1 deletion(-)

diff --git a/drivers/gpu/drm/msm/msm_gem_submit.c b/drivers/gpu/drm/msm/msm_gem_submit.c
index 166e84e..c102b55 100644
--- a/drivers/gpu/drm/msm/msm_gem_submit.c
+++ b/drivers/gpu/drm/msm/msm_gem_submit.c
@@ -383,10 +383,13 @@ int msm_ioctl_gem_submit(struct drm_device *dev, void *data,
 	struct msm_gpu *gpu = priv->gpu;
 	struct dma_fence *in_fence = NULL;
 	struct sync_file *sync_file = NULL;
+	int in_fence_fd = args->fence_fd;
 	int out_fence_fd = -1;
 	unsigned i;
 	int ret;
 
+	args->fence_fd = -1;
+
 	if (!gpu)
 		return -ENXIO;
 
@@ -427,7 +430,7 @@ int msm_ioctl_gem_submit(struct drm_device *dev, void *data,
 		goto out;
 
 	if (args->flags & MSM_SUBMIT_FENCE_FD_IN) {
-		in_fence = sync_file_get_fence(args->fence_fd);
+		in_fence = sync_file_get_fence(in_fence_fd);
 
 		if (!in_fence) {
 			ret = -EINVAL;
-- 
2.5.5

[toc] | [next] | [standalone]

#1540638

On Mon, Dec 12, 2016 at 05:41:08PM -0200, Gustavo Padovan wrote:
> From: Gustavo Padovan <gustavo.padovan@collabora.co.uk>
> 
> Previously we were returning garbage here, fix it by setting it to -1
> before the first possible point of failure.

The convention is that on error paths you do not modify user inputs. In
particular, consider EINTR where the usual pattern (e.g. drmIoctl) is

	do {
		err = ioctl(fd, SUBMIT, arg);
	} while (err == -EINTR);

If you modify the in fence before you consume it, you can't recreate it
after handling the signal.
-Chris

-- 
Chris Wilson, Intel Open Source Technology Centre

[toc] | [prev] | [next] | [standalone]

#1540678

2016-12-12 Chris Wilson <chris@chris-wilson.co.uk>:

> On Mon, Dec 12, 2016 at 05:41:08PM -0200, Gustavo Padovan wrote:
> > From: Gustavo Padovan <gustavo.padovan@collabora.co.uk>
> > 
> > Previously we were returning garbage here, fix it by setting it to -1
> > before the first possible point of failure.
> 
> The convention is that on error paths you do not modify user inputs. In
> particular, consider EINTR where the usual pattern (e.g. drmIoctl) is
> 
> 	do {
> 		err = ioctl(fd, SUBMIT, arg);
> 	} while (err == -EINTR);
> 
> If you modify the in fence before you consume it, you can't recreate it
> after handling the signal.

Right. I didn't know about that convention. So maybe we let it as is. :)

Gustavo

[toc] | [prev] | [standalone]

Back to top | Article view | linux.kernel

csiph-web