2015-07-01 09:24:55

by Jarkko Sakkinen

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

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 <[email protected]>
---
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

2015-07-02 07:29:38

by Daniel Vetter

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

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 <[email protected]>

Applied to topic/drm-misc, thanks. Btw is this generated with some cocci
script or not?
-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
> [email protected]
> http://lists.freedesktop.org/mailman/listinfo/dri-devel

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

2015-07-02 08:07:48

by Jani Nikula

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

On Wed, 01 Jul 2015, Jarkko Sakkinen <[email protected]> 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 <[email protected]>
> ---
> 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)

The risks of touching old code... who does access_ok on argp now? That
doesn't come from compat_alloc_user_space.

Same pattern repeated below a few times.

BR,
Jani.


> 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
> [email protected]
> http://lists.freedesktop.org/mailman/listinfo/dri-devel

--
Jani Nikula, Intel Open Source Technology Center

2015-07-02 14:21:46

by Daniel Vetter

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

On Thu, Jul 02, 2015 at 11:10:06AM +0300, Jani Nikula wrote:
> On Wed, 01 Jul 2015, Jarkko Sakkinen <[email protected]> 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 <[email protected]>
> > ---
> > 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)
>
> The risks of touching old code... who does access_ok on argp now? That
> doesn't come from compat_alloc_user_space.
>
> Same pattern repeated below a few times.

Oops, good catch. Patch dropped again.
-Daniel

>
> BR,
> Jani.
>
>
> > 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
> > [email protected]
> > http://lists.freedesktop.org/mailman/listinfo/dri-devel
>
> --
> Jani Nikula, Intel Open Source Technology Center
> _______________________________________________
> dri-devel mailing list
> [email protected]
> http://lists.freedesktop.org/mailman/listinfo/dri-devel

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

2015-07-03 10:59:51

by Jarkko Sakkinen

Subject: 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 <[email protected]>
> 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
>> [email protected]
>> http://lists.freedesktop.org/mailman/listinfo/dri-devel
/Jarkko

2015-07-03 11:03:32

by Jarkko Sakkinen

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

On 07/02/2015 11:10 AM, Jani Nikula wrote:
> On Wed, 01 Jul 2015, Jarkko Sakkinen <[email protected]> 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 <[email protected]>
>> ---
>> 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)
> The risks of touching old code... who does access_ok on argp now? That
> doesn't come from compat_alloc_user_space.
>
> Same pattern repeated below a few times.
Right. Thanks and sorry about sloppiness. I'll update the patch.

>
> BR,
> Jani.
>
>
>> 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
>> [email protected]
>> http://lists.freedesktop.org/mailman/listinfo/dri-devel

/Jarkko