[PATCH v10 RESEND 2/3] kernel.h: add u64_to_user_ptr()

Gustavo Padovan gustavo.padovan at collabora.co.uk
Wed Apr 20 19:02:54 UTC 2016


2016-04-20 Maarten Lankhorst <maarten.lankhorst at linux.intel.com>:

> Op 19-04-16 om 22:42 schreef Gustavo Padovan:
> > From: Gustavo Padovan <gustavo.padovan at collabora.co.uk>
> >
> > This function had copies in 3 different files. Unify them in kernel.h.
> >
> > Cc: Joe Perches <joe at perches.com>
> > Cc: Andrew Morton <akpm at linux-foundation.org>
> > Cc: David Airlie <airlied at linux.ie>
> > Cc: Daniel Vetter <daniel.vetter at intel.com>
> > Cc: Rob Clark <robdclark at gmail.com>
> > Signed-off-by: Gustavo Padovan <gustavo.padovan at 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


More information about the devel mailing list