From: Gustavo Padovan <gustavo.padovan@collabora.co.uk>
To: Maarten Lankhorst <maarten.lankhorst@linux.intel.com>
Cc: "Gustavo Padovan" <gustavo@padovan.org>,
"Greg Kroah-Hartman" <gregkh@linuxfoundation.org>,
linux-kernel@vger.kernel.org, devel@driverdev.osuosl.org,
dri-devel@lists.freedesktop.org,
"Daniel Stone" <daniels@collabora.com>,
"Arve Hjønnevåg" <arve@android.com>,
"Riley Andrews" <riandrews@android.com>,
"Daniel Vetter" <daniel.vetter@ffwll.ch>,
"Rob Clark" <robdclark@gmail.com>,
"Greg Hackmann" <ghackmann@google.com>,
"John Harrison" <John.C.Harrison@Intel.com>,
"Joe Perches" <joe@perches.com>,
"Andrew Morton" <akpm@linux-foundation.org>,
"David Airlie" <airlied@linux.ie>,
"Daniel Vetter" <daniel.vetter@Intel.com>
Subject: Re: [PATCH v10 RESEND 2/3] kernel.h: add u64_to_user_ptr()
Date: Wed, 20 Apr 2016 16:02:54 -0300 [thread overview]
Message-ID: <20160420190254.GB2398@joana> (raw)
In-Reply-To: <57172AB7.9080705@linux.intel.com>
2016-04-20 Maarten Lankhorst <maarten.lankhorst@linux.intel.com>:
> Op 19-04-16 om 22:42 schreef Gustavo Padovan:
> > From: Gustavo Padovan <gustavo.padovan@collabora.co.uk>
> >
> > This function had copies in 3 different files. Unify them in kernel.h.
> >
> > Cc: Joe Perches <joe@perches.com>
> > Cc: Andrew Morton <akpm@linux-foundation.org>
> > Cc: David Airlie <airlied@linux.ie>
> > Cc: Daniel Vetter <daniel.vetter@intel.com>
> > Cc: Rob Clark <robdclark@gmail.com>
> > Signed-off-by: Gustavo Padovan <gustavo.padovan@collabora.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
next prev parent reply other threads:[~2016-04-20 19:03 UTC|newest]
Thread overview: 5+ messages / expand[flat|nested] mbox.gz Atom feed top
2016-04-19 20:42 [PATCH v10 RESEND 1/3] staging/android: remove redundant comments on sync_merge_data Gustavo Padovan
2016-04-19 20:42 ` [PATCH v10 RESEND 2/3] kernel.h: add u64_to_user_ptr() Gustavo Padovan
2016-04-20 7:07 ` Maarten Lankhorst
2016-04-20 19:02 ` Gustavo Padovan [this message]
2016-04-19 20:42 ` [PATCH v10 RESEND 3/3] staging/android: refactor SYNC IOCTLs Gustavo Padovan
Reply instructions:
You may reply publicly to this message via plain-text email
using any one of the following methods:
* Save the following mbox file, import it into your mail client,
and reply-to-all from there: mbox
Avoid top-posting and favor interleaved quoting:
https://en.wikipedia.org/wiki/Posting_style#Interleaved_style
* Reply using the --to, --cc, and --in-reply-to
switches of git-send-email(1):
git send-email \
--in-reply-to=20160420190254.GB2398@joana \
--to=gustavo.padovan@collabora.co.uk \
--cc=John.C.Harrison@Intel.com \
--cc=airlied@linux.ie \
--cc=akpm@linux-foundation.org \
--cc=arve@android.com \
--cc=daniel.vetter@Intel.com \
--cc=daniel.vetter@ffwll.ch \
--cc=daniels@collabora.com \
--cc=devel@driverdev.osuosl.org \
--cc=dri-devel@lists.freedesktop.org \
--cc=ghackmann@google.com \
--cc=gregkh@linuxfoundation.org \
--cc=gustavo@padovan.org \
--cc=joe@perches.com \
--cc=linux-kernel@vger.kernel.org \
--cc=maarten.lankhorst@linux.intel.com \
--cc=riandrews@android.com \
--cc=robdclark@gmail.com \
/path/to/YOUR_REPLY
https://kernel.org/pub/software/scm/git/docs/git-send-email.html
* If your mail client supports setting the In-Reply-To header
via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line
before the message body.
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox
Powered by JetHome