| lists.openwall.net | lists / announce owl-users owl-dev john-users john-dev passwdqc-users yescrypt popa3d-users / oss-security kernel-hardening musl sabotage tlsify passwords / crypt-dev xvendor / Bugtraq Full-Disclosure linux-kernel linux-netdev linux-ext4 linux-hardening linux-cve-announce PHC | |
|
Open Source and information security mailing list archives
| ||
|
Message-ID: <20160420190254.GB2398@joana>
Date: Wed, 20 Apr 2016 16:02:54 -0300
From: Gustavo Padovan <gustavo.padovan@...labora.co.uk>
To: Maarten Lankhorst <maarten.lankhorst@...ux.intel.com>
Cc: Gustavo Padovan <gustavo@...ovan.org>,
Greg Kroah-Hartman <gregkh@...uxfoundation.org>,
linux-kernel@...r.kernel.org, devel@...verdev.osuosl.org,
dri-devel@...ts.freedesktop.org,
Daniel Stone <daniels@...labora.com>,
Arve Hjønnevåg <arve@...roid.com>,
Riley Andrews <riandrews@...roid.com>,
Daniel Vetter <daniel.vetter@...ll.ch>,
Rob Clark <robdclark@...il.com>,
Greg Hackmann <ghackmann@...gle.com>,
John Harrison <John.C.Harrison@...el.com>,
Joe Perches <joe@...ches.com>,
Andrew Morton <akpm@...ux-foundation.org>,
David Airlie <airlied@...ux.ie>,
Daniel Vetter <daniel.vetter@...el.com>
Subject: Re: [PATCH v10 RESEND 2/3] kernel.h: add u64_to_user_ptr()
2016-04-20 Maarten Lankhorst <maarten.lankhorst@...ux.intel.com>:
> Op 19-04-16 om 22:42 schreef Gustavo Padovan:
> > From: Gustavo Padovan <gustavo.padovan@...labora.co.uk>
> >
> > This function had copies in 3 different files. Unify them in kernel.h.
> >
> > Cc: Joe Perches <joe@...ches.com>
> > Cc: Andrew Morton <akpm@...ux-foundation.org>
> > Cc: David Airlie <airlied@...ux.ie>
> > Cc: Daniel Vetter <daniel.vetter@...el.com>
> > Cc: Rob Clark <robdclark@...il.com>
> > Signed-off-by: Gustavo Padovan <gustavo.padovan@...labora.co.uk>
> > ---
> > drivers/gpu/drm/etnaviv/etnaviv_gem_submit.c | 11 +++--------
> > drivers/gpu/drm/i915/i915_drv.h | 5 -----
> > drivers/gpu/drm/i915/i915_gem.c | 14 +++++++-------
> > drivers/gpu/drm/i915/i915_gem_execbuffer.c | 14 +++++++-------
> > drivers/gpu/drm/msm/msm_gem_submit.c | 11 +++--------
> > include/linux/kernel.h | 5 +++++
> > 6 files changed, 25 insertions(+), 35 deletions(-)
> >
> > diff --git a/drivers/gpu/drm/etnaviv/etnaviv_gem_submit.c b/drivers/gpu/drm/etnaviv/etnaviv_gem_submit.c
> > index 236ada9..afdd55d 100644
> > --- a/drivers/gpu/drm/etnaviv/etnaviv_gem_submit.c
> > +++ b/drivers/gpu/drm/etnaviv/etnaviv_gem_submit.c
> > @@ -28,11 +28,6 @@
> > #define BO_LOCKED 0x4000
> > #define BO_PINNED 0x2000
> >
> > -static inline void __user *to_user_ptr(u64 address)
> > -{
> > - return (void __user *)(uintptr_t)address;
> > -}
> > -
> > static struct etnaviv_gem_submit *submit_create(struct drm_device *dev,
> > struct etnaviv_gpu *gpu, size_t nr)
> > {
> > @@ -347,21 +342,21 @@ int etnaviv_ioctl_gem_submit(struct drm_device *dev, void *data,
> > cmdbuf->exec_state = args->exec_state;
> > cmdbuf->ctx = file->driver_priv;
> >
> > - ret = copy_from_user(bos, to_user_ptr(args->bos),
> > + ret = copy_from_user(bos, u64_to_user_ptr(args->bos),
> > args->nr_bos * sizeof(*bos));
> > if (ret) {
> > ret = -EFAULT;
> > goto err_submit_cmds;
> > }
> >
> > - ret = copy_from_user(relocs, to_user_ptr(args->relocs),
> > + ret = copy_from_user(relocs, u64_to_user_ptr(args->relocs),
> > args->nr_relocs * sizeof(*relocs));
> > if (ret) {
> > ret = -EFAULT;
> > goto err_submit_cmds;
> > }
> >
> > - ret = copy_from_user(stream, to_user_ptr(args->stream),
> > + ret = copy_from_user(stream, u64_to_user_ptr(args->stream),
> > args->stream_size);
> > if (ret) {
> > ret = -EFAULT;
> > diff --git a/drivers/gpu/drm/i915/i915_drv.h b/drivers/gpu/drm/i915/i915_drv.h
> > index 1048093..bb624cc 100644
> > --- a/drivers/gpu/drm/i915/i915_drv.h
> > +++ b/drivers/gpu/drm/i915/i915_drv.h
> > @@ -3576,11 +3576,6 @@ static inline i915_reg_t i915_vgacntrl_reg(struct drm_device *dev)
> > return VGACNTRL;
> > }
> >
> > -static inline void __user *to_user_ptr(u64 address)
> > -{
> > - return (void __user *)(uintptr_t)address;
> > -}
> > -
> > static inline unsigned long msecs_to_jiffies_timeout(const unsigned int m)
> > {
> > unsigned long j = msecs_to_jiffies(m);
> > diff --git a/drivers/gpu/drm/i915/i915_gem.c b/drivers/gpu/drm/i915/i915_gem.c
> > index dabc089..2889716 100644
> > --- a/drivers/gpu/drm/i915/i915_gem.c
> > +++ b/drivers/gpu/drm/i915/i915_gem.c
> > @@ -324,7 +324,7 @@ i915_gem_phys_pwrite(struct drm_i915_gem_object *obj,
> > {
> > struct drm_device *dev = obj->base.dev;
> > void *vaddr = obj->phys_handle->vaddr + args->offset;
> > - char __user *user_data = to_user_ptr(args->data_ptr);
> > + char __user *user_data = u64_to_user_ptr(args->data_ptr);
> > int ret = 0;
> >
> > /* We manually control the domain here and pretend that it
> > @@ -605,7 +605,7 @@ i915_gem_shmem_pread(struct drm_device *dev,
> > int needs_clflush = 0;
> > struct sg_page_iter sg_iter;
> >
> > - user_data = to_user_ptr(args->data_ptr);
> > + user_data = u64_to_user_ptr(args->data_ptr);
> > remain = args->size;
> >
> > obj_do_bit17_swizzling = i915_gem_object_needs_bit17_swizzle(obj);
> > @@ -692,7 +692,7 @@ i915_gem_pread_ioctl(struct drm_device *dev, void *data,
> > return 0;
> >
> > if (!access_ok(VERIFY_WRITE,
> > - to_user_ptr(args->data_ptr),
> > + u64_to_user_ptr(args->data_ptr),
> > args->size))
> > return -EFAULT;
> >
> > @@ -783,7 +783,7 @@ i915_gem_gtt_pwrite_fast(struct drm_device *dev,
> > if (ret)
> > goto out_unpin;
> >
> > - user_data = to_user_ptr(args->data_ptr);
> > + user_data = u64_to_user_ptr(args->data_ptr);
> > remain = args->size;
> >
> > offset = i915_gem_obj_ggtt_offset(obj) + args->offset;
> > @@ -907,7 +907,7 @@ i915_gem_shmem_pwrite(struct drm_device *dev,
> > int needs_clflush_before = 0;
> > struct sg_page_iter sg_iter;
> >
> > - user_data = to_user_ptr(args->data_ptr);
> > + user_data = u64_to_user_ptr(args->data_ptr);
> > remain = args->size;
> >
> > obj_do_bit17_swizzling = i915_gem_object_needs_bit17_swizzle(obj);
> > @@ -1036,12 +1036,12 @@ i915_gem_pwrite_ioctl(struct drm_device *dev, void *data,
> > return 0;
> >
> > if (!access_ok(VERIFY_READ,
> > - to_user_ptr(args->data_ptr),
> > + u64_to_user_ptr(args->data_ptr),
> > args->size))
> > return -EFAULT;
> >
> > if (likely(!i915.prefault_disable)) {
> > - ret = fault_in_multipages_readable(to_user_ptr(args->data_ptr),
> > + ret = fault_in_multipages_readable(u64_to_user_ptr(args->data_ptr),
> > args->size);
> > if (ret)
> > return -EFAULT;
> > diff --git a/drivers/gpu/drm/i915/i915_gem_execbuffer.c b/drivers/gpu/drm/i915/i915_gem_execbuffer.c
> > index 1328bc5..e60b4e7 100644
> > --- a/drivers/gpu/drm/i915/i915_gem_execbuffer.c
> > +++ b/drivers/gpu/drm/i915/i915_gem_execbuffer.c
> > @@ -514,7 +514,7 @@ i915_gem_execbuffer_relocate_vma(struct i915_vma *vma,
> > struct drm_i915_gem_exec_object2 *entry = vma->exec_entry;
> > int remain, ret;
> >
> > - user_relocs = to_user_ptr(entry->relocs_ptr);
> > + user_relocs = u64_to_user_ptr(entry->relocs_ptr);
> >
> > remain = entry->relocation_count;
> > while (remain) {
> > @@ -865,7 +865,7 @@ i915_gem_execbuffer_relocate_slow(struct drm_device *dev,
> > u64 invalid_offset = (u64)-1;
> > int j;
> >
> > - user_relocs = to_user_ptr(exec[i].relocs_ptr);
> > + user_relocs = u64_to_user_ptr(exec[i].relocs_ptr);
> >
> > if (copy_from_user(reloc+total, user_relocs,
> > exec[i].relocation_count * sizeof(*reloc))) {
> > @@ -1009,7 +1009,7 @@ validate_exec_list(struct drm_device *dev,
> > invalid_flags |= EXEC_OBJECT_NEEDS_GTT;
> >
> > for (i = 0; i < count; i++) {
> > - char __user *ptr = to_user_ptr(exec[i].relocs_ptr);
> > + char __user *ptr = u64_to_user_ptr(exec[i].relocs_ptr);
> > int length; /* limited by fault_in_pages_readable() */
> >
> > if (exec[i].flags & invalid_flags)
> > @@ -1696,7 +1696,7 @@ i915_gem_execbuffer(struct drm_device *dev, void *data,
> > return -ENOMEM;
> > }
> > ret = copy_from_user(exec_list,
> > - to_user_ptr(args->buffers_ptr),
> > + u64_to_user_ptr(args->buffers_ptr),
> > sizeof(*exec_list) * args->buffer_count);
> > if (ret != 0) {
> > DRM_DEBUG("copy %d exec entries failed %d\n",
> > @@ -1732,7 +1732,7 @@ i915_gem_execbuffer(struct drm_device *dev, void *data,
> > ret = i915_gem_do_execbuffer(dev, data, file, &exec2, exec2_list);
> > if (!ret) {
> > struct drm_i915_gem_exec_object __user *user_exec_list =
> > - to_user_ptr(args->buffers_ptr);
> > + u64_to_user_ptr(args->buffers_ptr);
> >
> > /* Copy the new buffer offsets back to the user's exec list. */
> > for (i = 0; i < args->buffer_count; i++) {
> > @@ -1786,7 +1786,7 @@ i915_gem_execbuffer2(struct drm_device *dev, void *data,
> > return -ENOMEM;
> > }
> > ret = copy_from_user(exec2_list,
> > - to_user_ptr(args->buffers_ptr),
> > + u64_to_user_ptr(args->buffers_ptr),
> > sizeof(*exec2_list) * args->buffer_count);
> > if (ret != 0) {
> > DRM_DEBUG("copy %d exec entries failed %d\n",
> > @@ -1799,7 +1799,7 @@ i915_gem_execbuffer2(struct drm_device *dev, void *data,
> > if (!ret) {
> > /* Copy the new buffer offsets back to the user's exec list. */
> > struct drm_i915_gem_exec_object2 __user *user_exec_list =
> > - to_user_ptr(args->buffers_ptr);
> > + u64_to_user_ptr(args->buffers_ptr);
> > int i;
> >
> > for (i = 0; i < args->buffer_count; i++) {
> > diff --git a/drivers/gpu/drm/msm/msm_gem_submit.c b/drivers/gpu/drm/msm/msm_gem_submit.c
> > index 43d2181..23d2528 100644
> > --- a/drivers/gpu/drm/msm/msm_gem_submit.c
> > +++ b/drivers/gpu/drm/msm/msm_gem_submit.c
> > @@ -28,11 +28,6 @@
> > #define BO_LOCKED 0x4000
> > #define BO_PINNED 0x2000
> >
> > -static inline void __user *to_user_ptr(u64 address)
> > -{
> > - return (void __user *)(uintptr_t)address;
> > -}
> > -
> > static struct msm_gem_submit *submit_create(struct drm_device *dev,
> > struct msm_gpu *gpu, int nr)
> > {
> > @@ -68,7 +63,7 @@ static int submit_lookup_objects(struct msm_gem_submit *submit,
> > struct drm_gem_object *obj;
> > struct msm_gem_object *msm_obj;
> > void __user *userptr =
> > - to_user_ptr(args->bos + (i * sizeof(submit_bo)));
> > + u64_to_user_ptr(args->bos + (i * sizeof(submit_bo)));
> >
> > ret = copy_from_user(&submit_bo, userptr, sizeof(submit_bo));
> > if (ret) {
> > @@ -257,7 +252,7 @@ static int submit_reloc(struct msm_gem_submit *submit, struct msm_gem_object *ob
> > for (i = 0; i < nr_relocs; i++) {
> > struct drm_msm_gem_submit_reloc submit_reloc;
> > void __user *userptr =
> > - to_user_ptr(relocs + (i * sizeof(submit_reloc)));
> > + u64_to_user_ptr(relocs + (i * sizeof(submit_reloc)));
> > uint32_t iova, off;
> > bool valid;
> >
> > @@ -356,7 +351,7 @@ int msm_ioctl_gem_submit(struct drm_device *dev, void *data,
> > for (i = 0; i < args->nr_cmds; i++) {
> > struct drm_msm_gem_submit_cmd submit_cmd;
> > void __user *userptr =
> > - to_user_ptr(args->cmds + (i * sizeof(submit_cmd)));
> > + u64_to_user_ptr(args->cmds + (i * sizeof(submit_cmd)));
> > struct msm_gem_object *msm_obj;
> > uint32_t iova;
> >
> > diff --git a/include/linux/kernel.h b/include/linux/kernel.h
> > index 2f7775e..0c843ec 100644
> > --- a/include/linux/kernel.h
> > +++ b/include/linux/kernel.h
> > @@ -53,6 +53,11 @@
> >
> > #define ARRAY_SIZE(arr) (sizeof(arr) / sizeof((arr)[0]) + __must_be_array(arr))
> >
> > +static inline void __user *u64_to_user_ptr(u64 address)
> > +{
> > + return (void __user *)(uintptr_t)address;
> > +}
> This could cast any integer to a user ptr, maybe add some preprocessor magic to only allow u64?
Right, that makes sense. I'll add typecheck() and send an updated v11 version.
Gustavo
Powered by blists - more mailing lists