Re: [PATCH] drm: remove redundant code form drm_ioc32.c

On 07/02/2015 10:32 AM, Daniel Vetter wrote:

On Wed, Jul 01, 2015 at 12:24:37PM +0300, Jarkko Sakkinen wrote:

...

The compat ioctl handler ends up calling access_ok() twice: first indirectly inside compat_alloc_user_space() and then after returning from that function. This patch fixes issue.

Signed-off-by: Jarkko Sakkinen jarkko.sakkinen@linux.intel.com

Applied to topic/drm-misc, thanks. Btw is this generated with some cocci script or not?

Nope. I just bumped into this when working on one yet-to-be-published driver on a completely different area and was comparing compat ioctl handlers.

-Daniel

...

---

drivers/gpu/drm/drm_ioc32.c | 55 +++++++++++++++++++++------------------------ 1 file changed, 26 insertions(+), 29 deletions(-)

diff --git a/drivers/gpu/drm/drm_ioc32.c b/drivers/gpu/drm/drm_ioc32.c index aa8bbb4..77b63e7 100644 --- a/drivers/gpu/drm/drm_ioc32.c +++ b/drivers/gpu/drm/drm_ioc32.c @@ -93,7 +93,7 @@ static int compat_drm_version(struct file *file, unsigned int cmd, return -EFAULT;

version = compat_alloc_user_space(sizeof(*version));

• if (!access_ok(VERIFY_WRITE, version, sizeof(*version)))

• if (!version) return -EFAULT; if (__put_user(v32.name_len, &version->name_len) || __put_user((void __user *)(unsigned long)v32.name,

@@ -140,7 +140,7 @@ static int compat_drm_getunique(struct file *file, unsigned int cmd, return -EFAULT;

u = compat_alloc_user_space(sizeof(*u));

• if (!access_ok(VERIFY_WRITE, u, sizeof(*u)))

• if (!u) return -EFAULT; if (__put_user(uq32.unique_len, &u->unique_len) || __put_user((void __user *)(unsigned long)uq32.unique,

@@ -168,7 +168,7 @@ static int compat_drm_setunique(struct file *file, unsigned int cmd, return -EFAULT;

u = compat_alloc_user_space(sizeof(*u));

• if (!access_ok(VERIFY_WRITE, u, sizeof(*u)))

• if (!u) return -EFAULT; if (__put_user(uq32.unique_len, &u->unique_len) || __put_user((void __user *)(unsigned long)uq32.unique,

@@ -200,7 +200,7 @@ static int compat_drm_getmap(struct file *file, unsigned int cmd, return -EFAULT;

map = compat_alloc_user_space(sizeof(*map));

• if (!access_ok(VERIFY_WRITE, map, sizeof(*map)))

• if (!map) return -EFAULT; if (__put_user(idx, &map->offset)) return -EFAULT;

@@ -237,7 +237,7 @@ static int compat_drm_addmap(struct file *file, unsigned int cmd, return -EFAULT;

map = compat_alloc_user_space(sizeof(*map));

• if (!access_ok(VERIFY_WRITE, map, sizeof(*map)))

• if (!map) return -EFAULT; if (__put_user(m32.offset, &map->offset) || __put_user(m32.size, &map->size)

@@ -277,7 +277,7 @@ static int compat_drm_rmmap(struct file *file, unsigned int cmd, return -EFAULT;

map = compat_alloc_user_space(sizeof(*map));

• if (!access_ok(VERIFY_WRITE, map, sizeof(*map)))

• if (!map) return -EFAULT; if (__put_user((void *)(unsigned long)handle, &map->handle)) return -EFAULT;

@@ -306,7 +306,7 @@ static int compat_drm_getclient(struct file *file, unsigned int cmd, return -EFAULT;

client = compat_alloc_user_space(sizeof(*client));

• if (!access_ok(VERIFY_WRITE, client, sizeof(*client)))

• if (!client) return -EFAULT; if (__put_user(idx, &client->idx)) return -EFAULT;

@@ -345,7 +345,7 @@ static int compat_drm_getstats(struct file *file, unsigned int cmd, int i, err;

stats = compat_alloc_user_space(sizeof(*stats));

• if (!access_ok(VERIFY_WRITE, stats, sizeof(*stats)))

• if (!stats) return -EFAULT;

  err = drm_ioctl(file, DRM_IOCTL_GET_STATS, (unsigned long)stats);

@@ -382,8 +382,7 @@ static int compat_drm_addbufs(struct file *file, unsigned int cmd, unsigned long agp_start;

buf = compat_alloc_user_space(sizeof(*buf));

• if (!access_ok(VERIFY_WRITE, buf, sizeof(*buf))

•    || !access_ok(VERIFY_WRITE, argp, sizeof(*argp)))
  

• if (!buf || !argp) return -EFAULT;

  if (__copy_in_user(buf, argp, offsetof(drm_buf_desc32_t, agp_start))

@@ -414,7 +413,7 @@ static int compat_drm_markbufs(struct file *file, unsigned int cmd, return -EFAULT;

buf = compat_alloc_user_space(sizeof(*buf));

• if (!access_ok(VERIFY_WRITE, buf, sizeof(*buf)))

• if (!buf) return -EFAULT;

  if (__put_user(b32.size, &buf->size)

@@ -455,7 +454,7 @@ static int compat_drm_infobufs(struct file *file, unsigned int cmd,

nbytes = sizeof(*request) + count * sizeof(struct drm_buf_desc); request = compat_alloc_user_space(nbytes);

• if (!access_ok(VERIFY_WRITE, request, nbytes))

• if (!request) return -EFAULT; list = (struct drm_buf_desc *) (request + 1);

@@ -516,7 +515,7 @@ static int compat_drm_mapbufs(struct file *file, unsigned int cmd, return -EINVAL; nbytes = sizeof(*request) + count * sizeof(struct drm_buf_pub); request = compat_alloc_user_space(nbytes);

• if (!access_ok(VERIFY_WRITE, request, nbytes))

• if (!request) return -EFAULT; list = (struct drm_buf_pub *) (request + 1);

@@ -563,7 +562,7 @@ static int compat_drm_freebufs(struct file *file, unsigned int cmd, return -EFAULT;

request = compat_alloc_user_space(sizeof(*request));

• if (!access_ok(VERIFY_WRITE, request, sizeof(*request)))

• if (!request) return -EFAULT; if (__put_user(req32.count, &request->count) || __put_user((int __user *)(unsigned long)req32.list,

@@ -589,7 +588,7 @@ static int compat_drm_setsareactx(struct file *file, unsigned int cmd, return -EFAULT;

request = compat_alloc_user_space(sizeof(*request));

• if (!access_ok(VERIFY_WRITE, request, sizeof(*request)))

• if (!request) return -EFAULT; if (__put_user(req32.ctx_id, &request->ctx_id) || __put_user((void *)(unsigned long)req32.handle,

@@ -613,7 +612,7 @@ static int compat_drm_getsareactx(struct file *file, unsigned int cmd, return -EFAULT;

request = compat_alloc_user_space(sizeof(*request));

• if (!access_ok(VERIFY_WRITE, request, sizeof(*request)))

• if (!request) return -EFAULT; if (__put_user(ctx_id, &request->ctx_id)) return -EFAULT;

@@ -646,7 +645,7 @@ static int compat_drm_resctx(struct file *file, unsigned int cmd, return -EFAULT;

res = compat_alloc_user_space(sizeof(*res));

• if (!access_ok(VERIFY_WRITE, res, sizeof(*res)))

• if (!res) return -EFAULT; if (__put_user(res32.count, &res->count) || __put_user((struct drm_ctx __user *) (unsigned long)res32.contexts,

@@ -689,7 +688,7 @@ static int compat_drm_dma(struct file *file, unsigned int cmd, return -EFAULT;

d = compat_alloc_user_space(sizeof(*d));

• if (!access_ok(VERIFY_WRITE, d, sizeof(*d)))

• if (!d) return -EFAULT;

  if (__put_user(d32.context, &d->context)

@@ -764,7 +763,7 @@ static int compat_drm_agp_info(struct file *file, unsigned int cmd, int err;

info = compat_alloc_user_space(sizeof(*info));

• if (!access_ok(VERIFY_WRITE, info, sizeof(*info)))

• if (!info) return -EFAULT;

  err = drm_ioctl(file, DRM_IOCTL_AGP_INFO, (unsigned long)info);

@@ -807,7 +806,7 @@ static int compat_drm_agp_alloc(struct file *file, unsigned int cmd, return -EFAULT;

request = compat_alloc_user_space(sizeof(*request));

• if (!access_ok(VERIFY_WRITE, request, sizeof(*request))

• if (!request || __put_user(req32.size, &request->size) || __put_user(req32.type, &request->type)) return -EFAULT;

@@ -834,7 +833,7 @@ static int compat_drm_agp_free(struct file *file, unsigned int cmd, u32 handle;

request = compat_alloc_user_space(sizeof(*request));

• if (!access_ok(VERIFY_WRITE, request, sizeof(*request))

• if (!request || get_user(handle, &argp->handle) || __put_user(handle, &request->handle)) return -EFAULT;

@@ -858,7 +857,7 @@ static int compat_drm_agp_bind(struct file *file, unsigned int cmd, return -EFAULT;

request = compat_alloc_user_space(sizeof(*request));

• if (!access_ok(VERIFY_WRITE, request, sizeof(*request))

• if (!request || __put_user(req32.handle, &request->handle) || __put_user(req32.offset, &request->offset)) return -EFAULT;

@@ -874,7 +873,7 @@ static int compat_drm_agp_unbind(struct file *file, unsigned int cmd, u32 handle;

request = compat_alloc_user_space(sizeof(*request));

• if (!access_ok(VERIFY_WRITE, request, sizeof(*request))

• if (!request || get_user(handle, &argp->handle) || __put_user(handle, &request->handle)) return -EFAULT;

@@ -897,8 +896,7 @@ static int compat_drm_sg_alloc(struct file *file, unsigned int cmd, unsigned long x;

request = compat_alloc_user_space(sizeof(*request));

• if (!access_ok(VERIFY_WRITE, request, sizeof(*request))

•    || !access_ok(VERIFY_WRITE, argp, sizeof(*argp))
  

• if (!request || !argp || __get_user(x, &argp->size) || __put_user(x, &request->size)) return -EFAULT;

@@ -923,8 +921,7 @@ static int compat_drm_sg_free(struct file *file, unsigned int cmd, unsigned long x;

request = compat_alloc_user_space(sizeof(*request));

• if (!access_ok(VERIFY_WRITE, request, sizeof(*request))

•    || !access_ok(VERIFY_WRITE, argp, sizeof(*argp))
  

• if (!request || !argp || __get_user(x, &argp->handle) || __put_user(x << PAGE_SHIFT, &request->handle)) return -EFAULT;

@@ -952,7 +949,7 @@ static int compat_drm_update_draw(struct file *file, unsigned int cmd, return -EFAULT;

request = compat_alloc_user_space(sizeof(*request));

• if (!access_ok(VERIFY_WRITE, request, sizeof(*request)) ||

• if (!request || __put_user(update32.handle, &request->handle) || __put_user(update32.type, &request->type) || __put_user(update32.num, &request->num) ||

@@ -994,7 +991,7 @@ static int compat_drm_wait_vblank(struct file *file, unsigned int cmd, return -EFAULT;

request = compat_alloc_user_space(sizeof(*request));

• if (!access_ok(VERIFY_WRITE, request, sizeof(*request))

• if (!request || __put_user(req32.request.type, &request->request.type) || __put_user(req32.request.sequence, &request->request.sequence) || __put_user(req32.request.signal, &request->request.signal))

-- 2.1.4

---

dri-devel mailing list dri-devel@lists.freedesktop.org http://lists.freedesktop.org/mailman/listinfo/dri-devel

/Jarkko