| Message ID | 20170818161553.27597-5-mperttunen@nvidia.com (mailing list archive) |
|---|---|
| State | New, archived |
| Headers | show |
On 18.08.2017 19:15, Mikko Perttunen wrote: > Use the u64_to_user_ptr helper macro to cast IOCTL argument u64 values > to user pointers instead of writing out the cast manually. > > Signed-off-by: Mikko Perttunen <mperttunen@nvidia.com> > --- > drivers/gpu/drm/tegra/drm.c | 9 ++++----- > 1 file changed, 4 insertions(+), 5 deletions(-) > > diff --git a/drivers/gpu/drm/tegra/drm.c b/drivers/gpu/drm/tegra/drm.c > index e3331a2bc082..78c98736b0a5 100644 > --- a/drivers/gpu/drm/tegra/drm.c > +++ b/drivers/gpu/drm/tegra/drm.c > @@ -389,11 +389,10 @@ int tegra_drm_submit(struct tegra_drm_context *context, > unsigned int num_relocs = args->num_relocs; > unsigned int num_waitchks = args->num_waitchks; > struct drm_tegra_cmdbuf __user *cmdbufs = > - (void __user *)(uintptr_t)args->cmdbufs; > - struct drm_tegra_reloc __user *relocs = > - (void __user *)(uintptr_t)args->relocs; > + u64_to_user_ptr(args->cmdbufs); > + struct drm_tegra_reloc __user *relocs = u64_to_user_ptr(args->relocs); > struct drm_tegra_waitchk __user *waitchks = > - (void __user *)(uintptr_t)args->waitchks; > + u64_to_user_ptr(args->waitchks); What about to factor out 'u64_to_user_ptr()' assignments to reduce messiness a tad? Like this: struct drm_tegra_waitchk __user *waitchks; struct drm_tegra_cmdbuf __user *cmdbufs; struct drm_tegra_reloc __user *relocs; ... waitchks = u64_to_user_ptr(args->waitchks); cmdbufs = u64_to_user_ptr(args->cmdbufs); relocs = u64_to_user_ptr(args->relocs); > struct drm_tegra_syncpt syncpt; > struct host1x *host1x = dev_get_drvdata(drm->dev->parent); > struct host1x_syncpt *sp; > @@ -520,7 +519,7 @@ int tegra_drm_submit(struct tegra_drm_context *context, > } > } > > - if (copy_from_user(&syncpt, (void __user *)(uintptr_t)args->syncpts, > + if (copy_from_user(&syncpt, u64_to_user_ptr(args->syncpts), What about to define and use 'struct drm_tegra_reloc __user *syncpts' for consistency with other '__user' definitions? > sizeof(syncpt))) { > err = -EFAULT; > goto fail; >
On 08/19/2017 01:05 AM, Dmitry Osipenko wrote: > On 18.08.2017 19:15, Mikko Perttunen wrote: >> Use the u64_to_user_ptr helper macro to cast IOCTL argument u64 values >> to user pointers instead of writing out the cast manually. >> >> Signed-off-by: Mikko Perttunen <mperttunen@nvidia.com> >> --- >> drivers/gpu/drm/tegra/drm.c | 9 ++++----- >> 1 file changed, 4 insertions(+), 5 deletions(-) >> >> diff --git a/drivers/gpu/drm/tegra/drm.c b/drivers/gpu/drm/tegra/drm.c >> index e3331a2bc082..78c98736b0a5 100644 >> --- a/drivers/gpu/drm/tegra/drm.c >> +++ b/drivers/gpu/drm/tegra/drm.c >> @@ -389,11 +389,10 @@ int tegra_drm_submit(struct tegra_drm_context *context, >> unsigned int num_relocs = args->num_relocs; >> unsigned int num_waitchks = args->num_waitchks; >> struct drm_tegra_cmdbuf __user *cmdbufs = >> - (void __user *)(uintptr_t)args->cmdbufs; >> - struct drm_tegra_reloc __user *relocs = >> - (void __user *)(uintptr_t)args->relocs; >> + u64_to_user_ptr(args->cmdbufs); >> + struct drm_tegra_reloc __user *relocs = u64_to_user_ptr(args->relocs); >> struct drm_tegra_waitchk __user *waitchks = >> - (void __user *)(uintptr_t)args->waitchks; >> + u64_to_user_ptr(args->waitchks); > > What about to factor out 'u64_to_user_ptr()' assignments to reduce messiness a > tad? Like this: > > struct drm_tegra_waitchk __user *waitchks; > struct drm_tegra_cmdbuf __user *cmdbufs; > struct drm_tegra_reloc __user *relocs; > ... > waitchks = u64_to_user_ptr(args->waitchks); > cmdbufs = u64_to_user_ptr(args->cmdbufs); > relocs = u64_to_user_ptr(args->relocs); > > >> struct drm_tegra_syncpt syncpt; >> struct host1x *host1x = dev_get_drvdata(drm->dev->parent); >> struct host1x_syncpt *sp; >> @@ -520,7 +519,7 @@ int tegra_drm_submit(struct tegra_drm_context *context, >> } >> } >> >> - if (copy_from_user(&syncpt, (void __user *)(uintptr_t)args->syncpts, >> + if (copy_from_user(&syncpt, u64_to_user_ptr(args->syncpts), > > What about to define and use 'struct drm_tegra_reloc __user *syncpts' for > consistency with other '__user' definitions? > >> sizeof(syncpt))) { >> err = -EFAULT; >> goto fail; >> > Yeah, these are good ideas. I'll post a v2 at some point with these changes. Thanks! Mikko
diff --git a/drivers/gpu/drm/tegra/drm.c b/drivers/gpu/drm/tegra/drm.c index e3331a2bc082..78c98736b0a5 100644 --- a/drivers/gpu/drm/tegra/drm.c +++ b/drivers/gpu/drm/tegra/drm.c @@ -389,11 +389,10 @@ int tegra_drm_submit(struct tegra_drm_context *context, unsigned int num_relocs = args->num_relocs; unsigned int num_waitchks = args->num_waitchks; struct drm_tegra_cmdbuf __user *cmdbufs = - (void __user *)(uintptr_t)args->cmdbufs; - struct drm_tegra_reloc __user *relocs = - (void __user *)(uintptr_t)args->relocs; + u64_to_user_ptr(args->cmdbufs); + struct drm_tegra_reloc __user *relocs = u64_to_user_ptr(args->relocs); struct drm_tegra_waitchk __user *waitchks = - (void __user *)(uintptr_t)args->waitchks; + u64_to_user_ptr(args->waitchks); struct drm_tegra_syncpt syncpt; struct host1x *host1x = dev_get_drvdata(drm->dev->parent); struct host1x_syncpt *sp; @@ -520,7 +519,7 @@ int tegra_drm_submit(struct tegra_drm_context *context, } } - if (copy_from_user(&syncpt, (void __user *)(uintptr_t)args->syncpts, + if (copy_from_user(&syncpt, u64_to_user_ptr(args->syncpts), sizeof(syncpt))) { err = -EFAULT; goto fail;
Use the u64_to_user_ptr helper macro to cast IOCTL argument u64 values to user pointers instead of writing out the cast manually. Signed-off-by: Mikko Perttunen <mperttunen@nvidia.com> --- drivers/gpu/drm/tegra/drm.c | 9 ++++----- 1 file changed, 4 insertions(+), 5 deletions(-)