Return-Path: Received: (majordomo@vger.kernel.org) by vger.kernel.org via listexpand id S1751308AbdHSIGj (ORCPT ); Sat, 19 Aug 2017 04:06:39 -0400 Received: from mail.kapsi.fi ([91.232.154.25]:35883 "EHLO mail.kapsi.fi" rhost-flags-OK-OK-OK-OK) by vger.kernel.org with ESMTP id S1750947AbdHSIGh (ORCPT ); Sat, 19 Aug 2017 04:06:37 -0400 Subject: Re: [PATCH 4/4] drm/tegra: Use u64_to_user_ptr helper To: Dmitry Osipenko , Mikko Perttunen , thierry.reding@gmail.com, jonathanh@nvidia.com Cc: dri-devel@lists.freedesktop.org, linux-tegra@vger.kernel.org, linux-kernel@vger.kernel.org References: <20170818161553.27597-1-mperttunen@nvidia.com> <20170818161553.27597-5-mperttunen@nvidia.com> From: Mikko Perttunen Message-ID: <433b4cb7-8011-fe50-9afb-09e6ead0196e@kapsi.fi> Date: Sat, 19 Aug 2017 11:06:31 +0300 User-Agent: Mozilla/5.0 (X11; Linux x86_64; rv:52.0) Gecko/20100101 Thunderbird/52.2.1 MIME-Version: 1.0 In-Reply-To: Content-Type: text/plain; charset=utf-8; format=flowed Content-Language: en-US Content-Transfer-Encoding: 7bit X-SA-Exim-Connect-IP: 86.115.6.181 X-SA-Exim-Mail-From: cyndis@kapsi.fi X-SA-Exim-Scanned: No (on mail.kapsi.fi); SAEximRunCond expanded to false Sender: linux-kernel-owner@vger.kernel.org List-ID: X-Mailing-List: linux-kernel@vger.kernel.org Content-Length: 2272 Lines: 62 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 >> --- >> 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