[PATCH v3 0/7] media: v4l2: compat ioctl fixes

[PATCH v3 0/7] media: v4l2: compat ioctl fixes

[Arnd Bergmann]

From: Arnd Bergmann <arnd@arndb.de>

There was a report from Syzbot a while ago that I tried fixed earlier,
but my fix did not get picked up because of a merge conflict with
another patch I had in my tree.

I finally managed to take a close enough look at the merge conflict
to figure out that the subdev driver handling for VIDIOC_DQEVENT_TIME32
was wrong in all combinations of the patches and just needs to be
removed. In the process I also came across a couple of other issues,
so the series has now grown to seven patches.

I have done randconfig build testing and found no compile time issues,
but the driver specific patches have not been tested so far.

        Arnd

Link: https://patchwork.linuxtv.org/project/linux-media/patch/20210318134334.2933141-1-arnd@kernel.org/
Link: https://patchwork.linuxtv.org/project/linux-media/list/?series=5655

Arnd Bergmann (8):
  media: v4l2-core: ignore native time32 ioctls on 64-bit
  media: v4l2-core: explicitly clear ioctl input data
  media: v4l2-core: fix whitespace damage in video_get_user()
  media: subdev: remove VIDIOC_DQEVENT_TIME32 handling
  media: v4l2-core: return -ENODEV from ioctl when not registered
  media: atomisp: remove compat_ioctl32 code
  media: subdev: fix compat_ioctl32
  media: subdev: disallow ioctl for saa6588/davinci

 drivers/media/i2c/adv7842.c                   |    3 +
 drivers/media/i2c/saa6588.c                   |    4 +-
 drivers/media/pci/bt8xx/bttv-driver.c         |    6 +-
 drivers/media/pci/saa7134/saa7134-video.c     |    6 +-
 drivers/media/platform/davinci/vpbe_display.c |    2 +-
 drivers/media/platform/davinci/vpbe_venc.c    |    6 +-
 drivers/media/radio/si4713/si4713.c           |    3 +
 drivers/media/v4l2-core/v4l2-compat-ioctl32.c |    3 +
 drivers/media/v4l2-core/v4l2-ioctl.c          |   32 +-
 drivers/media/v4l2-core/v4l2-subdev.c         |   43 +-
 drivers/staging/media/atomisp/Makefile        |    1 -
 drivers/staging/media/atomisp/TODO            |    5 +
 .../atomisp/pci/atomisp_compat_ioctl32.c      | 1202 -----------------
 .../staging/media/atomisp/pci/atomisp_fops.c  |    8 +-
 include/media/v4l2-subdev.h                   |    7 +-
 15 files changed, 66 insertions(+), 1265 deletions(-)
 delete mode 100644 drivers/staging/media/atomisp/pci/atomisp_compat_ioctl32.c

-- 
2.29.2

Cc: Hans Verkuil <hverkuil-cisco@xs4all.nl>
Cc: Mauro Carvalho Chehab <mchehab@kernel.org>
Cc: "Lad, Prabhakar" <prabhakar.csengg@gmail.com>
Cc: Eduardo Valentin <edubezval@gmail.com>
Cc: Sakari Ailus <sakari.ailus@linux.intel.com>
Cc: Greg Kroah-Hartman <gregkh@linuxfoundation.org>
Cc: Arnd Bergmann <arnd@arndb.de>
Cc: Vaibhav Gupta <vaibhavgupta40@gmail.com>
Cc: Liu Shixin <liushixin2@huawei.com>
Cc: Laurent Pinchart <laurent.pinchart@ideasonboard.com>
Cc: Jacopo Mondi <jacopo+renesas@jmondi.org>
Cc: Andy Shevchenko <andriy.shevchenko@linux.intel.com>
Cc: linux-kernel@vger.kernel.org
Cc: linux-media@vger.kernel.org
Cc: linux-staging@lists.linux.dev

[PATCH v3 1/8] media: v4l2-core: ignore native time32 ioctls on 64-bit

[Arnd Bergmann]

From: Arnd Bergmann <arnd@arndb.de>

Syzbot found that passing ioctl command 0xc0505609 into a 64-bit
kernel from a 32-bit process causes uninitialized kernel memory to
get passed to drivers instead of the user space data:

BUG: KMSAN: uninit-value in check_array_args drivers/media/v4l2-core/v4l2-ioctl.c:3041 [inline]
BUG: KMSAN: uninit-value in video_usercopy+0x1631/0x3d30 drivers/media/v4l2-core/v4l2-ioctl.c:3315
CPU: 0 PID: 19595 Comm: syz-executor.4 Not tainted 5.11.0-rc7-syzkaller #0
Hardware name: Google Google Compute Engine/Google Compute Engine, BIOS Google 01/01/2011
Call Trace:
 __dump_stack lib/dump_stack.c:79 [inline]
 dump_stack+0x21c/0x280 lib/dump_stack.c:120
 kmsan_report+0xfb/0x1e0 mm/kmsan/kmsan_report.c:118
 __msan_warning+0x5f/0xa0 mm/kmsan/kmsan_instr.c:197
 check_array_args drivers/media/v4l2-core/v4l2-ioctl.c:3041 [inline]
 video_usercopy+0x1631/0x3d30 drivers/media/v4l2-core/v4l2-ioctl.c:3315
 video_ioctl2+0x9f/0xb0 drivers/media/v4l2-core/v4l2-ioctl.c:3391
 v4l2_ioctl+0x255/0x290 drivers/media/v4l2-core/v4l2-dev.c:360
 v4l2_compat_ioctl32+0x2c6/0x370 drivers/media/v4l2-core/v4l2-compat-ioctl32.c:1248
 __do_compat_sys_ioctl fs/ioctl.c:842 [inline]
 __se_compat_sys_ioctl+0x53d/0x1100 fs/ioctl.c:793
 __ia32_compat_sys_ioctl+0x4a/0x70 fs/ioctl.c:793
 do_syscall_32_irqs_on arch/x86/entry/common.c:79 [inline]
 __do_fast_syscall_32+0x102/0x160 arch/x86/entry/common.c:141
 do_fast_syscall_32+0x6a/0xc0 arch/x86/entry/common.c:166
 do_SYSENTER_32+0x73/0x90 arch/x86/entry/common.c:209
 entry_SYSENTER_compat_after_hwframe+0x4d/0x5c

The time32 commands are defined but were never meant to be called on
64-bit machines, as those have always used time64 interfaces.  I missed
this in my patch that introduced the time64 handling on 32-bit platforms.

The problem in this case is the mismatch of one function checking for
the numeric value of the command and another function checking for the
type of process (native vs compat) instead, with the result being that
for this combination, nothing gets copied into the buffer at all.

Avoid this by only trying to convert the time32 commands when running
on a 32-bit kernel where these are defined in a meaningful way.

Fixes: 577c89b0ce72 ("media: v4l2-core: fix v4l2_buffer handling for time64 ABI")
Reported-by: syzbot+142888ffec98ab194028@syzkaller.appspotmail.com
Tested-by: Hans Verkuil <hverkuil-cisco@xs4all.nl>
Signed-off-by: Arnd Bergmann <arnd@arndb.de>
---
This patch adds two more changes than the version that Hans tested
Signed-off-by: Arnd Bergmann <arnd@arndb.de>
---
 drivers/media/v4l2-core/v4l2-ioctl.c | 6 +++---
 1 file changed, 3 insertions(+), 3 deletions(-)

diff --git a/drivers/media/v4l2-core/v4l2-ioctl.c b/drivers/media/v4l2-core/v4l2-ioctl.c
index 2673f51aafa4..58df927aec7e 100644
--- a/drivers/media/v4l2-core/v4l2-ioctl.c
+++ b/drivers/media/v4l2-core/v4l2-ioctl.c
@@ -3073,7 +3073,7 @@ static int check_array_args(unsigned int cmd, void *parg, size_t *array_size,
 static unsigned int video_translate_cmd(unsigned int cmd)
 {
 	switch (cmd) {
-#ifdef CONFIG_COMPAT_32BIT_TIME
+#if !defined(CONFIG_64BIT) && defined(CONFIG_COMPAT_32BIT_TIME)
 	case VIDIOC_DQEVENT_TIME32:
 		return VIDIOC_DQEVENT;
 	case VIDIOC_QUERYBUF_TIME32:
@@ -3127,7 +3127,7 @@ static int video_get_user(void __user *arg, void *parg,
 		err = v4l2_compat_get_user(arg, parg, cmd);
 	} else {
 		switch (cmd) {
-#ifdef CONFIG_COMPAT_32BIT_TIME
+#if !defined(CONFIG_64BIT) && defined(CONFIG_COMPAT_32BIT_TIME)
 		case VIDIOC_QUERYBUF_TIME32:
 		case VIDIOC_QBUF_TIME32:
 		case VIDIOC_DQBUF_TIME32:
@@ -3182,7 +3182,7 @@ static int video_put_user(void __user *arg, void *parg,
 		return v4l2_compat_put_user(arg, parg, cmd);
 
 	switch (cmd) {
-#ifdef CONFIG_COMPAT_32BIT_TIME
+#if !defined(CONFIG_64BIT) && defined(CONFIG_COMPAT_32BIT_TIME)
 	case VIDIOC_DQEVENT_TIME32: {
 		struct v4l2_event *ev = parg;
 		struct v4l2_event_time32 ev32;
-- 
2.29.2

Re: [PATCH v3 1/8] media: v4l2-core: ignore native time32 ioctls on 64-bit

[Andy Shevchenko]

On Mon, Jun 14, 2021 at 12:34:02PM +0200, Arnd Bergmann wrote:
> From: Arnd Bergmann <arnd@arndb.de>
> 
> Syzbot found that passing ioctl command 0xc0505609 into a 64-bit
> kernel from a 32-bit process causes uninitialized kernel memory to
> get passed to drivers instead of the user space data:

> BUG: KMSAN: uninit-value in check_array_args drivers/media/v4l2-core/v4l2-ioctl.c:3041 [inline]
> BUG: KMSAN: uninit-value in video_usercopy+0x1631/0x3d30 drivers/media/v4l2-core/v4l2-ioctl.c:3315
> CPU: 0 PID: 19595 Comm: syz-executor.4 Not tainted 5.11.0-rc7-syzkaller #0
> Hardware name: Google Google Compute Engine/Google Compute Engine, BIOS Google 01/01/2011
> Call Trace:
>  __dump_stack lib/dump_stack.c:79 [inline]
>  dump_stack+0x21c/0x280 lib/dump_stack.c:120
>  kmsan_report+0xfb/0x1e0 mm/kmsan/kmsan_report.c:118
>  __msan_warning+0x5f/0xa0 mm/kmsan/kmsan_instr.c:197
>  check_array_args drivers/media/v4l2-core/v4l2-ioctl.c:3041 [inline]
>  video_usercopy+0x1631/0x3d30 drivers/media/v4l2-core/v4l2-ioctl.c:3315
>  video_ioctl2+0x9f/0xb0 drivers/media/v4l2-core/v4l2-ioctl.c:3391
>  v4l2_ioctl+0x255/0x290 drivers/media/v4l2-core/v4l2-dev.c:360
>  v4l2_compat_ioctl32+0x2c6/0x370 drivers/media/v4l2-core/v4l2-compat-ioctl32.c:1248
>  __do_compat_sys_ioctl fs/ioctl.c:842 [inline]
>  __se_compat_sys_ioctl+0x53d/0x1100 fs/ioctl.c:793
>  __ia32_compat_sys_ioctl+0x4a/0x70 fs/ioctl.c:793
>  do_syscall_32_irqs_on arch/x86/entry/common.c:79 [inline]
>  __do_fast_syscall_32+0x102/0x160 arch/x86/entry/common.c:141
>  do_fast_syscall_32+0x6a/0xc0 arch/x86/entry/common.c:166
>  do_SYSENTER_32+0x73/0x90 arch/x86/entry/common.c:209
>  entry_SYSENTER_compat_after_hwframe+0x4d/0x5c

Can we, please, get a habit to reduce tracebacks to only significant lines?
This is not only reduces the storage foot print of repositories in the world
(and thus electricity consumed for any operation on it) but also increases
density of useful information on one (small, due to reading on laptops / small
screen size devices) page.

> The time32 commands are defined but were never meant to be called on
> 64-bit machines, as those have always used time64 interfaces.  I missed
> this in my patch that introduced the time64 handling on 32-bit platforms.
> 
> The problem in this case is the mismatch of one function checking for
> the numeric value of the command and another function checking for the
> type of process (native vs compat) instead, with the result being that
> for this combination, nothing gets copied into the buffer at all.
> 
> Avoid this by only trying to convert the time32 commands when running
> on a 32-bit kernel where these are defined in a meaningful way.

-- 
With Best Regards,
Andy Shevchenko

Re: [PATCH v3 1/8] media: v4l2-core: ignore native time32 ioctls on 64-bit

[Laurent Pinchart]

Hi Arnd,

Thank you for the patch.

On Mon, Jun 14, 2021 at 12:34:02PM +0200, Arnd Bergmann wrote:
> From: Arnd Bergmann <arnd@arndb.de>
> 
> Syzbot found that passing ioctl command 0xc0505609 into a 64-bit
> kernel from a 32-bit process causes uninitialized kernel memory to
> get passed to drivers instead of the user space data:
> 
> BUG: KMSAN: uninit-value in check_array_args drivers/media/v4l2-core/v4l2-ioctl.c:3041 [inline]
> BUG: KMSAN: uninit-value in video_usercopy+0x1631/0x3d30 drivers/media/v4l2-core/v4l2-ioctl.c:3315
> CPU: 0 PID: 19595 Comm: syz-executor.4 Not tainted 5.11.0-rc7-syzkaller #0
> Hardware name: Google Google Compute Engine/Google Compute Engine, BIOS Google 01/01/2011
> Call Trace:
>  __dump_stack lib/dump_stack.c:79 [inline]
>  dump_stack+0x21c/0x280 lib/dump_stack.c:120
>  kmsan_report+0xfb/0x1e0 mm/kmsan/kmsan_report.c:118
>  __msan_warning+0x5f/0xa0 mm/kmsan/kmsan_instr.c:197
>  check_array_args drivers/media/v4l2-core/v4l2-ioctl.c:3041 [inline]
>  video_usercopy+0x1631/0x3d30 drivers/media/v4l2-core/v4l2-ioctl.c:3315
>  video_ioctl2+0x9f/0xb0 drivers/media/v4l2-core/v4l2-ioctl.c:3391
>  v4l2_ioctl+0x255/0x290 drivers/media/v4l2-core/v4l2-dev.c:360
>  v4l2_compat_ioctl32+0x2c6/0x370 drivers/media/v4l2-core/v4l2-compat-ioctl32.c:1248
>  __do_compat_sys_ioctl fs/ioctl.c:842 [inline]
>  __se_compat_sys_ioctl+0x53d/0x1100 fs/ioctl.c:793
>  __ia32_compat_sys_ioctl+0x4a/0x70 fs/ioctl.c:793
>  do_syscall_32_irqs_on arch/x86/entry/common.c:79 [inline]
>  __do_fast_syscall_32+0x102/0x160 arch/x86/entry/common.c:141
>  do_fast_syscall_32+0x6a/0xc0 arch/x86/entry/common.c:166
>  do_SYSENTER_32+0x73/0x90 arch/x86/entry/common.c:209
>  entry_SYSENTER_compat_after_hwframe+0x4d/0x5c
> 
> The time32 commands are defined but were never meant to be called on
> 64-bit machines, as those have always used time64 interfaces.  I missed
> this in my patch that introduced the time64 handling on 32-bit platforms.
> 
> The problem in this case is the mismatch of one function checking for
> the numeric value of the command and another function checking for the
> type of process (native vs compat) instead, with the result being that
> for this combination, nothing gets copied into the buffer at all.
> 
> Avoid this by only trying to convert the time32 commands when running
> on a 32-bit kernel where these are defined in a meaningful way.
> 
> Fixes: 577c89b0ce72 ("media: v4l2-core: fix v4l2_buffer handling for time64 ABI")
> Reported-by: syzbot+142888ffec98ab194028@syzkaller.appspotmail.com
> Tested-by: Hans Verkuil <hverkuil-cisco@xs4all.nl>
> Signed-off-by: Arnd Bergmann <arnd@arndb.de>
> ---
> This patch adds two more changes than the version that Hans tested
> Signed-off-by: Arnd Bergmann <arnd@arndb.de>

Reviewed-by: Laurent Pinchart <laurent.pinchart@ideasonboard.com>

> ---
>  drivers/media/v4l2-core/v4l2-ioctl.c | 6 +++---
>  1 file changed, 3 insertions(+), 3 deletions(-)
> 
> diff --git a/drivers/media/v4l2-core/v4l2-ioctl.c b/drivers/media/v4l2-core/v4l2-ioctl.c
> index 2673f51aafa4..58df927aec7e 100644
> --- a/drivers/media/v4l2-core/v4l2-ioctl.c
> +++ b/drivers/media/v4l2-core/v4l2-ioctl.c
> @@ -3073,7 +3073,7 @@ static int check_array_args(unsigned int cmd, void *parg, size_t *array_size,
>  static unsigned int video_translate_cmd(unsigned int cmd)
>  {
>  	switch (cmd) {
> -#ifdef CONFIG_COMPAT_32BIT_TIME
> +#if !defined(CONFIG_64BIT) && defined(CONFIG_COMPAT_32BIT_TIME)
>  	case VIDIOC_DQEVENT_TIME32:
>  		return VIDIOC_DQEVENT;
>  	case VIDIOC_QUERYBUF_TIME32:
> @@ -3127,7 +3127,7 @@ static int video_get_user(void __user *arg, void *parg,
>  		err = v4l2_compat_get_user(arg, parg, cmd);
>  	} else {
>  		switch (cmd) {
> -#ifdef CONFIG_COMPAT_32BIT_TIME
> +#if !defined(CONFIG_64BIT) && defined(CONFIG_COMPAT_32BIT_TIME)
>  		case VIDIOC_QUERYBUF_TIME32:
>  		case VIDIOC_QBUF_TIME32:
>  		case VIDIOC_DQBUF_TIME32:
> @@ -3182,7 +3182,7 @@ static int video_put_user(void __user *arg, void *parg,
>  		return v4l2_compat_put_user(arg, parg, cmd);
>  
>  	switch (cmd) {
> -#ifdef CONFIG_COMPAT_32BIT_TIME
> +#if !defined(CONFIG_64BIT) && defined(CONFIG_COMPAT_32BIT_TIME)
>  	case VIDIOC_DQEVENT_TIME32: {
>  		struct v4l2_event *ev = parg;
>  		struct v4l2_event_time32 ev32;

-- 
Regards,

Laurent Pinchart

[PATCH v3 2/8] media: v4l2-core: explicitly clear ioctl input data

[Arnd Bergmann]

From: Arnd Bergmann <arnd@arndb.de>

As seen from a recent syzbot bug report, mistakes in the compat ioctl
implementation can lead to uninitialized kernel stack data getting used
as input for driver ioctl handlers.

The reported bug is now fixed, but it's possible that other related
bugs are still present or get added in the future. As the drivers need
to check user input already, the possible impact is fairly low, but it
might still cause an information leak.

To be on the safe side, always clear the entire ioctl buffer before
calling the conversion handler functions that are meant to initialize
them.

Signed-off-by: Arnd Bergmann <arnd@arndb.de>
---
 drivers/media/v4l2-core/v4l2-ioctl.c | 2 ++
 1 file changed, 2 insertions(+)

diff --git a/drivers/media/v4l2-core/v4l2-ioctl.c b/drivers/media/v4l2-core/v4l2-ioctl.c
index 58df927aec7e..f19e56116e53 100644
--- a/drivers/media/v4l2-core/v4l2-ioctl.c
+++ b/drivers/media/v4l2-core/v4l2-ioctl.c
@@ -3124,8 +3124,10 @@ static int video_get_user(void __user *arg, void *parg,
 		if (copy_from_user(parg, (void __user *)arg, n))
 			err = -EFAULT;
 	} else if (in_compat_syscall()) {
+		memset(parg, 0, n);
 		err = v4l2_compat_get_user(arg, parg, cmd);
 	} else {
+		memset(parg, 0, n);
 		switch (cmd) {
 #if !defined(CONFIG_64BIT) && defined(CONFIG_COMPAT_32BIT_TIME)
 		case VIDIOC_QUERYBUF_TIME32:
-- 
2.29.2

Re: [PATCH v3 2/8] media: v4l2-core: explicitly clear ioctl input data

[Laurent Pinchart]

Hi Arnd,

Thank you for the patch.

On Mon, Jun 14, 2021 at 12:34:03PM +0200, Arnd Bergmann wrote:
> From: Arnd Bergmann <arnd@arndb.de>
> 
> As seen from a recent syzbot bug report, mistakes in the compat ioctl
> implementation can lead to uninitialized kernel stack data getting used
> as input for driver ioctl handlers.
> 
> The reported bug is now fixed, but it's possible that other related
> bugs are still present or get added in the future. As the drivers need
> to check user input already, the possible impact is fairly low, but it
> might still cause an information leak.
> 
> To be on the safe side, always clear the entire ioctl buffer before
> calling the conversion handler functions that are meant to initialize
> them.
> 
> Signed-off-by: Arnd Bergmann <arnd@arndb.de>
> ---
>  drivers/media/v4l2-core/v4l2-ioctl.c | 2 ++
>  1 file changed, 2 insertions(+)
> 
> diff --git a/drivers/media/v4l2-core/v4l2-ioctl.c b/drivers/media/v4l2-core/v4l2-ioctl.c
> index 58df927aec7e..f19e56116e53 100644
> --- a/drivers/media/v4l2-core/v4l2-ioctl.c
> +++ b/drivers/media/v4l2-core/v4l2-ioctl.c
> @@ -3124,8 +3124,10 @@ static int video_get_user(void __user *arg, void *parg,
>  		if (copy_from_user(parg, (void __user *)arg, n))
>  			err = -EFAULT;
>  	} else if (in_compat_syscall()) {
> +		memset(parg, 0, n);
>  		err = v4l2_compat_get_user(arg, parg, cmd);
>  	} else {
> +		memset(parg, 0, n);

This could possibly be moved with the #if block by making it cover the
whole switch, but I don't think this code path will be hit when cmd
isn't one of the values handled below, so it shouldn't matter.

Reviewed-by: Laurent Pinchart <laurent.pinchart@ideasonboard.com>

>  		switch (cmd) {
>  #if !defined(CONFIG_64BIT) && defined(CONFIG_COMPAT_32BIT_TIME)
>  		case VIDIOC_QUERYBUF_TIME32:

-- 
Regards,

Laurent Pinchart

[PATCH v3 3/8] media: v4l2-core: fix whitespace damage in video_get_user()

[Arnd Bergmann]

From: Arnd Bergmann <arnd@arndb.de>

The initialization was indented with an extra tab in most lines,
remove them to get the normal coding style.

Signed-off-by: Arnd Bergmann <arnd@arndb.de>
---
 drivers/media/v4l2-core/v4l2-ioctl.c | 24 ++++++++++++------------
 1 file changed, 12 insertions(+), 12 deletions(-)

diff --git a/drivers/media/v4l2-core/v4l2-ioctl.c b/drivers/media/v4l2-core/v4l2-ioctl.c
index f19e56116e53..d94389145479 100644
--- a/drivers/media/v4l2-core/v4l2-ioctl.c
+++ b/drivers/media/v4l2-core/v4l2-ioctl.c
@@ -3142,18 +3142,18 @@ static int video_get_user(void __user *arg, void *parg,
 
 			*vb = (struct v4l2_buffer) {
 				.index		= vb32.index,
-					.type		= vb32.type,
-					.bytesused	= vb32.bytesused,
-					.flags		= vb32.flags,
-					.field		= vb32.field,
-					.timestamp.tv_sec	= vb32.timestamp.tv_sec,
-					.timestamp.tv_usec	= vb32.timestamp.tv_usec,
-					.timecode	= vb32.timecode,
-					.sequence	= vb32.sequence,
-					.memory		= vb32.memory,
-					.m.userptr	= vb32.m.userptr,
-					.length		= vb32.length,
-					.request_fd	= vb32.request_fd,
+				.type		= vb32.type,
+				.bytesused	= vb32.bytesused,
+				.flags		= vb32.flags,
+				.field		= vb32.field,
+				.timestamp.tv_sec	= vb32.timestamp.tv_sec,
+				.timestamp.tv_usec	= vb32.timestamp.tv_usec,
+				.timecode	= vb32.timecode,
+				.sequence	= vb32.sequence,
+				.memory		= vb32.memory,
+				.m.userptr	= vb32.m.userptr,
+				.length		= vb32.length,
+				.request_fd	= vb32.request_fd,
 			};
 			break;
 		}
-- 
2.29.2

Re: [PATCH v3 3/8] media: v4l2-core: fix whitespace damage in video_get_user()

[Laurent Pinchart]

Hi Arnd,

Thank you for the patch.

On Mon, Jun 14, 2021 at 12:34:04PM +0200, Arnd Bergmann wrote:
> From: Arnd Bergmann <arnd@arndb.de>
> 
> The initialization was indented with an extra tab in most lines,
> remove them to get the normal coding style.
> 
> Signed-off-by: Arnd Bergmann <arnd@arndb.de>

Reviewed-by: Laurent Pinchart <laurent.pinchart@ideasonboard.com>

> ---
>  drivers/media/v4l2-core/v4l2-ioctl.c | 24 ++++++++++++------------
>  1 file changed, 12 insertions(+), 12 deletions(-)
> 
> diff --git a/drivers/media/v4l2-core/v4l2-ioctl.c b/drivers/media/v4l2-core/v4l2-ioctl.c
> index f19e56116e53..d94389145479 100644
> --- a/drivers/media/v4l2-core/v4l2-ioctl.c
> +++ b/drivers/media/v4l2-core/v4l2-ioctl.c
> @@ -3142,18 +3142,18 @@ static int video_get_user(void __user *arg, void *parg,
>  
>  			*vb = (struct v4l2_buffer) {
>  				.index		= vb32.index,
> -					.type		= vb32.type,
> -					.bytesused	= vb32.bytesused,
> -					.flags		= vb32.flags,
> -					.field		= vb32.field,
> -					.timestamp.tv_sec	= vb32.timestamp.tv_sec,
> -					.timestamp.tv_usec	= vb32.timestamp.tv_usec,
> -					.timecode	= vb32.timecode,
> -					.sequence	= vb32.sequence,
> -					.memory		= vb32.memory,
> -					.m.userptr	= vb32.m.userptr,
> -					.length		= vb32.length,
> -					.request_fd	= vb32.request_fd,
> +				.type		= vb32.type,
> +				.bytesused	= vb32.bytesused,
> +				.flags		= vb32.flags,
> +				.field		= vb32.field,
> +				.timestamp.tv_sec	= vb32.timestamp.tv_sec,
> +				.timestamp.tv_usec	= vb32.timestamp.tv_usec,
> +				.timecode	= vb32.timecode,
> +				.sequence	= vb32.sequence,
> +				.memory		= vb32.memory,
> +				.m.userptr	= vb32.m.userptr,
> +				.length		= vb32.length,
> +				.request_fd	= vb32.request_fd,
>  			};
>  			break;
>  		}

-- 
Regards,

Laurent Pinchart

[PATCH v3 4/8] media: subdev: remove VIDIOC_DQEVENT_TIME32 handling

[Arnd Bergmann]

From: Arnd Bergmann <arnd@arndb.de>

Converting the VIDIOC_DQEVENT_TIME32/VIDIOC_DQEVENT32/
VIDIOC_DQEVENT32_TIME32 arguments to the canonical form is done in common
code, but for some reason I ended up adding another conversion helper to
subdev_do_ioctl() as well. I must have concluded that this does not go
through the common conversion, but it has done that since the ioctl
handler was first added.

I assume this one is harmless as there should be no way to arrive here
from user space, but since it is dead code, it should just get removed.

Fixes: 1a6c0b36dd19 ("media: v4l2-core: fix VIDIOC_DQEVENT for time64 ABI")
Signed-off-by: Arnd Bergmann <arnd@arndb.de>
---
 drivers/media/v4l2-core/v4l2-subdev.c | 24 ------------------------
 1 file changed, 24 deletions(-)

diff --git a/drivers/media/v4l2-core/v4l2-subdev.c b/drivers/media/v4l2-core/v4l2-subdev.c
index 956dafab43d4..bf3aa9252458 100644
--- a/drivers/media/v4l2-core/v4l2-subdev.c
+++ b/drivers/media/v4l2-core/v4l2-subdev.c
@@ -428,30 +428,6 @@ static long subdev_do_ioctl(struct file *file, unsigned int cmd, void *arg)
 
 		return v4l2_event_dequeue(vfh, arg, file->f_flags & O_NONBLOCK);
 
-	case VIDIOC_DQEVENT_TIME32: {
-		struct v4l2_event_time32 *ev32 = arg;
-		struct v4l2_event ev = { };
-
-		if (!(sd->flags & V4L2_SUBDEV_FL_HAS_EVENTS))
-			return -ENOIOCTLCMD;
-
-		rval = v4l2_event_dequeue(vfh, &ev, file->f_flags & O_NONBLOCK);
-
-		*ev32 = (struct v4l2_event_time32) {
-			.type		= ev.type,
-			.pending	= ev.pending,
-			.sequence	= ev.sequence,
-			.timestamp.tv_sec  = ev.timestamp.tv_sec,
-			.timestamp.tv_nsec = ev.timestamp.tv_nsec,
-			.id		= ev.id,
-		};
-
-		memcpy(&ev32->u, &ev.u, sizeof(ev.u));
-		memcpy(&ev32->reserved, &ev.reserved, sizeof(ev.reserved));
-
-		return rval;
-	}
-
 	case VIDIOC_SUBSCRIBE_EVENT:
 		return v4l2_subdev_call(sd, core, subscribe_event, vfh, arg);
 
-- 
2.29.2

Re: [PATCH v3 4/8] media: subdev: remove VIDIOC_DQEVENT_TIME32 handling

[Laurent Pinchart]

Hi Arnd,

Thank you for the patch.

On Mon, Jun 14, 2021 at 12:34:05PM +0200, Arnd Bergmann wrote:
> From: Arnd Bergmann <arnd@arndb.de>
> 
> Converting the VIDIOC_DQEVENT_TIME32/VIDIOC_DQEVENT32/
> VIDIOC_DQEVENT32_TIME32 arguments to the canonical form is done in common
> code, but for some reason I ended up adding another conversion helper to
> subdev_do_ioctl() as well. I must have concluded that this does not go
> through the common conversion, but it has done that since the ioctl
> handler was first added.
> 
> I assume this one is harmless as there should be no way to arrive here
> from user space, but since it is dead code, it should just get removed.

If I'm not mistaken, this could be reached when
!CONFIG_COMPAT_32BIT_TIME, can't it ? Still, there's no need for this
code in that case, so it seems fine to me.

> Fixes: 1a6c0b36dd19 ("media: v4l2-core: fix VIDIOC_DQEVENT for time64 ABI")
> Signed-off-by: Arnd Bergmann <arnd@arndb.de>

With an updated commit message if the above is correct,

Reviewed-by: Laurent Pinchart <laurent.pinchart@ideasonboard.com>

> ---
>  drivers/media/v4l2-core/v4l2-subdev.c | 24 ------------------------
>  1 file changed, 24 deletions(-)
> 
> diff --git a/drivers/media/v4l2-core/v4l2-subdev.c b/drivers/media/v4l2-core/v4l2-subdev.c
> index 956dafab43d4..bf3aa9252458 100644
> --- a/drivers/media/v4l2-core/v4l2-subdev.c
> +++ b/drivers/media/v4l2-core/v4l2-subdev.c
> @@ -428,30 +428,6 @@ static long subdev_do_ioctl(struct file *file, unsigned int cmd, void *arg)
>  
>  		return v4l2_event_dequeue(vfh, arg, file->f_flags & O_NONBLOCK);
>  
> -	case VIDIOC_DQEVENT_TIME32: {
> -		struct v4l2_event_time32 *ev32 = arg;
> -		struct v4l2_event ev = { };
> -
> -		if (!(sd->flags & V4L2_SUBDEV_FL_HAS_EVENTS))
> -			return -ENOIOCTLCMD;
> -
> -		rval = v4l2_event_dequeue(vfh, &ev, file->f_flags & O_NONBLOCK);
> -
> -		*ev32 = (struct v4l2_event_time32) {
> -			.type		= ev.type,
> -			.pending	= ev.pending,
> -			.sequence	= ev.sequence,
> -			.timestamp.tv_sec  = ev.timestamp.tv_sec,
> -			.timestamp.tv_nsec = ev.timestamp.tv_nsec,
> -			.id		= ev.id,
> -		};
> -
> -		memcpy(&ev32->u, &ev.u, sizeof(ev.u));
> -		memcpy(&ev32->reserved, &ev.reserved, sizeof(ev.reserved));
> -
> -		return rval;
> -	}
> -
>  	case VIDIOC_SUBSCRIBE_EVENT:
>  		return v4l2_subdev_call(sd, core, subscribe_event, vfh, arg);
>  

-- 
Regards,

Laurent Pinchart

Re: [PATCH v3 4/8] media: subdev: remove VIDIOC_DQEVENT_TIME32 handling

[Arnd Bergmann]

On Mon, Jun 14, 2021 at 7:02 PM Laurent Pinchart
<laurent.pinchart@ideasonboard.com> wrote:
>
> On Mon, Jun 14, 2021 at 12:34:05PM +0200, Arnd Bergmann wrote:
> > From: Arnd Bergmann <arnd@arndb.de>
> >
> > Converting the VIDIOC_DQEVENT_TIME32/VIDIOC_DQEVENT32/
> > VIDIOC_DQEVENT32_TIME32 arguments to the canonical form is done in common
> > code, but for some reason I ended up adding another conversion helper to
> > subdev_do_ioctl() as well. I must have concluded that this does not go
> > through the common conversion, but it has done that since the ioctl
> > handler was first added.
> >
> > I assume this one is harmless as there should be no way to arrive here
> > from user space, but since it is dead code, it should just get removed.
>
> If I'm not mistaken, this could be reached when
> !CONFIG_COMPAT_32BIT_TIME, can't it ? Still, there's no need for this
> code in that case, so it seems fine to me.

Yes, that is correct, I missed that condition. We definitely should not handle
the command in that case.

Hans, since you mentioned you would pick up this patch, I assume  you
are going to reword the patch as you see fit. If you prefer me to resend it,
let me know.

       Arnd

Re: [PATCH v3 4/8] media: subdev: remove VIDIOC_DQEVENT_TIME32 handling

[Hans Verkuil]

On 15/06/2021 10:43, Arnd Bergmann wrote:
> On Mon, Jun 14, 2021 at 7:02 PM Laurent Pinchart
> <laurent.pinchart@ideasonboard.com> wrote:
>>
>> On Mon, Jun 14, 2021 at 12:34:05PM +0200, Arnd Bergmann wrote:
>>> From: Arnd Bergmann <arnd@arndb.de>
>>>
>>> Converting the VIDIOC_DQEVENT_TIME32/VIDIOC_DQEVENT32/
>>> VIDIOC_DQEVENT32_TIME32 arguments to the canonical form is done in common
>>> code, but for some reason I ended up adding another conversion helper to
>>> subdev_do_ioctl() as well. I must have concluded that this does not go
>>> through the common conversion, but it has done that since the ioctl
>>> handler was first added.
>>>
>>> I assume this one is harmless as there should be no way to arrive here
>>> from user space, but since it is dead code, it should just get removed.

I changed this to:

"I assume this one is harmless as there should be no way to arrive here
from user space if CONFIG_COMPAT_32BIT_TIME is set,"

If it is not set, then this will just fall into the default case and is
handled as if it is a potential custom ioctl, as you would expect.

Let me know if you have a better text, I can still update it.

Regards,

	Hans

>>
>> If I'm not mistaken, this could be reached when
>> !CONFIG_COMPAT_32BIT_TIME, can't it ? Still, there's no need for this
>> code in that case, so it seems fine to me.
> 
> Yes, that is correct, I missed that condition. We definitely should not handle
> the command in that case.
> 
> Hans, since you mentioned you would pick up this patch, I assume  you
> are going to reword the patch as you see fit. If you prefer me to resend it,
> let me know.
> 
>        Arnd
> 

Re: [PATCH v3 4/8] media: subdev: remove VIDIOC_DQEVENT_TIME32 handling

[Arnd Bergmann]

On Tue, Jun 15, 2021 at 10:48 AM Hans Verkuil <hverkuil-cisco@xs4all.nl> wrote:
>
> On 15/06/2021 10:43, Arnd Bergmann wrote:
> > On Mon, Jun 14, 2021 at 7:02 PM Laurent Pinchart
> > <laurent.pinchart@ideasonboard.com> wrote:
> >>
> >> On Mon, Jun 14, 2021 at 12:34:05PM +0200, Arnd Bergmann wrote:
> >>> From: Arnd Bergmann <arnd@arndb.de>
> >>>
> >>> Converting the VIDIOC_DQEVENT_TIME32/VIDIOC_DQEVENT32/
> >>> VIDIOC_DQEVENT32_TIME32 arguments to the canonical form is done in common
> >>> code, but for some reason I ended up adding another conversion helper to
> >>> subdev_do_ioctl() as well. I must have concluded that this does not go
> >>> through the common conversion, but it has done that since the ioctl
> >>> handler was first added.
> >>>
> >>> I assume this one is harmless as there should be no way to arrive here
> >>> from user space, but since it is dead code, it should just get removed.
>
> I changed this to:
>
> "I assume this one is harmless as there should be no way to arrive here
> from user space if CONFIG_COMPAT_32BIT_TIME is set,"
>
> If it is not set, then this will just fall into the default case and is
> handled as if it is a potential custom ioctl, as you would expect.
>
> Let me know if you have a better text, I can still update it.

Looks good. One more sentence I would add:

"On a 64-bit architecture, as well as a 32-bit architecture without
CONFIG_COMPAT_32BIT_TIME, handling this command is a mistake,
and the kernel should return an error".

         Arnd

[PATCH v3 5/8] media: v4l2-core: return -ENODEV from ioctl when not registered

[Arnd Bergmann]

From: Arnd Bergmann <arnd@arndb.de>

I spotted a minor difference is handling of unregistered devices
between native and compat ioctls: the native handler never tries
to call into the driver if a device is not marked as registered.

I did not check whether this can cause issues in the kernel, or
just a different between return codes, but it clearly makes
sense that both should behave the same way.

Signed-off-by: Arnd Bergmann <arnd@arndb.de>
---
 drivers/media/v4l2-core/v4l2-compat-ioctl32.c | 3 +++
 1 file changed, 3 insertions(+)

diff --git a/drivers/media/v4l2-core/v4l2-compat-ioctl32.c b/drivers/media/v4l2-core/v4l2-compat-ioctl32.c
index 0ca75f6784c5..47aff3b19742 100644
--- a/drivers/media/v4l2-core/v4l2-compat-ioctl32.c
+++ b/drivers/media/v4l2-core/v4l2-compat-ioctl32.c
@@ -1244,6 +1244,9 @@ long v4l2_compat_ioctl32(struct file *file, unsigned int cmd, unsigned long arg)
 	if (!file->f_op->unlocked_ioctl)
 		return ret;
 
+	if (!video_is_registered(vdev))
+		return -ENODEV;
+
 	if (_IOC_TYPE(cmd) == 'V' && _IOC_NR(cmd) < BASE_VIDIOC_PRIVATE)
 		ret = file->f_op->unlocked_ioctl(file, cmd,
 					(unsigned long)compat_ptr(arg));
-- 
2.29.2

Re: [PATCH v3 5/8] media: v4l2-core: return -ENODEV from ioctl when not registered

[Laurent Pinchart]

Hi Arnd,

Thank you for the patch.

On Mon, Jun 14, 2021 at 12:34:06PM +0200, Arnd Bergmann wrote:
> From: Arnd Bergmann <arnd@arndb.de>
> 
> I spotted a minor difference is handling of unregistered devices
> between native and compat ioctls: the native handler never tries
> to call into the driver if a device is not marked as registered.
> 
> I did not check whether this can cause issues in the kernel, or
> just a different between return codes, but it clearly makes
> sense that both should behave the same way.
> 
> Signed-off-by: Arnd Bergmann <arnd@arndb.de>

Reviewed-by: Laurent Pinchart <laurent.pinchart@ideasonboard.com>

> ---
>  drivers/media/v4l2-core/v4l2-compat-ioctl32.c | 3 +++
>  1 file changed, 3 insertions(+)
> 
> diff --git a/drivers/media/v4l2-core/v4l2-compat-ioctl32.c b/drivers/media/v4l2-core/v4l2-compat-ioctl32.c
> index 0ca75f6784c5..47aff3b19742 100644
> --- a/drivers/media/v4l2-core/v4l2-compat-ioctl32.c
> +++ b/drivers/media/v4l2-core/v4l2-compat-ioctl32.c
> @@ -1244,6 +1244,9 @@ long v4l2_compat_ioctl32(struct file *file, unsigned int cmd, unsigned long arg)
>  	if (!file->f_op->unlocked_ioctl)
>  		return ret;
>  
> +	if (!video_is_registered(vdev))
> +		return -ENODEV;
> +
>  	if (_IOC_TYPE(cmd) == 'V' && _IOC_NR(cmd) < BASE_VIDIOC_PRIVATE)
>  		ret = file->f_op->unlocked_ioctl(file, cmd,
>  					(unsigned long)compat_ptr(arg));

-- 
Regards,

Laurent Pinchart

Re: [PATCH v3 5/8] media: v4l2-core: return -ENODEV from ioctl when not registered

[Laurent Pinchart]

Hi Arnd,

Thank you for the patch.

On Mon, Jun 14, 2021 at 12:34:06PM +0200, Arnd Bergmann wrote:
> From: Arnd Bergmann <arnd@arndb.de>
> 
> I spotted a minor difference is handling of unregistered devices
> between native and compat ioctls: the native handler never tries
> to call into the driver if a device is not marked as registered.
> 
> I did not check whether this can cause issues in the kernel, or
> just a different between return codes, but it clearly makes
> sense that both should behave the same way.
> 
> Signed-off-by: Arnd Bergmann <arnd@arndb.de>

Reviewed-by: Laurent Pinchart <laurent.pinchart@ideasonboard.com>

> ---
>  drivers/media/v4l2-core/v4l2-compat-ioctl32.c | 3 +++
>  1 file changed, 3 insertions(+)
> 
> diff --git a/drivers/media/v4l2-core/v4l2-compat-ioctl32.c b/drivers/media/v4l2-core/v4l2-compat-ioctl32.c
> index 0ca75f6784c5..47aff3b19742 100644
> --- a/drivers/media/v4l2-core/v4l2-compat-ioctl32.c
> +++ b/drivers/media/v4l2-core/v4l2-compat-ioctl32.c
> @@ -1244,6 +1244,9 @@ long v4l2_compat_ioctl32(struct file *file, unsigned int cmd, unsigned long arg)
>  	if (!file->f_op->unlocked_ioctl)
>  		return ret;
>  
> +	if (!video_is_registered(vdev))
> +		return -ENODEV;
> +
>  	if (_IOC_TYPE(cmd) == 'V' && _IOC_NR(cmd) < BASE_VIDIOC_PRIVATE)
>  		ret = file->f_op->unlocked_ioctl(file, cmd,
>  					(unsigned long)compat_ptr(arg));

-- 
Regards,

Laurent Pinchart

[PATCH v3 6/8] media: atomisp: remove compat_ioctl32 code

[Arnd Bergmann]

From: Arnd Bergmann <arnd@arndb.de>

This is one of the last remaining users of compat_alloc_user_space()
and copy_in_user(), which are in the process of getting removed.

As of commit 57e6b6f2303e ("media: atomisp_fops.c: disable
atomisp_compat_ioctl32"), nothing in this file is actually getting used
as the only reference has been stubbed out.

Remove the entire file -- anyone willing to restore the functionality
can equally well just look up the contents in the git history if needed.

Cc: Sakari Ailus <sakari.ailus@linux.intel.com>
Cc: Hans Verkuil <hverkuil@xs4all.nl>
Acked-by: Sakari Ailus <sakari.ailus@linux.intel.com>
Suggested-by: Christoph Hellwig <hch@lst.de>
Reviewed-by: Andy Shevchenko <andriy.shevchenko@linux.intel.com>
Signed-off-by: Arnd Bergmann <arnd@arndb.de>
---
 drivers/staging/media/atomisp/Makefile        |    1 -
 drivers/staging/media/atomisp/TODO            |    5 +
 .../atomisp/pci/atomisp_compat_ioctl32.c      | 1202 -----------------
 .../staging/media/atomisp/pci/atomisp_fops.c  |    8 +-
 4 files changed, 8 insertions(+), 1208 deletions(-)
 delete mode 100644 drivers/staging/media/atomisp/pci/atomisp_compat_ioctl32.c

diff --git a/drivers/staging/media/atomisp/Makefile b/drivers/staging/media/atomisp/Makefile
index 51498b2e85b8..606b7754fdfd 100644
--- a/drivers/staging/media/atomisp/Makefile
+++ b/drivers/staging/media/atomisp/Makefile
@@ -16,7 +16,6 @@ atomisp-objs += \
 	pci/atomisp_acc.o \
 	pci/atomisp_cmd.o \
 	pci/atomisp_compat_css20.o \
-	pci/atomisp_compat_ioctl32.o \
 	pci/atomisp_csi2.o \
 	pci/atomisp_drvfs.o \
 	pci/atomisp_file.o \
diff --git a/drivers/staging/media/atomisp/TODO b/drivers/staging/media/atomisp/TODO
index 6987bb2d32cf..2d1ef9eb262a 100644
--- a/drivers/staging/media/atomisp/TODO
+++ b/drivers/staging/media/atomisp/TODO
@@ -120,6 +120,11 @@ TODO
     for this driver until the other work is done, as there will be a lot
     of code churn until this driver becomes functional again.
 
+16. Fix private ioctls to not need a compat_ioctl handler for running
+    32-bit tasks. The compat code has been removed because of bugs,
+    and should not be needed for modern drivers. Fixing this properly
+    unfortunately means an incompatible ABI change.
+
 Limitations
 ===========
 
diff --git a/drivers/staging/media/atomisp/pci/atomisp_compat_ioctl32.c b/drivers/staging/media/atomisp/pci/atomisp_compat_ioctl32.c
deleted file mode 100644
index e5553df5bad4..000000000000
--- a/drivers/staging/media/atomisp/pci/atomisp_compat_ioctl32.c
+++ /dev/null
@@ -1,1202 +0,0 @@
-// SPDX-License-Identifier: GPL-2.0
-/*
- * Support for Intel Camera Imaging ISP subsystem.
- *
- * Copyright (c) 2013 Intel Corporation. All Rights Reserved.
- *
- * This program is free software; you can redistribute it and/or
- * modify it under the terms of the GNU General Public License version
- * 2 as published by the Free Software Foundation.
- *
- * This program is distributed in the hope that it will be useful,
- * but WITHOUT ANY WARRANTY; without even the implied warranty of
- * MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE.  See the
- * GNU General Public License for more details.
- *
- *
- */
-#ifdef CONFIG_COMPAT
-#include <linux/compat.h>
-
-#include <linux/videodev2.h>
-
-#include "atomisp_internal.h"
-#include "atomisp_compat.h"
-#include "atomisp_ioctl.h"
-#include "atomisp_compat_ioctl32.h"
-
-/* Macros borrowed from v4l2-compat-ioctl32.c */
-
-#define get_user_cast(__x, __ptr)					\
-({									\
-	get_user(__x, (typeof(*__ptr) __user *)(__ptr));		\
-})
-
-#define put_user_force(__x, __ptr)					\
-({									\
-	put_user((typeof(*__x) __force *)(__x), __ptr);			\
-})
-
-/* Use the same argument order as copy_in_user */
-#define assign_in_user(to, from)					\
-({									\
-	typeof(*from) __assign_tmp;					\
-									\
-	get_user_cast(__assign_tmp, from) || put_user(__assign_tmp, to);\
-})
-
-static int get_atomisp_histogram32(struct atomisp_histogram __user *kp,
-				   struct atomisp_histogram32 __user *up)
-{
-	compat_uptr_t tmp;
-
-	if (!access_ok(up, sizeof(struct atomisp_histogram32)) ||
-	    assign_in_user(&kp->num_elements, &up->num_elements) ||
-	    get_user(tmp, &up->data) ||
-	    put_user(compat_ptr(tmp), &kp->data))
-		return -EFAULT;
-
-	return 0;
-}
-
-static int put_atomisp_histogram32(struct atomisp_histogram __user *kp,
-				   struct atomisp_histogram32 __user *up)
-{
-	void __user *tmp;
-
-	if (!access_ok(up, sizeof(struct atomisp_histogram32)) ||
-	    assign_in_user(&up->num_elements, &kp->num_elements) ||
-	    get_user(tmp, &kp->data) ||
-	    put_user(ptr_to_compat(tmp), &up->data))
-		return -EFAULT;
-
-	return 0;
-}
-
-static int get_v4l2_framebuffer32(struct v4l2_framebuffer __user *kp,
-				  struct v4l2_framebuffer32 __user *up)
-{
-	compat_uptr_t tmp;
-
-	if (!access_ok(up, sizeof(struct v4l2_framebuffer32)) ||
-	    get_user(tmp, &up->base) ||
-	    put_user_force(compat_ptr(tmp), &kp->base) ||
-	    assign_in_user(&kp->capability, &up->capability) ||
-	    assign_in_user(&kp->flags, &up->flags) ||
-	    copy_in_user(&kp->fmt, &up->fmt, sizeof(kp->fmt)))
-		return -EFAULT;
-
-	return 0;
-}
-
-static int get_atomisp_dis_statistics32(struct atomisp_dis_statistics __user *kp,
-					struct atomisp_dis_statistics32 __user *up)
-{
-	compat_uptr_t hor_prod_odd_real;
-	compat_uptr_t hor_prod_odd_imag;
-	compat_uptr_t hor_prod_even_real;
-	compat_uptr_t hor_prod_even_imag;
-	compat_uptr_t ver_prod_odd_real;
-	compat_uptr_t ver_prod_odd_imag;
-	compat_uptr_t ver_prod_even_real;
-	compat_uptr_t ver_prod_even_imag;
-
-	if (!access_ok(up, sizeof(struct atomisp_dis_statistics32)) ||
-	    copy_in_user(kp, up, sizeof(struct atomisp_dvs_grid_info)) ||
-	    get_user(hor_prod_odd_real,
-		     &up->dvs2_stat.hor_prod.odd_real) ||
-	    get_user(hor_prod_odd_imag,
-		     &up->dvs2_stat.hor_prod.odd_imag) ||
-	    get_user(hor_prod_even_real,
-		     &up->dvs2_stat.hor_prod.even_real) ||
-	    get_user(hor_prod_even_imag,
-		     &up->dvs2_stat.hor_prod.even_imag) ||
-	    get_user(ver_prod_odd_real,
-		     &up->dvs2_stat.ver_prod.odd_real) ||
-	    get_user(ver_prod_odd_imag,
-		     &up->dvs2_stat.ver_prod.odd_imag) ||
-	    get_user(ver_prod_even_real,
-		     &up->dvs2_stat.ver_prod.even_real) ||
-	    get_user(ver_prod_even_imag,
-		     &up->dvs2_stat.ver_prod.even_imag) ||
-	    assign_in_user(&kp->exp_id, &up->exp_id) ||
-	    put_user(compat_ptr(hor_prod_odd_real),
-		     &kp->dvs2_stat.hor_prod.odd_real) ||
-	    put_user(compat_ptr(hor_prod_odd_imag),
-		     &kp->dvs2_stat.hor_prod.odd_imag) ||
-	    put_user(compat_ptr(hor_prod_even_real),
-		     &kp->dvs2_stat.hor_prod.even_real) ||
-	    put_user(compat_ptr(hor_prod_even_imag),
-		     &kp->dvs2_stat.hor_prod.even_imag) ||
-	    put_user(compat_ptr(ver_prod_odd_real),
-		     &kp->dvs2_stat.ver_prod.odd_real) ||
-	    put_user(compat_ptr(ver_prod_odd_imag),
-		     &kp->dvs2_stat.ver_prod.odd_imag) ||
-	    put_user(compat_ptr(ver_prod_even_real),
-		     &kp->dvs2_stat.ver_prod.even_real) ||
-	    put_user(compat_ptr(ver_prod_even_imag),
-		     &kp->dvs2_stat.ver_prod.even_imag))
-		return -EFAULT;
-
-	return 0;
-}
-
-static int put_atomisp_dis_statistics32(struct atomisp_dis_statistics __user *kp,
-					struct atomisp_dis_statistics32 __user *up)
-{
-	void __user *hor_prod_odd_real;
-	void __user *hor_prod_odd_imag;
-	void __user *hor_prod_even_real;
-	void __user *hor_prod_even_imag;
-	void __user *ver_prod_odd_real;
-	void __user *ver_prod_odd_imag;
-	void __user *ver_prod_even_real;
-	void __user *ver_prod_even_imag;
-
-	if (!!access_ok(up, sizeof(struct atomisp_dis_statistics32)) ||
-	    copy_in_user(up, kp, sizeof(struct atomisp_dvs_grid_info)) ||
-	    get_user(hor_prod_odd_real,
-		     &kp->dvs2_stat.hor_prod.odd_real) ||
-	    get_user(hor_prod_odd_imag,
-		     &kp->dvs2_stat.hor_prod.odd_imag) ||
-	    get_user(hor_prod_even_real,
-		     &kp->dvs2_stat.hor_prod.even_real) ||
-	    get_user(hor_prod_even_imag,
-		     &kp->dvs2_stat.hor_prod.even_imag) ||
-	    get_user(ver_prod_odd_real,
-		     &kp->dvs2_stat.ver_prod.odd_real) ||
-	    get_user(ver_prod_odd_imag,
-		     &kp->dvs2_stat.ver_prod.odd_imag) ||
-	    get_user(ver_prod_even_real,
-		     &kp->dvs2_stat.ver_prod.even_real) ||
-	    get_user(ver_prod_even_imag,
-		     &kp->dvs2_stat.ver_prod.even_imag) ||
-	    put_user(ptr_to_compat(hor_prod_odd_real),
-		     &up->dvs2_stat.hor_prod.odd_real) ||
-	    put_user(ptr_to_compat(hor_prod_odd_imag),
-		     &up->dvs2_stat.hor_prod.odd_imag) ||
-	    put_user(ptr_to_compat(hor_prod_even_real),
-		     &up->dvs2_stat.hor_prod.even_real) ||
-	    put_user(ptr_to_compat(hor_prod_even_imag),
-		     &up->dvs2_stat.hor_prod.even_imag) ||
-	    put_user(ptr_to_compat(ver_prod_odd_real),
-		     &up->dvs2_stat.ver_prod.odd_real) ||
-	    put_user(ptr_to_compat(ver_prod_odd_imag),
-		     &up->dvs2_stat.ver_prod.odd_imag) ||
-	    put_user(ptr_to_compat(ver_prod_even_real),
-		     &up->dvs2_stat.ver_prod.even_real) ||
-	    put_user(ptr_to_compat(ver_prod_even_imag),
-		     &up->dvs2_stat.ver_prod.even_imag) ||
-	    assign_in_user(&up->exp_id, &kp->exp_id))
-		return -EFAULT;
-
-	return 0;
-}
-
-static int get_atomisp_dis_coefficients32(struct atomisp_dis_coefficients __user *kp,
-					  struct atomisp_dis_coefficients32 __user *up)
-{
-	compat_uptr_t hor_coefs_odd_real;
-	compat_uptr_t hor_coefs_odd_imag;
-	compat_uptr_t hor_coefs_even_real;
-	compat_uptr_t hor_coefs_even_imag;
-	compat_uptr_t ver_coefs_odd_real;
-	compat_uptr_t ver_coefs_odd_imag;
-	compat_uptr_t ver_coefs_even_real;
-	compat_uptr_t ver_coefs_even_imag;
-
-	if (!access_ok(up, sizeof(struct atomisp_dis_coefficients32)) ||
-	    copy_in_user(kp, up, sizeof(struct atomisp_dvs_grid_info)) ||
-	    get_user(hor_coefs_odd_real, &up->hor_coefs.odd_real) ||
-	    get_user(hor_coefs_odd_imag, &up->hor_coefs.odd_imag) ||
-	    get_user(hor_coefs_even_real, &up->hor_coefs.even_real) ||
-	    get_user(hor_coefs_even_imag, &up->hor_coefs.even_imag) ||
-	    get_user(ver_coefs_odd_real, &up->ver_coefs.odd_real) ||
-	    get_user(ver_coefs_odd_imag, &up->ver_coefs.odd_imag) ||
-	    get_user(ver_coefs_even_real, &up->ver_coefs.even_real) ||
-	    get_user(ver_coefs_even_imag, &up->ver_coefs.even_imag) ||
-	    put_user(compat_ptr(hor_coefs_odd_real),
-		     &kp->hor_coefs.odd_real) ||
-	    put_user(compat_ptr(hor_coefs_odd_imag),
-		     &kp->hor_coefs.odd_imag) ||
-	    put_user(compat_ptr(hor_coefs_even_real),
-		     &kp->hor_coefs.even_real) ||
-	    put_user(compat_ptr(hor_coefs_even_imag),
-		     &kp->hor_coefs.even_imag) ||
-	    put_user(compat_ptr(ver_coefs_odd_real),
-		     &kp->ver_coefs.odd_real) ||
-	    put_user(compat_ptr(ver_coefs_odd_imag),
-		     &kp->ver_coefs.odd_imag) ||
-	    put_user(compat_ptr(ver_coefs_even_real),
-		     &kp->ver_coefs.even_real) ||
-	    put_user(compat_ptr(ver_coefs_even_imag),
-		     &kp->ver_coefs.even_imag))
-		return -EFAULT;
-
-	return 0;
-}
-
-static int get_atomisp_dvs_6axis_config32(struct atomisp_dvs_6axis_config __user *kp,
-					  struct atomisp_dvs_6axis_config32 __user *up)
-{
-	compat_uptr_t xcoords_y;
-	compat_uptr_t ycoords_y;
-	compat_uptr_t xcoords_uv;
-	compat_uptr_t ycoords_uv;
-
-	if (!access_ok(up, sizeof(struct atomisp_dvs_6axis_config32)) ||
-	    assign_in_user(&kp->exp_id, &up->exp_id) ||
-	    assign_in_user(&kp->width_y, &up->width_y) ||
-	    assign_in_user(&kp->height_y, &up->height_y) ||
-	    assign_in_user(&kp->width_uv, &up->width_uv) ||
-	    assign_in_user(&kp->height_uv, &up->height_uv) ||
-	    get_user(xcoords_y, &up->xcoords_y) ||
-	    get_user(ycoords_y, &up->ycoords_y) ||
-	    get_user(xcoords_uv, &up->xcoords_uv) ||
-	    get_user(ycoords_uv, &up->ycoords_uv) ||
-	    put_user_force(compat_ptr(xcoords_y), &kp->xcoords_y) ||
-	    put_user_force(compat_ptr(ycoords_y), &kp->ycoords_y) ||
-	    put_user_force(compat_ptr(xcoords_uv), &kp->xcoords_uv) ||
-	    put_user_force(compat_ptr(ycoords_uv), &kp->ycoords_uv))
-		return -EFAULT;
-
-	return 0;
-}
-
-static int get_atomisp_3a_statistics32(struct atomisp_3a_statistics __user *kp,
-				       struct atomisp_3a_statistics32 __user *up)
-{
-	compat_uptr_t data;
-	compat_uptr_t rgby_data;
-
-	if (!access_ok(up, sizeof(struct atomisp_3a_statistics32)) ||
-	    copy_in_user(kp, up, sizeof(struct atomisp_grid_info)) ||
-	    get_user(rgby_data, &up->rgby_data) ||
-	    put_user(compat_ptr(rgby_data), &kp->rgby_data) ||
-	    get_user(data, &up->data) ||
-	    put_user(compat_ptr(data), &kp->data) ||
-	    assign_in_user(&kp->exp_id, &up->exp_id) ||
-	    assign_in_user(&kp->isp_config_id, &up->isp_config_id))
-		return -EFAULT;
-
-	return 0;
-}
-
-static int put_atomisp_3a_statistics32(struct atomisp_3a_statistics __user *kp,
-				       struct atomisp_3a_statistics32 __user *up)
-{
-	void __user *data;
-	void __user *rgby_data;
-
-	if (!access_ok(up, sizeof(struct atomisp_3a_statistics32)) ||
-	    copy_in_user(up, kp, sizeof(struct atomisp_grid_info)) ||
-	    get_user(rgby_data, &kp->rgby_data) ||
-	    put_user(ptr_to_compat(rgby_data), &up->rgby_data) ||
-	    get_user(data, &kp->data) ||
-	    put_user(ptr_to_compat(data), &up->data) ||
-	    assign_in_user(&up->exp_id, &kp->exp_id) ||
-	    assign_in_user(&up->isp_config_id, &kp->isp_config_id))
-		return -EFAULT;
-
-	return 0;
-}
-
-static int get_atomisp_metadata_stat32(struct atomisp_metadata __user *kp,
-				       struct atomisp_metadata32 __user *up)
-{
-	compat_uptr_t data;
-	compat_uptr_t effective_width;
-
-	if (!access_ok(up, sizeof(struct atomisp_metadata32)) ||
-	    get_user(data, &up->data) ||
-	    put_user(compat_ptr(data), &kp->data) ||
-	    assign_in_user(&kp->width, &up->width) ||
-	    assign_in_user(&kp->height, &up->height) ||
-	    assign_in_user(&kp->stride, &up->stride) ||
-	    assign_in_user(&kp->exp_id, &up->exp_id) ||
-	    get_user(effective_width, &up->effective_width) ||
-	    put_user_force(compat_ptr(effective_width), &kp->effective_width))
-		return -EFAULT;
-
-	return 0;
-}
-
-static int put_atomisp_metadata_stat32(struct atomisp_metadata __user *kp,
-				struct atomisp_metadata32 __user *up)
-{
-	void __user *data;
-	void *effective_width;
-
-	if (!access_ok(up, sizeof(struct atomisp_metadata32)) ||
-	    get_user(data, &kp->data) ||
-	    put_user(ptr_to_compat(data), &up->data) ||
-	    assign_in_user(&up->width, &kp->width) ||
-	    assign_in_user(&up->height, &kp->height) ||
-	    assign_in_user(&up->stride, &kp->stride) ||
-	    assign_in_user(&up->exp_id, &kp->exp_id) ||
-	    get_user(effective_width, &kp->effective_width) ||
-	    put_user(ptr_to_compat((void __user *)effective_width),
-				   &up->effective_width))
-		return -EFAULT;
-
-	return 0;
-}
-
-static int
-put_atomisp_metadata_by_type_stat32(struct atomisp_metadata_with_type __user *kp,
-				    struct atomisp_metadata_with_type32 __user *up)
-{
-	void __user *data;
-	u32 *effective_width;
-
-	if (!access_ok(up, sizeof(struct atomisp_metadata_with_type32)) ||
-	    get_user(data, &kp->data) ||
-	    put_user(ptr_to_compat(data), &up->data) ||
-	    assign_in_user(&up->width, &kp->width) ||
-	    assign_in_user(&up->height, &kp->height) ||
-	    assign_in_user(&up->stride, &kp->stride) ||
-	    assign_in_user(&up->exp_id, &kp->exp_id) ||
-	    get_user(effective_width, &kp->effective_width) ||
-	    put_user(ptr_to_compat((void __user *)effective_width),
-		     &up->effective_width) ||
-	    assign_in_user(&up->type, &kp->type))
-		return -EFAULT;
-
-	return 0;
-}
-
-static int
-get_atomisp_metadata_by_type_stat32(struct atomisp_metadata_with_type __user *kp,
-				    struct atomisp_metadata_with_type32 __user *up)
-{
-	compat_uptr_t data;
-	compat_uptr_t effective_width;
-
-	if (!access_ok(up, sizeof(struct atomisp_metadata_with_type32)) ||
-	    get_user(data, &up->data) ||
-	    put_user(compat_ptr(data), &kp->data) ||
-	    assign_in_user(&kp->width, &up->width) ||
-	    assign_in_user(&kp->height, &up->height) ||
-	    assign_in_user(&kp->stride, &up->stride) ||
-	    assign_in_user(&kp->exp_id, &up->exp_id) ||
-	    get_user(effective_width, &up->effective_width) ||
-	    put_user_force(compat_ptr(effective_width), &kp->effective_width) ||
-	    assign_in_user(&kp->type, &up->type))
-		return -EFAULT;
-
-	return 0;
-}
-
-static int
-get_atomisp_morph_table32(struct atomisp_morph_table __user *kp,
-			  struct atomisp_morph_table32 __user *up)
-{
-	unsigned int n = ATOMISP_MORPH_TABLE_NUM_PLANES;
-
-	if (!access_ok(up, sizeof(struct atomisp_morph_table32)) ||
-		assign_in_user(&kp->enabled, &up->enabled) ||
-		assign_in_user(&kp->width, &up->width) ||
-		assign_in_user(&kp->height, &up->height))
-			return -EFAULT;
-
-	while (n-- > 0) {
-		compat_uptr_t coord_kp;
-
-		if (get_user(coord_kp, &up->coordinates_x[n]) ||
-		    put_user(compat_ptr(coord_kp), &kp->coordinates_x[n]) ||
-		    get_user(coord_kp, &up->coordinates_y[n]) ||
-		    put_user(compat_ptr(coord_kp), &kp->coordinates_y[n]))
-			return -EFAULT;
-	}
-	return 0;
-}
-
-static int put_atomisp_morph_table32(struct atomisp_morph_table __user *kp,
-				     struct atomisp_morph_table32 __user *up)
-{
-	unsigned int n = ATOMISP_MORPH_TABLE_NUM_PLANES;
-
-	if (!access_ok(up, sizeof(struct atomisp_morph_table32)) ||
-		assign_in_user(&up->enabled, &kp->enabled) ||
-		assign_in_user(&up->width, &kp->width) ||
-		assign_in_user(&up->height, &kp->height))
-			return -EFAULT;
-
-	while (n-- > 0) {
-		void __user *coord_kp;
-
-		if (get_user(coord_kp, &kp->coordinates_x[n]) ||
-		    put_user(ptr_to_compat(coord_kp), &up->coordinates_x[n]) ||
-		    get_user(coord_kp, &kp->coordinates_y[n]) ||
-		    put_user(ptr_to_compat(coord_kp), &up->coordinates_y[n]))
-			return -EFAULT;
-	}
-	return 0;
-}
-
-static int get_atomisp_overlay32(struct atomisp_overlay __user *kp,
-				 struct atomisp_overlay32 __user *up)
-{
-	compat_uptr_t frame;
-
-	if (!access_ok(up, sizeof(struct atomisp_overlay32)) ||
-	    get_user(frame, &up->frame) ||
-	    put_user_force(compat_ptr(frame), &kp->frame) ||
-	    assign_in_user(&kp->bg_y, &up->bg_y) ||
-	    assign_in_user(&kp->bg_u, &up->bg_u) ||
-	    assign_in_user(&kp->bg_v, &up->bg_v) ||
-	    assign_in_user(&kp->blend_input_perc_y,
-			   &up->blend_input_perc_y) ||
-	    assign_in_user(&kp->blend_input_perc_u,
-			   &up->blend_input_perc_u) ||
-	    assign_in_user(&kp->blend_input_perc_v,
-			   &up->blend_input_perc_v) ||
-	    assign_in_user(&kp->blend_overlay_perc_y,
-			   &up->blend_overlay_perc_y) ||
-	    assign_in_user(&kp->blend_overlay_perc_u,
-			   &up->blend_overlay_perc_u) ||
-	    assign_in_user(&kp->blend_overlay_perc_v,
-			   &up->blend_overlay_perc_v) ||
-	    assign_in_user(&kp->overlay_start_x, &up->overlay_start_x) ||
-	    assign_in_user(&kp->overlay_start_y, &up->overlay_start_y))
-		return -EFAULT;
-
-	return 0;
-}
-
-static int put_atomisp_overlay32(struct atomisp_overlay __user *kp,
-				 struct atomisp_overlay32 __user *up)
-{
-	void *frame;
-
-	if (!access_ok(up, sizeof(struct atomisp_overlay32)) ||
-	    get_user(frame, &kp->frame) ||
-	    put_user(ptr_to_compat((void __user *)frame), &up->frame) ||
-	    assign_in_user(&up->bg_y, &kp->bg_y) ||
-	    assign_in_user(&up->bg_u, &kp->bg_u) ||
-	    assign_in_user(&up->bg_v, &kp->bg_v) ||
-	    assign_in_user(&up->blend_input_perc_y,
-			   &kp->blend_input_perc_y) ||
-	    assign_in_user(&up->blend_input_perc_u,
-			   &kp->blend_input_perc_u) ||
-	    assign_in_user(&up->blend_input_perc_v,
-			   &kp->blend_input_perc_v) ||
-	    assign_in_user(&up->blend_overlay_perc_y,
-			   &kp->blend_overlay_perc_y) ||
-	    assign_in_user(&up->blend_overlay_perc_u,
-			   &kp->blend_overlay_perc_u) ||
-	    assign_in_user(&up->blend_overlay_perc_v,
-			   &kp->blend_overlay_perc_v) ||
-	    assign_in_user(&up->overlay_start_x, &kp->overlay_start_x) ||
-	    assign_in_user(&up->overlay_start_y, &kp->overlay_start_y))
-		return -EFAULT;
-
-	return 0;
-}
-
-static int
-get_atomisp_calibration_group32(struct atomisp_calibration_group __user *kp,
-				struct atomisp_calibration_group32 __user *up)
-{
-	compat_uptr_t calb_grp_values;
-
-	if (!access_ok(up, sizeof(struct atomisp_calibration_group32)) ||
-	    assign_in_user(&kp->size, &up->size) ||
-	    assign_in_user(&kp->type, &up->type) ||
-	    get_user(calb_grp_values, &up->calb_grp_values) ||
-	    put_user_force(compat_ptr(calb_grp_values), &kp->calb_grp_values))
-		return -EFAULT;
-
-	return 0;
-}
-
-static int
-put_atomisp_calibration_group32(struct atomisp_calibration_group __user *kp,
-				struct atomisp_calibration_group32 __user *up)
-{
-	void *calb_grp_values;
-
-	if (!access_ok(up, sizeof(struct atomisp_calibration_group32)) ||
-	    assign_in_user(&up->size, &kp->size) ||
-	    assign_in_user(&up->type, &kp->type) ||
-	    get_user(calb_grp_values, &kp->calb_grp_values) ||
-	    put_user(ptr_to_compat((void __user *)calb_grp_values),
-		     &up->calb_grp_values))
-		return -EFAULT;
-
-	return 0;
-}
-
-static int get_atomisp_acc_fw_load32(struct atomisp_acc_fw_load __user *kp,
-				     struct atomisp_acc_fw_load32 __user *up)
-{
-	compat_uptr_t data;
-
-	if (!access_ok(up, sizeof(struct atomisp_acc_fw_load32)) ||
-	    assign_in_user(&kp->size, &up->size) ||
-	    assign_in_user(&kp->fw_handle, &up->fw_handle) ||
-	    get_user_cast(data, &up->data) ||
-	    put_user(compat_ptr(data), &kp->data))
-		return -EFAULT;
-
-	return 0;
-}
-
-static int put_atomisp_acc_fw_load32(struct atomisp_acc_fw_load __user *kp,
-				     struct atomisp_acc_fw_load32 __user *up)
-{
-	void __user *data;
-
-	if (!access_ok(up, sizeof(struct atomisp_acc_fw_load32)) ||
-	    assign_in_user(&up->size, &kp->size) ||
-	    assign_in_user(&up->fw_handle, &kp->fw_handle) ||
-	    get_user(data, &kp->data) ||
-	    put_user(ptr_to_compat(data), &up->data))
-		return -EFAULT;
-
-	return 0;
-}
-
-static int get_atomisp_acc_fw_arg32(struct atomisp_acc_fw_arg __user *kp,
-				    struct atomisp_acc_fw_arg32 __user *up)
-{
-	compat_uptr_t value;
-
-	if (!access_ok(up, sizeof(struct atomisp_acc_fw_arg32)) ||
-	    assign_in_user(&kp->fw_handle, &up->fw_handle) ||
-	    assign_in_user(&kp->index, &up->index) ||
-	    get_user(value, &up->value) ||
-	    put_user(compat_ptr(value), &kp->value) ||
-	    assign_in_user(&kp->size, &up->size))
-		return -EFAULT;
-
-	return 0;
-}
-
-static int put_atomisp_acc_fw_arg32(struct atomisp_acc_fw_arg __user *kp,
-				    struct atomisp_acc_fw_arg32 __user *up)
-{
-	void __user *value;
-
-	if (!access_ok(up, sizeof(struct atomisp_acc_fw_arg32)) ||
-	    assign_in_user(&up->fw_handle, &kp->fw_handle) ||
-	    assign_in_user(&up->index, &kp->index) ||
-	    get_user(value, &kp->value) ||
-	    put_user(ptr_to_compat(value), &up->value) ||
-	    assign_in_user(&up->size, &kp->size))
-		return -EFAULT;
-
-	return 0;
-}
-
-static int get_v4l2_private_int_data32(struct v4l2_private_int_data __user *kp,
-				       struct v4l2_private_int_data32 __user *up)
-{
-	compat_uptr_t data;
-
-	if (!access_ok(up, sizeof(struct v4l2_private_int_data32)) ||
-	    assign_in_user(&kp->size, &up->size) ||
-	    get_user(data, &up->data) ||
-	    put_user(compat_ptr(data), &kp->data) ||
-	    assign_in_user(&kp->reserved[0], &up->reserved[0]) ||
-	    assign_in_user(&kp->reserved[1], &up->reserved[1]))
-		return -EFAULT;
-
-	return 0;
-}
-
-static int put_v4l2_private_int_data32(struct v4l2_private_int_data __user *kp,
-				       struct v4l2_private_int_data32 __user *up)
-{
-	void __user *data;
-
-	if (!access_ok(up, sizeof(struct v4l2_private_int_data32)) ||
-	    assign_in_user(&up->size, &kp->size) ||
-	    get_user(data, &kp->data) ||
-	    put_user(ptr_to_compat(data), &up->data) ||
-	    assign_in_user(&up->reserved[0], &kp->reserved[0]) ||
-	    assign_in_user(&up->reserved[1], &kp->reserved[1]))
-		return -EFAULT;
-
-	return 0;
-}
-
-static int get_atomisp_shading_table32(struct atomisp_shading_table __user *kp,
-				       struct atomisp_shading_table32 __user *up)
-{
-	unsigned int n = ATOMISP_NUM_SC_COLORS;
-
-	if (!access_ok(up, sizeof(struct atomisp_shading_table32)) ||
-	    assign_in_user(&kp->enable, &up->enable) ||
-	    assign_in_user(&kp->sensor_width, &up->sensor_width) ||
-	    assign_in_user(&kp->sensor_height, &up->sensor_height) ||
-	    assign_in_user(&kp->width, &up->width) ||
-	    assign_in_user(&kp->height, &up->height) ||
-	    assign_in_user(&kp->fraction_bits, &up->fraction_bits))
-		return -EFAULT;
-
-	while (n-- > 0) {
-		compat_uptr_t tmp;
-
-		if (get_user(tmp, &up->data[n]) ||
-		    put_user_force(compat_ptr(tmp), &kp->data[n]))
-			return -EFAULT;
-	}
-	return 0;
-}
-
-static int get_atomisp_acc_map32(struct atomisp_acc_map __user *kp,
-				 struct atomisp_acc_map32 __user *up)
-{
-	compat_uptr_t user_ptr;
-
-	if (!access_ok(up, sizeof(struct atomisp_acc_map32)) ||
-	    assign_in_user(&kp->flags, &up->flags) ||
-	    assign_in_user(&kp->length, &up->length) ||
-	    get_user(user_ptr, &up->user_ptr) ||
-	    put_user(compat_ptr(user_ptr), &kp->user_ptr) ||
-	    assign_in_user(&kp->css_ptr, &up->css_ptr) ||
-	    assign_in_user(&kp->reserved[0], &up->reserved[0]) ||
-	    assign_in_user(&kp->reserved[1], &up->reserved[1]) ||
-	    assign_in_user(&kp->reserved[2], &up->reserved[2]) ||
-	    assign_in_user(&kp->reserved[3], &up->reserved[3]))
-		return -EFAULT;
-
-	return 0;
-}
-
-static int put_atomisp_acc_map32(struct atomisp_acc_map __user *kp,
-				 struct atomisp_acc_map32 __user *up)
-{
-	void __user *user_ptr;
-
-	if (!access_ok(up, sizeof(struct atomisp_acc_map32)) ||
-	    assign_in_user(&up->flags, &kp->flags) ||
-	    assign_in_user(&up->length, &kp->length) ||
-	    get_user(user_ptr, &kp->user_ptr) ||
-	    put_user(ptr_to_compat(user_ptr), &up->user_ptr) ||
-	    assign_in_user(&up->css_ptr, &kp->css_ptr) ||
-	    assign_in_user(&up->reserved[0], &kp->reserved[0]) ||
-	    assign_in_user(&up->reserved[1], &kp->reserved[1]) ||
-	    assign_in_user(&up->reserved[2], &kp->reserved[2]) ||
-	    assign_in_user(&up->reserved[3], &kp->reserved[3]))
-		return -EFAULT;
-
-	return 0;
-}
-
-static int
-get_atomisp_acc_s_mapped_arg32(struct atomisp_acc_s_mapped_arg __user *kp,
-			       struct atomisp_acc_s_mapped_arg32 __user *up)
-{
-	if (!access_ok(up, sizeof(struct atomisp_acc_s_mapped_arg32)) ||
-	    assign_in_user(&kp->fw_handle, &up->fw_handle) ||
-	    assign_in_user(&kp->memory, &up->memory) ||
-	    assign_in_user(&kp->length, &up->length) ||
-	    assign_in_user(&kp->css_ptr, &up->css_ptr))
-		return -EFAULT;
-
-	return 0;
-}
-
-static int
-put_atomisp_acc_s_mapped_arg32(struct atomisp_acc_s_mapped_arg __user *kp,
-			       struct atomisp_acc_s_mapped_arg32 __user *up)
-{
-	if (!access_ok(up, sizeof(struct atomisp_acc_s_mapped_arg32)) ||
-	    assign_in_user(&up->fw_handle, &kp->fw_handle) ||
-	    assign_in_user(&up->memory, &kp->memory) ||
-	    assign_in_user(&up->length, &kp->length) ||
-	    assign_in_user(&up->css_ptr, &kp->css_ptr))
-		return -EFAULT;
-
-	return 0;
-}
-
-static int get_atomisp_parameters32(struct atomisp_parameters __user *kp,
-				    struct atomisp_parameters32 __user *up)
-{
-	int n = offsetof(struct atomisp_parameters32, output_frame) /
-		sizeof(compat_uptr_t);
-	compat_uptr_t stp, mtp, dcp, dscp;
-	struct {
-		struct atomisp_shading_table shading_table;
-		struct atomisp_morph_table morph_table;
-		struct atomisp_dis_coefficients dvs2_coefs;
-		struct atomisp_dvs_6axis_config dvs_6axis_config;
-	} __user *karg = (void __user *)(kp + 1);
-
-	if (!access_ok(up, sizeof(struct atomisp_parameters32)))
-		return -EFAULT;
-
-	while (n >= 0) {
-		compat_uptr_t __user *src = (compat_uptr_t __user *)up + n;
-		void * __user *dst = (void * __user *)kp + n;
-		compat_uptr_t tmp;
-
-		if (get_user_cast(tmp, src) || put_user_force(compat_ptr(tmp), dst))
-			return -EFAULT;
-		n--;
-	}
-
-	if (assign_in_user(&kp->isp_config_id, &up->isp_config_id) ||
-	    assign_in_user(&kp->per_frame_setting, &up->per_frame_setting) ||
-	    get_user(stp, &up->shading_table) ||
-	    get_user(mtp, &up->morph_table) ||
-	    get_user(dcp, &up->dvs2_coefs) ||
-	    get_user(dscp, &up->dvs_6axis_config))
-		return -EFAULT;
-
-	/* handle shading table */
-	if (stp && (get_atomisp_shading_table32(&karg->shading_table,
-						compat_ptr(stp)) ||
-		    put_user_force(&karg->shading_table, &kp->shading_table)))
-		return -EFAULT;
-
-	/* handle morph table */
-	if (mtp && (get_atomisp_morph_table32(&karg->morph_table,
-					      compat_ptr(mtp)) ||
-		    put_user_force(&karg->morph_table, &kp->morph_table)))
-		return -EFAULT;
-
-	/* handle dvs2 coefficients */
-	if (dcp && (get_atomisp_dis_coefficients32(&karg->dvs2_coefs,
-						   compat_ptr(dcp)) ||
-		    put_user_force(&karg->dvs2_coefs, &kp->dvs2_coefs)))
-		return -EFAULT;
-
-	/* handle dvs 6axis configuration */
-	if (dscp &&
-	    (get_atomisp_dvs_6axis_config32(&karg->dvs_6axis_config,
-					    compat_ptr(dscp)) ||
-	     put_user_force(&karg->dvs_6axis_config, &kp->dvs_6axis_config)))
-		return -EFAULT;
-
-	return 0;
-}
-
-static int
-get_atomisp_acc_fw_load_to_pipe32(struct atomisp_acc_fw_load_to_pipe __user *kp,
-				  struct atomisp_acc_fw_load_to_pipe32 __user *up)
-{
-	compat_uptr_t data;
-
-	if (!access_ok(up, sizeof(struct atomisp_acc_fw_load_to_pipe32)) ||
-	    assign_in_user(&kp->flags, &up->flags) ||
-	    assign_in_user(&kp->fw_handle, &up->fw_handle) ||
-	    assign_in_user(&kp->size, &up->size) ||
-	    assign_in_user(&kp->type, &up->type) ||
-	    assign_in_user(&kp->reserved[0], &up->reserved[0]) ||
-	    assign_in_user(&kp->reserved[1], &up->reserved[1]) ||
-	    assign_in_user(&kp->reserved[2], &up->reserved[2]) ||
-	    get_user(data, &up->data) ||
-	    put_user(compat_ptr(data), &kp->data))
-		return -EFAULT;
-
-	return 0;
-}
-
-static int
-put_atomisp_acc_fw_load_to_pipe32(struct atomisp_acc_fw_load_to_pipe __user *kp,
-				  struct atomisp_acc_fw_load_to_pipe32 __user *up)
-{
-	void __user *data;
-
-	if (!access_ok(up, sizeof(struct atomisp_acc_fw_load_to_pipe32)) ||
-	    assign_in_user(&up->flags, &kp->flags) ||
-	    assign_in_user(&up->fw_handle, &kp->fw_handle) ||
-	    assign_in_user(&up->size, &kp->size) ||
-	    assign_in_user(&up->type, &kp->type) ||
-	    assign_in_user(&up->reserved[0], &kp->reserved[0]) ||
-	    assign_in_user(&up->reserved[1], &kp->reserved[1]) ||
-	    assign_in_user(&up->reserved[2], &kp->reserved[2]) ||
-	    get_user(data, &kp->data) ||
-	    put_user(ptr_to_compat(data), &up->data))
-		return -EFAULT;
-
-	return 0;
-}
-
-static int
-get_atomisp_sensor_ae_bracketing_lut(struct atomisp_sensor_ae_bracketing_lut __user *kp,
-				     struct atomisp_sensor_ae_bracketing_lut32 __user *up)
-{
-	compat_uptr_t lut;
-
-	if (!access_ok(up, sizeof(struct atomisp_sensor_ae_bracketing_lut32)) ||
-	    assign_in_user(&kp->lut_size, &up->lut_size) ||
-	    get_user(lut, &up->lut) ||
-	    put_user_force(compat_ptr(lut), &kp->lut))
-		return -EFAULT;
-
-	return 0;
-}
-
-static long native_ioctl(struct file *file, unsigned int cmd, unsigned long arg)
-{
-	long ret = -ENOIOCTLCMD;
-
-	if (file->f_op->unlocked_ioctl)
-		ret = file->f_op->unlocked_ioctl(file, cmd, arg);
-
-	return ret;
-}
-
-static long atomisp_do_compat_ioctl(struct file *file,
-				    unsigned int cmd, unsigned long arg)
-{
-	union {
-		struct atomisp_histogram his;
-		struct atomisp_dis_statistics dis_s;
-		struct atomisp_dis_coefficients dis_c;
-		struct atomisp_dvs_6axis_config dvs_c;
-		struct atomisp_3a_statistics s3a_s;
-		struct atomisp_morph_table mor_t;
-		struct v4l2_framebuffer v4l2_buf;
-		struct atomisp_overlay overlay;
-		struct atomisp_calibration_group cal_grp;
-		struct atomisp_acc_fw_load acc_fw_load;
-		struct atomisp_acc_fw_arg acc_fw_arg;
-		struct v4l2_private_int_data v4l2_pri_data;
-		struct atomisp_shading_table shd_tbl;
-		struct atomisp_acc_map acc_map;
-		struct atomisp_acc_s_mapped_arg acc_map_arg;
-		struct atomisp_parameters param;
-		struct atomisp_acc_fw_load_to_pipe acc_fw_to_pipe;
-		struct atomisp_metadata md;
-		struct atomisp_metadata_with_type md_with_type;
-		struct atomisp_sensor_ae_bracketing_lut lut;
-	} __user *karg;
-	void __user *up = compat_ptr(arg);
-	long err = -ENOIOCTLCMD;
-
-	karg = compat_alloc_user_space(
-		sizeof(*karg) + (cmd == ATOMISP_IOC_S_PARAMETERS32 ?
-				 sizeof(struct atomisp_shading_table) +
-				 sizeof(struct atomisp_morph_table) +
-				 sizeof(struct atomisp_dis_coefficients) +
-				 sizeof(struct atomisp_dvs_6axis_config) : 0));
-	if (!karg)
-		return -ENOMEM;
-
-	/* First, convert the command. */
-	switch (cmd) {
-	case ATOMISP_IOC_G_HISTOGRAM32:
-		cmd = ATOMISP_IOC_G_HISTOGRAM;
-		break;
-	case ATOMISP_IOC_S_HISTOGRAM32:
-		cmd = ATOMISP_IOC_S_HISTOGRAM;
-		break;
-	case ATOMISP_IOC_G_DIS_STAT32:
-		cmd = ATOMISP_IOC_G_DIS_STAT;
-		break;
-	case ATOMISP_IOC_S_DIS_COEFS32:
-		cmd = ATOMISP_IOC_S_DIS_COEFS;
-		break;
-	case ATOMISP_IOC_S_DIS_VECTOR32:
-		cmd = ATOMISP_IOC_S_DIS_VECTOR;
-		break;
-	case ATOMISP_IOC_G_3A_STAT32:
-		cmd = ATOMISP_IOC_G_3A_STAT;
-		break;
-	case ATOMISP_IOC_G_ISP_GDC_TAB32:
-		cmd = ATOMISP_IOC_G_ISP_GDC_TAB;
-		break;
-	case ATOMISP_IOC_S_ISP_GDC_TAB32:
-		cmd = ATOMISP_IOC_S_ISP_GDC_TAB;
-		break;
-	case ATOMISP_IOC_S_ISP_FPN_TABLE32:
-		cmd = ATOMISP_IOC_S_ISP_FPN_TABLE;
-		break;
-	case ATOMISP_IOC_G_ISP_OVERLAY32:
-		cmd = ATOMISP_IOC_G_ISP_OVERLAY;
-		break;
-	case ATOMISP_IOC_S_ISP_OVERLAY32:
-		cmd = ATOMISP_IOC_S_ISP_OVERLAY;
-		break;
-	case ATOMISP_IOC_G_SENSOR_CALIBRATION_GROUP32:
-		cmd = ATOMISP_IOC_G_SENSOR_CALIBRATION_GROUP;
-		break;
-	case ATOMISP_IOC_ACC_LOAD32:
-		cmd = ATOMISP_IOC_ACC_LOAD;
-		break;
-	case ATOMISP_IOC_ACC_S_ARG32:
-		cmd = ATOMISP_IOC_ACC_S_ARG;
-		break;
-	case ATOMISP_IOC_G_SENSOR_PRIV_INT_DATA32:
-		cmd = ATOMISP_IOC_G_SENSOR_PRIV_INT_DATA;
-		break;
-	case ATOMISP_IOC_S_ISP_SHD_TAB32:
-		cmd = ATOMISP_IOC_S_ISP_SHD_TAB;
-		break;
-	case ATOMISP_IOC_ACC_DESTAB32:
-		cmd = ATOMISP_IOC_ACC_DESTAB;
-		break;
-	case ATOMISP_IOC_G_MOTOR_PRIV_INT_DATA32:
-		cmd = ATOMISP_IOC_G_MOTOR_PRIV_INT_DATA;
-		break;
-	case ATOMISP_IOC_ACC_MAP32:
-		cmd = ATOMISP_IOC_ACC_MAP;
-		break;
-	case ATOMISP_IOC_ACC_UNMAP32:
-		cmd = ATOMISP_IOC_ACC_UNMAP;
-		break;
-	case ATOMISP_IOC_ACC_S_MAPPED_ARG32:
-		cmd = ATOMISP_IOC_ACC_S_MAPPED_ARG;
-		break;
-	case ATOMISP_IOC_S_PARAMETERS32:
-		cmd = ATOMISP_IOC_S_PARAMETERS;
-		break;
-	case ATOMISP_IOC_ACC_LOAD_TO_PIPE32:
-		cmd = ATOMISP_IOC_ACC_LOAD_TO_PIPE;
-		break;
-	case ATOMISP_IOC_G_METADATA32:
-		cmd = ATOMISP_IOC_G_METADATA;
-		break;
-	case ATOMISP_IOC_G_METADATA_BY_TYPE32:
-		cmd = ATOMISP_IOC_G_METADATA_BY_TYPE;
-		break;
-	case ATOMISP_IOC_S_SENSOR_AE_BRACKETING_LUT32:
-		cmd = ATOMISP_IOC_S_SENSOR_AE_BRACKETING_LUT;
-		break;
-	}
-
-	switch (cmd) {
-	case ATOMISP_IOC_G_HISTOGRAM:
-	case ATOMISP_IOC_S_HISTOGRAM:
-		err = get_atomisp_histogram32(&karg->his, up);
-		break;
-	case ATOMISP_IOC_G_DIS_STAT:
-		err = get_atomisp_dis_statistics32(&karg->dis_s, up);
-		break;
-	case ATOMISP_IOC_S_DIS_COEFS:
-		err = get_atomisp_dis_coefficients32(&karg->dis_c, up);
-		break;
-	case ATOMISP_IOC_S_DIS_VECTOR:
-		err = get_atomisp_dvs_6axis_config32(&karg->dvs_c, up);
-		break;
-	case ATOMISP_IOC_G_3A_STAT:
-		err = get_atomisp_3a_statistics32(&karg->s3a_s, up);
-		break;
-	case ATOMISP_IOC_G_ISP_GDC_TAB:
-	case ATOMISP_IOC_S_ISP_GDC_TAB:
-		err = get_atomisp_morph_table32(&karg->mor_t, up);
-		break;
-	case ATOMISP_IOC_S_ISP_FPN_TABLE:
-		err = get_v4l2_framebuffer32(&karg->v4l2_buf, up);
-		break;
-	case ATOMISP_IOC_G_ISP_OVERLAY:
-	case ATOMISP_IOC_S_ISP_OVERLAY:
-		err = get_atomisp_overlay32(&karg->overlay, up);
-		break;
-	case ATOMISP_IOC_G_SENSOR_CALIBRATION_GROUP:
-		err = get_atomisp_calibration_group32(&karg->cal_grp, up);
-		break;
-	case ATOMISP_IOC_ACC_LOAD:
-		err = get_atomisp_acc_fw_load32(&karg->acc_fw_load, up);
-		break;
-	case ATOMISP_IOC_ACC_S_ARG:
-	case ATOMISP_IOC_ACC_DESTAB:
-		err = get_atomisp_acc_fw_arg32(&karg->acc_fw_arg, up);
-		break;
-	case ATOMISP_IOC_G_SENSOR_PRIV_INT_DATA:
-	case ATOMISP_IOC_G_MOTOR_PRIV_INT_DATA:
-		err = get_v4l2_private_int_data32(&karg->v4l2_pri_data, up);
-		break;
-	case ATOMISP_IOC_S_ISP_SHD_TAB:
-		err = get_atomisp_shading_table32(&karg->shd_tbl, up);
-		break;
-	case ATOMISP_IOC_ACC_MAP:
-	case ATOMISP_IOC_ACC_UNMAP:
-		err = get_atomisp_acc_map32(&karg->acc_map, up);
-		break;
-	case ATOMISP_IOC_ACC_S_MAPPED_ARG:
-		err = get_atomisp_acc_s_mapped_arg32(&karg->acc_map_arg, up);
-		break;
-	case ATOMISP_IOC_S_PARAMETERS:
-		err = get_atomisp_parameters32(&karg->param, up);
-		break;
-	case ATOMISP_IOC_ACC_LOAD_TO_PIPE:
-		err = get_atomisp_acc_fw_load_to_pipe32(&karg->acc_fw_to_pipe,
-							up);
-		break;
-	case ATOMISP_IOC_G_METADATA:
-		err = get_atomisp_metadata_stat32(&karg->md, up);
-		break;
-	case ATOMISP_IOC_G_METADATA_BY_TYPE:
-		err = get_atomisp_metadata_by_type_stat32(&karg->md_with_type,
-							  up);
-		break;
-	case ATOMISP_IOC_S_SENSOR_AE_BRACKETING_LUT:
-		err = get_atomisp_sensor_ae_bracketing_lut(&karg->lut, up);
-		break;
-	}
-	if (err)
-		return err;
-
-	err = native_ioctl(file, cmd, (unsigned long)karg);
-	if (err)
-		return err;
-
-	switch (cmd) {
-	case ATOMISP_IOC_G_HISTOGRAM:
-		err = put_atomisp_histogram32(&karg->his, up);
-		break;
-	case ATOMISP_IOC_G_DIS_STAT:
-		err = put_atomisp_dis_statistics32(&karg->dis_s, up);
-		break;
-	case ATOMISP_IOC_G_3A_STAT:
-		err = put_atomisp_3a_statistics32(&karg->s3a_s, up);
-		break;
-	case ATOMISP_IOC_G_ISP_GDC_TAB:
-		err = put_atomisp_morph_table32(&karg->mor_t, up);
-		break;
-	case ATOMISP_IOC_G_ISP_OVERLAY:
-		err = put_atomisp_overlay32(&karg->overlay, up);
-		break;
-	case ATOMISP_IOC_G_SENSOR_CALIBRATION_GROUP:
-		err = put_atomisp_calibration_group32(&karg->cal_grp, up);
-		break;
-	case ATOMISP_IOC_ACC_LOAD:
-		err = put_atomisp_acc_fw_load32(&karg->acc_fw_load, up);
-		break;
-	case ATOMISP_IOC_ACC_S_ARG:
-	case ATOMISP_IOC_ACC_DESTAB:
-		err = put_atomisp_acc_fw_arg32(&karg->acc_fw_arg, up);
-		break;
-	case ATOMISP_IOC_G_SENSOR_PRIV_INT_DATA:
-	case ATOMISP_IOC_G_MOTOR_PRIV_INT_DATA:
-		err = put_v4l2_private_int_data32(&karg->v4l2_pri_data, up);
-		break;
-	case ATOMISP_IOC_ACC_MAP:
-	case ATOMISP_IOC_ACC_UNMAP:
-		err = put_atomisp_acc_map32(&karg->acc_map, up);
-		break;
-	case ATOMISP_IOC_ACC_S_MAPPED_ARG:
-		err = put_atomisp_acc_s_mapped_arg32(&karg->acc_map_arg, up);
-		break;
-	case ATOMISP_IOC_ACC_LOAD_TO_PIPE:
-		err = put_atomisp_acc_fw_load_to_pipe32(&karg->acc_fw_to_pipe,
-							up);
-		break;
-	case ATOMISP_IOC_G_METADATA:
-		err = put_atomisp_metadata_stat32(&karg->md, up);
-		break;
-	case ATOMISP_IOC_G_METADATA_BY_TYPE:
-		err = put_atomisp_metadata_by_type_stat32(&karg->md_with_type,
-							  up);
-		break;
-	}
-
-	return err;
-}
-
-long atomisp_compat_ioctl32(struct file *file,
-			    unsigned int cmd, unsigned long arg)
-{
-	struct video_device *vdev = video_devdata(file);
-	struct atomisp_device *isp = video_get_drvdata(vdev);
-	long ret = -ENOIOCTLCMD;
-
-	if (!file->f_op->unlocked_ioctl)
-		return ret;
-
-	switch (cmd) {
-	case ATOMISP_IOC_G_XNR:
-	case ATOMISP_IOC_S_XNR:
-	case ATOMISP_IOC_G_NR:
-	case ATOMISP_IOC_S_NR:
-	case ATOMISP_IOC_G_TNR:
-	case ATOMISP_IOC_S_TNR:
-	case ATOMISP_IOC_G_BLACK_LEVEL_COMP:
-	case ATOMISP_IOC_S_BLACK_LEVEL_COMP:
-	case ATOMISP_IOC_G_EE:
-	case ATOMISP_IOC_S_EE:
-	case ATOMISP_IOC_S_DIS_VECTOR:
-	case ATOMISP_IOC_G_ISP_PARM:
-	case ATOMISP_IOC_S_ISP_PARM:
-	case ATOMISP_IOC_G_ISP_GAMMA:
-	case ATOMISP_IOC_S_ISP_GAMMA:
-	case ATOMISP_IOC_ISP_MAKERNOTE:
-	case ATOMISP_IOC_G_ISP_MACC:
-	case ATOMISP_IOC_S_ISP_MACC:
-	case ATOMISP_IOC_G_ISP_BAD_PIXEL_DETECTION:
-	case ATOMISP_IOC_S_ISP_BAD_PIXEL_DETECTION:
-	case ATOMISP_IOC_G_ISP_FALSE_COLOR_CORRECTION:
-	case ATOMISP_IOC_S_ISP_FALSE_COLOR_CORRECTION:
-	case ATOMISP_IOC_G_ISP_CTC:
-	case ATOMISP_IOC_S_ISP_CTC:
-	case ATOMISP_IOC_G_ISP_WHITE_BALANCE:
-	case ATOMISP_IOC_S_ISP_WHITE_BALANCE:
-	case ATOMISP_IOC_CAMERA_BRIDGE:
-	case ATOMISP_IOC_G_SENSOR_MODE_DATA:
-	case ATOMISP_IOC_S_EXPOSURE:
-	case ATOMISP_IOC_G_3A_CONFIG:
-	case ATOMISP_IOC_S_3A_CONFIG:
-	case ATOMISP_IOC_ACC_UNLOAD:
-	case ATOMISP_IOC_ACC_START:
-	case ATOMISP_IOC_ACC_WAIT:
-	case ATOMISP_IOC_ACC_ABORT:
-	case ATOMISP_IOC_G_ISP_GAMMA_CORRECTION:
-	case ATOMISP_IOC_S_ISP_GAMMA_CORRECTION:
-	case ATOMISP_IOC_S_CONT_CAPTURE_CONFIG:
-	case ATOMISP_IOC_G_DVS2_BQ_RESOLUTIONS:
-	case ATOMISP_IOC_EXT_ISP_CTRL:
-	case ATOMISP_IOC_EXP_ID_UNLOCK:
-	case ATOMISP_IOC_EXP_ID_CAPTURE:
-	case ATOMISP_IOC_S_ENABLE_DZ_CAPT_PIPE:
-	case ATOMISP_IOC_G_FORMATS_CONFIG:
-	case ATOMISP_IOC_S_FORMATS_CONFIG:
-	case ATOMISP_IOC_S_EXPOSURE_WINDOW:
-	case ATOMISP_IOC_S_ACC_STATE:
-	case ATOMISP_IOC_G_ACC_STATE:
-	case ATOMISP_IOC_INJECT_A_FAKE_EVENT:
-	case ATOMISP_IOC_G_SENSOR_AE_BRACKETING_INFO:
-	case ATOMISP_IOC_S_SENSOR_AE_BRACKETING_MODE:
-	case ATOMISP_IOC_G_SENSOR_AE_BRACKETING_MODE:
-	case ATOMISP_IOC_G_INVALID_FRAME_NUM:
-	case ATOMISP_IOC_S_ARRAY_RESOLUTION:
-	case ATOMISP_IOC_S_SENSOR_RUNMODE:
-	case ATOMISP_IOC_G_UPDATE_EXPOSURE:
-		ret = native_ioctl(file, cmd, arg);
-		break;
-
-	case ATOMISP_IOC_G_HISTOGRAM32:
-	case ATOMISP_IOC_S_HISTOGRAM32:
-	case ATOMISP_IOC_G_DIS_STAT32:
-	case ATOMISP_IOC_S_DIS_COEFS32:
-	case ATOMISP_IOC_S_DIS_VECTOR32:
-	case ATOMISP_IOC_G_3A_STAT32:
-	case ATOMISP_IOC_G_ISP_GDC_TAB32:
-	case ATOMISP_IOC_S_ISP_GDC_TAB32:
-	case ATOMISP_IOC_S_ISP_FPN_TABLE32:
-	case ATOMISP_IOC_G_ISP_OVERLAY32:
-	case ATOMISP_IOC_S_ISP_OVERLAY32:
-	case ATOMISP_IOC_G_SENSOR_CALIBRATION_GROUP32:
-	case ATOMISP_IOC_ACC_LOAD32:
-	case ATOMISP_IOC_ACC_S_ARG32:
-	case ATOMISP_IOC_G_SENSOR_PRIV_INT_DATA32:
-	case ATOMISP_IOC_S_ISP_SHD_TAB32:
-	case ATOMISP_IOC_ACC_DESTAB32:
-	case ATOMISP_IOC_G_MOTOR_PRIV_INT_DATA32:
-	case ATOMISP_IOC_ACC_MAP32:
-	case ATOMISP_IOC_ACC_UNMAP32:
-	case ATOMISP_IOC_ACC_S_MAPPED_ARG32:
-	case ATOMISP_IOC_S_PARAMETERS32:
-	case ATOMISP_IOC_ACC_LOAD_TO_PIPE32:
-	case ATOMISP_IOC_G_METADATA32:
-	case ATOMISP_IOC_G_METADATA_BY_TYPE32:
-	case ATOMISP_IOC_S_SENSOR_AE_BRACKETING_LUT32:
-		ret = atomisp_do_compat_ioctl(file, cmd, arg);
-		break;
-
-	default:
-		dev_warn(isp->dev,
-			 "%s: unknown ioctl '%c', dir=%d, #%d (0x%08x)\n",
-			 __func__, _IOC_TYPE(cmd), _IOC_DIR(cmd), _IOC_NR(cmd),
-			 cmd);
-		break;
-	}
-	return ret;
-}
-#endif /* CONFIG_COMPAT */
diff --git a/drivers/staging/media/atomisp/pci/atomisp_fops.c b/drivers/staging/media/atomisp/pci/atomisp_fops.c
index 26d05474a035..be58f21ab208 100644
--- a/drivers/staging/media/atomisp/pci/atomisp_fops.c
+++ b/drivers/staging/media/atomisp/pci/atomisp_fops.c
@@ -1283,7 +1283,8 @@ const struct v4l2_file_operations atomisp_fops = {
 	.unlocked_ioctl = video_ioctl2,
 #ifdef CONFIG_COMPAT
 	/*
-	 * There are problems with this code. Disable this for now.
+	 * this was removed because of bugs, the interface
+	 * needs to be made safe for compat tasks instead.
 	.compat_ioctl32 = atomisp_compat_ioctl32,
 	 */
 #endif
@@ -1297,10 +1298,7 @@ const struct v4l2_file_operations atomisp_file_fops = {
 	.mmap = atomisp_file_mmap,
 	.unlocked_ioctl = video_ioctl2,
 #ifdef CONFIG_COMPAT
-	/*
-	 * There are problems with this code. Disable this for now.
-	.compat_ioctl32 = atomisp_compat_ioctl32,
-	 */
+	/* .compat_ioctl32 = atomisp_compat_ioctl32, */
 #endif
 	.poll = atomisp_poll,
 };
-- 
2.29.2

Re: [PATCH v3 6/8] media: atomisp: remove compat_ioctl32 code

[Laurent Pinchart]

Hi Arnd,

Thank you for the patch.

On Mon, Jun 14, 2021 at 12:34:07PM +0200, Arnd Bergmann wrote:
> From: Arnd Bergmann <arnd@arndb.de>
> 
> This is one of the last remaining users of compat_alloc_user_space()
> and copy_in_user(), which are in the process of getting removed.
> 
> As of commit 57e6b6f2303e ("media: atomisp_fops.c: disable
> atomisp_compat_ioctl32"), nothing in this file is actually getting used
> as the only reference has been stubbed out.
> 
> Remove the entire file -- anyone willing to restore the functionality
> can equally well just look up the contents in the git history if needed.
> 
> Cc: Sakari Ailus <sakari.ailus@linux.intel.com>
> Cc: Hans Verkuil <hverkuil@xs4all.nl>
> Acked-by: Sakari Ailus <sakari.ailus@linux.intel.com>
> Suggested-by: Christoph Hellwig <hch@lst.de>
> Reviewed-by: Andy Shevchenko <andriy.shevchenko@linux.intel.com>
> Signed-off-by: Arnd Bergmann <arnd@arndb.de>

Acked-by: Laurent Pinchart <laurent.pinchart@ideasonboard.com>

> ---
>  drivers/staging/media/atomisp/Makefile        |    1 -
>  drivers/staging/media/atomisp/TODO            |    5 +
>  .../atomisp/pci/atomisp_compat_ioctl32.c      | 1202 -----------------
>  .../staging/media/atomisp/pci/atomisp_fops.c  |    8 +-
>  4 files changed, 8 insertions(+), 1208 deletions(-)
>  delete mode 100644 drivers/staging/media/atomisp/pci/atomisp_compat_ioctl32.c
> 
> diff --git a/drivers/staging/media/atomisp/Makefile b/drivers/staging/media/atomisp/Makefile
> index 51498b2e85b8..606b7754fdfd 100644
> --- a/drivers/staging/media/atomisp/Makefile
> +++ b/drivers/staging/media/atomisp/Makefile
> @@ -16,7 +16,6 @@ atomisp-objs += \
>  	pci/atomisp_acc.o \
>  	pci/atomisp_cmd.o \
>  	pci/atomisp_compat_css20.o \
> -	pci/atomisp_compat_ioctl32.o \
>  	pci/atomisp_csi2.o \
>  	pci/atomisp_drvfs.o \
>  	pci/atomisp_file.o \
> diff --git a/drivers/staging/media/atomisp/TODO b/drivers/staging/media/atomisp/TODO
> index 6987bb2d32cf..2d1ef9eb262a 100644
> --- a/drivers/staging/media/atomisp/TODO
> +++ b/drivers/staging/media/atomisp/TODO
> @@ -120,6 +120,11 @@ TODO
>      for this driver until the other work is done, as there will be a lot
>      of code churn until this driver becomes functional again.
>  
> +16. Fix private ioctls to not need a compat_ioctl handler for running
> +    32-bit tasks. The compat code has been removed because of bugs,
> +    and should not be needed for modern drivers. Fixing this properly
> +    unfortunately means an incompatible ABI change.
> +
>  Limitations
>  ===========
>  

[snip]

> diff --git a/drivers/staging/media/atomisp/pci/atomisp_fops.c b/drivers/staging/media/atomisp/pci/atomisp_fops.c
> index 26d05474a035..be58f21ab208 100644
> --- a/drivers/staging/media/atomisp/pci/atomisp_fops.c
> +++ b/drivers/staging/media/atomisp/pci/atomisp_fops.c
> @@ -1283,7 +1283,8 @@ const struct v4l2_file_operations atomisp_fops = {
>  	.unlocked_ioctl = video_ioctl2,
>  #ifdef CONFIG_COMPAT
>  	/*
> -	 * There are problems with this code. Disable this for now.
> +	 * this was removed because of bugs, the interface
> +	 * needs to be made safe for compat tasks instead.
>  	.compat_ioctl32 = atomisp_compat_ioctl32,
>  	 */
>  #endif
> @@ -1297,10 +1298,7 @@ const struct v4l2_file_operations atomisp_file_fops = {
>  	.mmap = atomisp_file_mmap,
>  	.unlocked_ioctl = video_ioctl2,
>  #ifdef CONFIG_COMPAT
> -	/*
> -	 * There are problems with this code. Disable this for now.
> -	.compat_ioctl32 = atomisp_compat_ioctl32,
> -	 */
> +	/* .compat_ioctl32 = atomisp_compat_ioctl32, */
>  #endif
>  	.poll = atomisp_poll,
>  };

-- 
Regards,

Laurent Pinchart

[PATCH v3 7/8] media: subdev: fix compat_ioctl32

[Arnd Bergmann]

From: Arnd Bergmann <arnd@arndb.de>

The adv7842 and si4713 drivers each define one private ioctl command that
are handled through the subdev_ioctl() helpers, but that don't work in
compat mode because this does not handle private ioctl commands.

The compat_ioctl32 callback for subdevs has outdated calling conventions,
but as there are no users of that, it is easy to change the function
pointer type and the caller to make it behave the same way as the normal
ioctl callback and hook in the two drivers that need no argument
conversion.

Signed-off-by: Arnd Bergmann <arnd@arndb.de>
---
 drivers/media/i2c/adv7842.c           |  3 +++
 drivers/media/radio/si4713/si4713.c   |  3 +++
 drivers/media/v4l2-core/v4l2-subdev.c | 19 ++++++++++++++++---
 include/media/v4l2-subdev.h           |  3 +--
 4 files changed, 23 insertions(+), 5 deletions(-)

diff --git a/drivers/media/i2c/adv7842.c b/drivers/media/i2c/adv7842.c
index 78e61fe6f2f0..cd6df4f52f33 100644
--- a/drivers/media/i2c/adv7842.c
+++ b/drivers/media/i2c/adv7842.c
@@ -3293,6 +3293,9 @@ static const struct v4l2_ctrl_ops adv7842_ctrl_ops = {
 static const struct v4l2_subdev_core_ops adv7842_core_ops = {
 	.log_status = adv7842_log_status,
 	.ioctl = adv7842_ioctl,
+#ifdef CONFIG_COMPAT
+	.compat_ioctl32 = adv7842_ioctl,
+#endif
 	.interrupt_service_routine = adv7842_isr,
 	.subscribe_event = adv7842_subscribe_event,
 	.unsubscribe_event = v4l2_event_subdev_unsubscribe,
diff --git a/drivers/media/radio/si4713/si4713.c b/drivers/media/radio/si4713/si4713.c
index adbf43ff6a21..ae7e477774e3 100644
--- a/drivers/media/radio/si4713/si4713.c
+++ b/drivers/media/radio/si4713/si4713.c
@@ -1398,6 +1398,9 @@ static const struct v4l2_ctrl_ops si4713_ctrl_ops = {
 
 static const struct v4l2_subdev_core_ops si4713_subdev_core_ops = {
 	.ioctl		= si4713_ioctl,
+#ifdef CONFIG_COMPAT
+	.compat_ioctl32	= si4713_ioctl,
+#endif
 };
 
 static const struct v4l2_subdev_tuner_ops si4713_subdev_tuner_ops = {
diff --git a/drivers/media/v4l2-core/v4l2-subdev.c b/drivers/media/v4l2-core/v4l2-subdev.c
index bf3aa9252458..fbd176d6c415 100644
--- a/drivers/media/v4l2-core/v4l2-subdev.c
+++ b/drivers/media/v4l2-core/v4l2-subdev.c
@@ -686,13 +686,26 @@ static long subdev_ioctl(struct file *file, unsigned int cmd,
 }
 
 #ifdef CONFIG_COMPAT
-static long subdev_compat_ioctl32(struct file *file, unsigned int cmd,
-	unsigned long arg)
+static long subdev_do_compat_ioctl32(struct file *file, unsigned int cmd, void *arg)
 {
 	struct video_device *vdev = video_devdata(file);
 	struct v4l2_subdev *sd = vdev_to_v4l2_subdev(vdev);
+	struct mutex *lock = vdev->lock;
+	long ret = -ENODEV;
 
-	return v4l2_subdev_call(sd, core, compat_ioctl32, cmd, arg);
+	if (lock && mutex_lock_interruptible(lock))
+		return -ERESTARTSYS;
+	if (video_is_registered(vdev))
+		ret = v4l2_subdev_call(sd, core, compat_ioctl32, cmd, arg);
+	if (lock)
+		mutex_unlock(lock);
+	return ret;
+}
+
+static long subdev_compat_ioctl32(struct file *file, unsigned int cmd,
+	unsigned long arg)
+{
+	return video_usercopy(file, cmd, arg, subdev_do_compat_ioctl32);
 }
 #endif
 
diff --git a/include/media/v4l2-subdev.h b/include/media/v4l2-subdev.h
index d0e9a5bdb08b..42aa1f6c7c3f 100644
--- a/include/media/v4l2-subdev.h
+++ b/include/media/v4l2-subdev.h
@@ -195,8 +195,7 @@ struct v4l2_subdev_core_ops {
 	int (*s_gpio)(struct v4l2_subdev *sd, u32 val);
 	long (*ioctl)(struct v4l2_subdev *sd, unsigned int cmd, void *arg);
 #ifdef CONFIG_COMPAT
-	long (*compat_ioctl32)(struct v4l2_subdev *sd, unsigned int cmd,
-			       unsigned long arg);
+	long (*compat_ioctl32)(struct v4l2_subdev *sd, unsigned int cmd, void *arg);
 #endif
 #ifdef CONFIG_VIDEO_ADV_DEBUG
 	int (*g_register)(struct v4l2_subdev *sd, struct v4l2_dbg_register *reg);
-- 
2.29.2

Re: [PATCH v3 7/8] media: subdev: fix compat_ioctl32

[Laurent Pinchart]

Hi Arnd,

Thank you for the patch.

On Mon, Jun 14, 2021 at 12:34:08PM +0200, Arnd Bergmann wrote:
> From: Arnd Bergmann <arnd@arndb.de>
> 
> The adv7842 and si4713 drivers each define one private ioctl command that
> are handled through the subdev_ioctl() helpers, but that don't work in

s/don't/doesn't/

> compat mode because this does not handle private ioctl commands.
> 
> The compat_ioctl32 callback for subdevs has outdated calling conventions,
> but as there are no users of that, it is easy to change the function
> pointer type and the caller to make it behave the same way as the normal
> ioctl callback and hook in the two drivers that need no argument
> conversion.
> 
> Signed-off-by: Arnd Bergmann <arnd@arndb.de>
> ---
>  drivers/media/i2c/adv7842.c           |  3 +++
>  drivers/media/radio/si4713/si4713.c   |  3 +++
>  drivers/media/v4l2-core/v4l2-subdev.c | 19 ++++++++++++++++---
>  include/media/v4l2-subdev.h           |  3 +--
>  4 files changed, 23 insertions(+), 5 deletions(-)
> 
> diff --git a/drivers/media/i2c/adv7842.c b/drivers/media/i2c/adv7842.c
> index 78e61fe6f2f0..cd6df4f52f33 100644
> --- a/drivers/media/i2c/adv7842.c
> +++ b/drivers/media/i2c/adv7842.c
> @@ -3293,6 +3293,9 @@ static const struct v4l2_ctrl_ops adv7842_ctrl_ops = {
>  static const struct v4l2_subdev_core_ops adv7842_core_ops = {
>  	.log_status = adv7842_log_status,
>  	.ioctl = adv7842_ioctl,
> +#ifdef CONFIG_COMPAT
> +	.compat_ioctl32 = adv7842_ioctl,
> +#endif
>  	.interrupt_service_routine = adv7842_isr,
>  	.subscribe_event = adv7842_subscribe_event,
>  	.unsubscribe_event = v4l2_event_subdev_unsubscribe,
> diff --git a/drivers/media/radio/si4713/si4713.c b/drivers/media/radio/si4713/si4713.c
> index adbf43ff6a21..ae7e477774e3 100644
> --- a/drivers/media/radio/si4713/si4713.c
> +++ b/drivers/media/radio/si4713/si4713.c
> @@ -1398,6 +1398,9 @@ static const struct v4l2_ctrl_ops si4713_ctrl_ops = {
>  
>  static const struct v4l2_subdev_core_ops si4713_subdev_core_ops = {
>  	.ioctl		= si4713_ioctl,
> +#ifdef CONFIG_COMPAT
> +	.compat_ioctl32	= si4713_ioctl,
> +#endif

Should we drop v4l2_subdev_core_ops.compat_ioctl32 and call
v4l2_subdev_core_ops.ioctl from subdev_do_compat_ioctl32() ? New drivers
should design custom ioctls in a way that doesn't require compat code.

>  };
>  
>  static const struct v4l2_subdev_tuner_ops si4713_subdev_tuner_ops = {
> diff --git a/drivers/media/v4l2-core/v4l2-subdev.c b/drivers/media/v4l2-core/v4l2-subdev.c
> index bf3aa9252458..fbd176d6c415 100644
> --- a/drivers/media/v4l2-core/v4l2-subdev.c
> +++ b/drivers/media/v4l2-core/v4l2-subdev.c
> @@ -686,13 +686,26 @@ static long subdev_ioctl(struct file *file, unsigned int cmd,
>  }
>  
>  #ifdef CONFIG_COMPAT
> -static long subdev_compat_ioctl32(struct file *file, unsigned int cmd,
> -	unsigned long arg)
> +static long subdev_do_compat_ioctl32(struct file *file, unsigned int cmd, void *arg)
>  {
>  	struct video_device *vdev = video_devdata(file);
>  	struct v4l2_subdev *sd = vdev_to_v4l2_subdev(vdev);
> +	struct mutex *lock = vdev->lock;
> +	long ret = -ENODEV;
>  
> -	return v4l2_subdev_call(sd, core, compat_ioctl32, cmd, arg);
> +	if (lock && mutex_lock_interruptible(lock))
> +		return -ERESTARTSYS;
> +	if (video_is_registered(vdev))
> +		ret = v4l2_subdev_call(sd, core, compat_ioctl32, cmd, arg);
> +	if (lock)
> +		mutex_unlock(lock);
> +	return ret;
> +}
> +
> +static long subdev_compat_ioctl32(struct file *file, unsigned int cmd,
> +	unsigned long arg)
> +{
> +	return video_usercopy(file, cmd, arg, subdev_do_compat_ioctl32);
>  }
>  #endif
>  
> diff --git a/include/media/v4l2-subdev.h b/include/media/v4l2-subdev.h
> index d0e9a5bdb08b..42aa1f6c7c3f 100644
> --- a/include/media/v4l2-subdev.h
> +++ b/include/media/v4l2-subdev.h
> @@ -195,8 +195,7 @@ struct v4l2_subdev_core_ops {
>  	int (*s_gpio)(struct v4l2_subdev *sd, u32 val);
>  	long (*ioctl)(struct v4l2_subdev *sd, unsigned int cmd, void *arg);
>  #ifdef CONFIG_COMPAT
> -	long (*compat_ioctl32)(struct v4l2_subdev *sd, unsigned int cmd,
> -			       unsigned long arg);
> +	long (*compat_ioctl32)(struct v4l2_subdev *sd, unsigned int cmd, void *arg);
>  #endif
>  #ifdef CONFIG_VIDEO_ADV_DEBUG
>  	int (*g_register)(struct v4l2_subdev *sd, struct v4l2_dbg_register *reg);

-- 
Regards,

Laurent Pinchart

Re: [PATCH v3 7/8] media: subdev: fix compat_ioctl32

[Hans Verkuil]

On 14/06/2021 19:18, Laurent Pinchart wrote:
> Hi Arnd,
> 
> Thank you for the patch.
> 
> On Mon, Jun 14, 2021 at 12:34:08PM +0200, Arnd Bergmann wrote:
>> From: Arnd Bergmann <arnd@arndb.de>
>>
>> The adv7842 and si4713 drivers each define one private ioctl command that
>> are handled through the subdev_ioctl() helpers, but that don't work in
> 
> s/don't/doesn't/
> 
>> compat mode because this does not handle private ioctl commands.
>>
>> The compat_ioctl32 callback for subdevs has outdated calling conventions,
>> but as there are no users of that, it is easy to change the function
>> pointer type and the caller to make it behave the same way as the normal
>> ioctl callback and hook in the two drivers that need no argument
>> conversion.
>>
>> Signed-off-by: Arnd Bergmann <arnd@arndb.de>
>> ---
>>  drivers/media/i2c/adv7842.c           |  3 +++
>>  drivers/media/radio/si4713/si4713.c   |  3 +++
>>  drivers/media/v4l2-core/v4l2-subdev.c | 19 ++++++++++++++++---
>>  include/media/v4l2-subdev.h           |  3 +--
>>  4 files changed, 23 insertions(+), 5 deletions(-)
>>
>> diff --git a/drivers/media/i2c/adv7842.c b/drivers/media/i2c/adv7842.c
>> index 78e61fe6f2f0..cd6df4f52f33 100644
>> --- a/drivers/media/i2c/adv7842.c
>> +++ b/drivers/media/i2c/adv7842.c
>> @@ -3293,6 +3293,9 @@ static const struct v4l2_ctrl_ops adv7842_ctrl_ops = {
>>  static const struct v4l2_subdev_core_ops adv7842_core_ops = {
>>  	.log_status = adv7842_log_status,
>>  	.ioctl = adv7842_ioctl,
>> +#ifdef CONFIG_COMPAT
>> +	.compat_ioctl32 = adv7842_ioctl,
>> +#endif
>>  	.interrupt_service_routine = adv7842_isr,
>>  	.subscribe_event = adv7842_subscribe_event,
>>  	.unsubscribe_event = v4l2_event_subdev_unsubscribe,
>> diff --git a/drivers/media/radio/si4713/si4713.c b/drivers/media/radio/si4713/si4713.c
>> index adbf43ff6a21..ae7e477774e3 100644
>> --- a/drivers/media/radio/si4713/si4713.c
>> +++ b/drivers/media/radio/si4713/si4713.c
>> @@ -1398,6 +1398,9 @@ static const struct v4l2_ctrl_ops si4713_ctrl_ops = {
>>  
>>  static const struct v4l2_subdev_core_ops si4713_subdev_core_ops = {
>>  	.ioctl		= si4713_ioctl,
>> +#ifdef CONFIG_COMPAT
>> +	.compat_ioctl32	= si4713_ioctl,
>> +#endif
> 
> Should we drop v4l2_subdev_core_ops.compat_ioctl32 and call
> v4l2_subdev_core_ops.ioctl from subdev_do_compat_ioctl32() ? New drivers
> should design custom ioctls in a way that doesn't require compat code.

I agree, we can drop it completely.

I'll skip this patch, but I'll take the other 7 patches and make a v3 PR with
updated Reviewed-by tags from Laurent.

Regards,

	Hans

> 
>>  };
>>  
>>  static const struct v4l2_subdev_tuner_ops si4713_subdev_tuner_ops = {
>> diff --git a/drivers/media/v4l2-core/v4l2-subdev.c b/drivers/media/v4l2-core/v4l2-subdev.c
>> index bf3aa9252458..fbd176d6c415 100644
>> --- a/drivers/media/v4l2-core/v4l2-subdev.c
>> +++ b/drivers/media/v4l2-core/v4l2-subdev.c
>> @@ -686,13 +686,26 @@ static long subdev_ioctl(struct file *file, unsigned int cmd,
>>  }
>>  
>>  #ifdef CONFIG_COMPAT
>> -static long subdev_compat_ioctl32(struct file *file, unsigned int cmd,
>> -	unsigned long arg)
>> +static long subdev_do_compat_ioctl32(struct file *file, unsigned int cmd, void *arg)
>>  {
>>  	struct video_device *vdev = video_devdata(file);
>>  	struct v4l2_subdev *sd = vdev_to_v4l2_subdev(vdev);
>> +	struct mutex *lock = vdev->lock;
>> +	long ret = -ENODEV;
>>  
>> -	return v4l2_subdev_call(sd, core, compat_ioctl32, cmd, arg);
>> +	if (lock && mutex_lock_interruptible(lock))
>> +		return -ERESTARTSYS;
>> +	if (video_is_registered(vdev))
>> +		ret = v4l2_subdev_call(sd, core, compat_ioctl32, cmd, arg);
>> +	if (lock)
>> +		mutex_unlock(lock);
>> +	return ret;
>> +}
>> +
>> +static long subdev_compat_ioctl32(struct file *file, unsigned int cmd,
>> +	unsigned long arg)
>> +{
>> +	return video_usercopy(file, cmd, arg, subdev_do_compat_ioctl32);
>>  }
>>  #endif
>>  
>> diff --git a/include/media/v4l2-subdev.h b/include/media/v4l2-subdev.h
>> index d0e9a5bdb08b..42aa1f6c7c3f 100644
>> --- a/include/media/v4l2-subdev.h
>> +++ b/include/media/v4l2-subdev.h
>> @@ -195,8 +195,7 @@ struct v4l2_subdev_core_ops {
>>  	int (*s_gpio)(struct v4l2_subdev *sd, u32 val);
>>  	long (*ioctl)(struct v4l2_subdev *sd, unsigned int cmd, void *arg);
>>  #ifdef CONFIG_COMPAT
>> -	long (*compat_ioctl32)(struct v4l2_subdev *sd, unsigned int cmd,
>> -			       unsigned long arg);
>> +	long (*compat_ioctl32)(struct v4l2_subdev *sd, unsigned int cmd, void *arg);
>>  #endif
>>  #ifdef CONFIG_VIDEO_ADV_DEBUG
>>  	int (*g_register)(struct v4l2_subdev *sd, struct v4l2_dbg_register *reg);
> 

Re: [PATCH v3 7/8] media: subdev: fix compat_ioctl32

[Arnd Bergmann]

On Tue, Jun 15, 2021 at 10:27 AM Hans Verkuil <hverkuil-cisco@xs4all.nl> wrote:
> On 14/06/2021 19:18, Laurent Pinchart wrote:

> >
> > Should we drop v4l2_subdev_core_ops.compat_ioctl32 and call
> > v4l2_subdev_core_ops.ioctl from subdev_do_compat_ioctl32() ? New drivers
> > should design custom ioctls in a way that doesn't require compat code.
>
> I agree, we can drop it completely.

I agree about new drivers defining their ioctls in a compatible way,
though Ideally
I'd say subdev drivers should not use custom ioctls at all. There are
two other drivers
that define private subdev ioctls and that lack a working compat handler:

- the omap3 isp driver would not work in compat mode, though I don't think there
  are any 64-bit SoCs using this hardware. The Sitara AM6 and J72 SoCs
  are the only chips in that family at the moment, but these don't
seem to include
  any image processor.

- The Intel Atom ISP driver has a broken compat ioctl handler that I remove
  in this series, but the  v4l2_subdev_core_ops->compat_ioctl32 callback
  would be the correct place to hook these up if they do not get fixed.

> I'll skip this patch, but I'll take the other 7 patches and make a v3 PR with
> updated Reviewed-by tags from Laurent.

Thanks!

        Arnd

[PATCH v3 8/8] media: subdev: disallow ioctl for saa6588/davinci

[Arnd Bergmann]

From: Arnd Bergmann <arnd@arndb.de>

The saa6588_ioctl() function expects to get called from other kernel
functions with a 'saa6588_command' pointer, but I found nothing stops it
from getting called from user space instead, which seems rather dangerous.

The same thing happens in the davinci vpbe driver with its VENC_GET_FLD
command.

As a quick fix, add a separate .command() callback pointer for this
driver and change the two callers over to that.  This change can easily
get backported to stable kernels if necessary, but since there are only
two drivers, we may want to eventually replace this with a set of more
specialized callbacks in the long run.

Fixes: c3fda7f835b0 ("V4L/DVB (10537): saa6588: convert to v4l2_subdev.")
Cc: stable@vger.kernel.org
Signed-off-by: Arnd Bergmann <arnd@arndb.de>
---
 drivers/media/i2c/saa6588.c                   | 4 ++--
 drivers/media/pci/bt8xx/bttv-driver.c         | 6 +++---
 drivers/media/pci/saa7134/saa7134-video.c     | 6 +++---
 drivers/media/platform/davinci/vpbe_display.c | 2 +-
 drivers/media/platform/davinci/vpbe_venc.c    | 6 ++----
 include/media/v4l2-subdev.h                   | 4 ++++
 6 files changed, 15 insertions(+), 13 deletions(-)

diff --git a/drivers/media/i2c/saa6588.c b/drivers/media/i2c/saa6588.c
index ecb491d5f2ab..d1e0716bdfff 100644
--- a/drivers/media/i2c/saa6588.c
+++ b/drivers/media/i2c/saa6588.c
@@ -380,7 +380,7 @@ static void saa6588_configure(struct saa6588 *s)
 
 /* ---------------------------------------------------------------------- */
 
-static long saa6588_ioctl(struct v4l2_subdev *sd, unsigned int cmd, void *arg)
+static long saa6588_command(struct v4l2_subdev *sd, unsigned int cmd, void *arg)
 {
 	struct saa6588 *s = to_saa6588(sd);
 	struct saa6588_command *a = arg;
@@ -433,7 +433,7 @@ static int saa6588_s_tuner(struct v4l2_subdev *sd, const struct v4l2_tuner *vt)
 /* ----------------------------------------------------------------------- */
 
 static const struct v4l2_subdev_core_ops saa6588_core_ops = {
-	.ioctl = saa6588_ioctl,
+	.command = saa6588_command,
 };
 
 static const struct v4l2_subdev_tuner_ops saa6588_tuner_ops = {
diff --git a/drivers/media/pci/bt8xx/bttv-driver.c b/drivers/media/pci/bt8xx/bttv-driver.c
index 1f62a9d8ea1d..0e9df8b35ac6 100644
--- a/drivers/media/pci/bt8xx/bttv-driver.c
+++ b/drivers/media/pci/bt8xx/bttv-driver.c
@@ -3179,7 +3179,7 @@ static int radio_release(struct file *file)
 
 	btv->radio_user--;
 
-	bttv_call_all(btv, core, ioctl, SAA6588_CMD_CLOSE, &cmd);
+	bttv_call_all(btv, core, command, SAA6588_CMD_CLOSE, &cmd);
 
 	if (btv->radio_user == 0)
 		btv->has_radio_tuner = 0;
@@ -3260,7 +3260,7 @@ static ssize_t radio_read(struct file *file, char __user *data,
 	cmd.result = -ENODEV;
 	radio_enable(btv);
 
-	bttv_call_all(btv, core, ioctl, SAA6588_CMD_READ, &cmd);
+	bttv_call_all(btv, core, command, SAA6588_CMD_READ, &cmd);
 
 	return cmd.result;
 }
@@ -3281,7 +3281,7 @@ static __poll_t radio_poll(struct file *file, poll_table *wait)
 	cmd.instance = file;
 	cmd.event_list = wait;
 	cmd.poll_mask = res;
-	bttv_call_all(btv, core, ioctl, SAA6588_CMD_POLL, &cmd);
+	bttv_call_all(btv, core, command, SAA6588_CMD_POLL, &cmd);
 
 	return cmd.poll_mask;
 }
diff --git a/drivers/media/pci/saa7134/saa7134-video.c b/drivers/media/pci/saa7134/saa7134-video.c
index 0f9d6b9edb90..374c8e1087de 100644
--- a/drivers/media/pci/saa7134/saa7134-video.c
+++ b/drivers/media/pci/saa7134/saa7134-video.c
@@ -1181,7 +1181,7 @@ static int video_release(struct file *file)
 
 	saa_call_all(dev, tuner, standby);
 	if (vdev->vfl_type == VFL_TYPE_RADIO)
-		saa_call_all(dev, core, ioctl, SAA6588_CMD_CLOSE, &cmd);
+		saa_call_all(dev, core, command, SAA6588_CMD_CLOSE, &cmd);
 	mutex_unlock(&dev->lock);
 
 	return 0;
@@ -1200,7 +1200,7 @@ static ssize_t radio_read(struct file *file, char __user *data,
 	cmd.result = -ENODEV;
 
 	mutex_lock(&dev->lock);
-	saa_call_all(dev, core, ioctl, SAA6588_CMD_READ, &cmd);
+	saa_call_all(dev, core, command, SAA6588_CMD_READ, &cmd);
 	mutex_unlock(&dev->lock);
 
 	return cmd.result;
@@ -1216,7 +1216,7 @@ static __poll_t radio_poll(struct file *file, poll_table *wait)
 	cmd.event_list = wait;
 	cmd.poll_mask = 0;
 	mutex_lock(&dev->lock);
-	saa_call_all(dev, core, ioctl, SAA6588_CMD_POLL, &cmd);
+	saa_call_all(dev, core, command, SAA6588_CMD_POLL, &cmd);
 	mutex_unlock(&dev->lock);
 
 	return rc | cmd.poll_mask;
diff --git a/drivers/media/platform/davinci/vpbe_display.c b/drivers/media/platform/davinci/vpbe_display.c
index d19bad997f30..bf3c3e76b921 100644
--- a/drivers/media/platform/davinci/vpbe_display.c
+++ b/drivers/media/platform/davinci/vpbe_display.c
@@ -47,7 +47,7 @@ static int venc_is_second_field(struct vpbe_display *disp_dev)
 
 	ret = v4l2_subdev_call(vpbe_dev->venc,
 			       core,
-			       ioctl,
+			       command,
 			       VENC_GET_FLD,
 			       &val);
 	if (ret < 0) {
diff --git a/drivers/media/platform/davinci/vpbe_venc.c b/drivers/media/platform/davinci/vpbe_venc.c
index 8caa084e5704..bde241c26d79 100644
--- a/drivers/media/platform/davinci/vpbe_venc.c
+++ b/drivers/media/platform/davinci/vpbe_venc.c
@@ -521,9 +521,7 @@ static int venc_s_routing(struct v4l2_subdev *sd, u32 input, u32 output,
 	return ret;
 }
 
-static long venc_ioctl(struct v4l2_subdev *sd,
-			unsigned int cmd,
-			void *arg)
+static long venc_command(struct v4l2_subdev *sd, unsigned int cmd, void *arg)
 {
 	u32 val;
 
@@ -542,7 +540,7 @@ static long venc_ioctl(struct v4l2_subdev *sd,
 }
 
 static const struct v4l2_subdev_core_ops venc_core_ops = {
-	.ioctl      = venc_ioctl,
+	.command      = venc_command,
 };
 
 static const struct v4l2_subdev_video_ops venc_video_ops = {
diff --git a/include/media/v4l2-subdev.h b/include/media/v4l2-subdev.h
index 42aa1f6c7c3f..115b1e41e933 100644
--- a/include/media/v4l2-subdev.h
+++ b/include/media/v4l2-subdev.h
@@ -162,6 +162,9 @@ struct v4l2_subdev_io_pin_config {
  * @s_gpio: set GPIO pins. Very simple right now, might need to be extended with
  *	a direction argument if needed.
  *
+ * @command: called by in-kernel drivers in order to call functions internal
+ *	   to subdev drivers driver that have a separate callback.
+ *
  * @ioctl: called at the end of ioctl() syscall handler at the V4L2 core.
  *	   used to provide support for private ioctls used on the driver.
  *
@@ -193,6 +196,7 @@ struct v4l2_subdev_core_ops {
 	int (*load_fw)(struct v4l2_subdev *sd);
 	int (*reset)(struct v4l2_subdev *sd, u32 val);
 	int (*s_gpio)(struct v4l2_subdev *sd, u32 val);
+	long (*command)(struct v4l2_subdev *sd, unsigned int cmd, void *arg);
 	long (*ioctl)(struct v4l2_subdev *sd, unsigned int cmd, void *arg);
 #ifdef CONFIG_COMPAT
 	long (*compat_ioctl32)(struct v4l2_subdev *sd, unsigned int cmd, void *arg);
-- 
2.29.2

Re: [PATCH v3 8/8] media: subdev: disallow ioctl for saa6588/davinci

[Laurent Pinchart]

Hi Arnd,

Thank you for the patch.

On Mon, Jun 14, 2021 at 12:34:09PM +0200, Arnd Bergmann wrote:
> From: Arnd Bergmann <arnd@arndb.de>
> 
> The saa6588_ioctl() function expects to get called from other kernel
> functions with a 'saa6588_command' pointer, but I found nothing stops it
> from getting called from user space instead, which seems rather dangerous.
> 
> The same thing happens in the davinci vpbe driver with its VENC_GET_FLD
> command.
> 
> As a quick fix, add a separate .command() callback pointer for this
> driver and change the two callers over to that.  This change can easily
> get backported to stable kernels if necessary, but since there are only
> two drivers, we may want to eventually replace this with a set of more
> specialized callbacks in the long run.
> 
> Fixes: c3fda7f835b0 ("V4L/DVB (10537): saa6588: convert to v4l2_subdev.")
> Cc: stable@vger.kernel.org
> Signed-off-by: Arnd Bergmann <arnd@arndb.de>

Reviewed-by: Laurent Pinchart <laurent.pinchart@ideasonboard.com>

> ---
>  drivers/media/i2c/saa6588.c                   | 4 ++--
>  drivers/media/pci/bt8xx/bttv-driver.c         | 6 +++---
>  drivers/media/pci/saa7134/saa7134-video.c     | 6 +++---
>  drivers/media/platform/davinci/vpbe_display.c | 2 +-
>  drivers/media/platform/davinci/vpbe_venc.c    | 6 ++----
>  include/media/v4l2-subdev.h                   | 4 ++++
>  6 files changed, 15 insertions(+), 13 deletions(-)
> 
> diff --git a/drivers/media/i2c/saa6588.c b/drivers/media/i2c/saa6588.c
> index ecb491d5f2ab..d1e0716bdfff 100644
> --- a/drivers/media/i2c/saa6588.c
> +++ b/drivers/media/i2c/saa6588.c
> @@ -380,7 +380,7 @@ static void saa6588_configure(struct saa6588 *s)
>  
>  /* ---------------------------------------------------------------------- */
>  
> -static long saa6588_ioctl(struct v4l2_subdev *sd, unsigned int cmd, void *arg)
> +static long saa6588_command(struct v4l2_subdev *sd, unsigned int cmd, void *arg)
>  {
>  	struct saa6588 *s = to_saa6588(sd);
>  	struct saa6588_command *a = arg;
> @@ -433,7 +433,7 @@ static int saa6588_s_tuner(struct v4l2_subdev *sd, const struct v4l2_tuner *vt)
>  /* ----------------------------------------------------------------------- */
>  
>  static const struct v4l2_subdev_core_ops saa6588_core_ops = {
> -	.ioctl = saa6588_ioctl,
> +	.command = saa6588_command,
>  };
>  
>  static const struct v4l2_subdev_tuner_ops saa6588_tuner_ops = {
> diff --git a/drivers/media/pci/bt8xx/bttv-driver.c b/drivers/media/pci/bt8xx/bttv-driver.c
> index 1f62a9d8ea1d..0e9df8b35ac6 100644
> --- a/drivers/media/pci/bt8xx/bttv-driver.c
> +++ b/drivers/media/pci/bt8xx/bttv-driver.c
> @@ -3179,7 +3179,7 @@ static int radio_release(struct file *file)
>  
>  	btv->radio_user--;
>  
> -	bttv_call_all(btv, core, ioctl, SAA6588_CMD_CLOSE, &cmd);
> +	bttv_call_all(btv, core, command, SAA6588_CMD_CLOSE, &cmd);
>  
>  	if (btv->radio_user == 0)
>  		btv->has_radio_tuner = 0;
> @@ -3260,7 +3260,7 @@ static ssize_t radio_read(struct file *file, char __user *data,
>  	cmd.result = -ENODEV;
>  	radio_enable(btv);
>  
> -	bttv_call_all(btv, core, ioctl, SAA6588_CMD_READ, &cmd);
> +	bttv_call_all(btv, core, command, SAA6588_CMD_READ, &cmd);
>  
>  	return cmd.result;
>  }
> @@ -3281,7 +3281,7 @@ static __poll_t radio_poll(struct file *file, poll_table *wait)
>  	cmd.instance = file;
>  	cmd.event_list = wait;
>  	cmd.poll_mask = res;
> -	bttv_call_all(btv, core, ioctl, SAA6588_CMD_POLL, &cmd);
> +	bttv_call_all(btv, core, command, SAA6588_CMD_POLL, &cmd);
>  
>  	return cmd.poll_mask;
>  }
> diff --git a/drivers/media/pci/saa7134/saa7134-video.c b/drivers/media/pci/saa7134/saa7134-video.c
> index 0f9d6b9edb90..374c8e1087de 100644
> --- a/drivers/media/pci/saa7134/saa7134-video.c
> +++ b/drivers/media/pci/saa7134/saa7134-video.c
> @@ -1181,7 +1181,7 @@ static int video_release(struct file *file)
>  
>  	saa_call_all(dev, tuner, standby);
>  	if (vdev->vfl_type == VFL_TYPE_RADIO)
> -		saa_call_all(dev, core, ioctl, SAA6588_CMD_CLOSE, &cmd);
> +		saa_call_all(dev, core, command, SAA6588_CMD_CLOSE, &cmd);
>  	mutex_unlock(&dev->lock);
>  
>  	return 0;
> @@ -1200,7 +1200,7 @@ static ssize_t radio_read(struct file *file, char __user *data,
>  	cmd.result = -ENODEV;
>  
>  	mutex_lock(&dev->lock);
> -	saa_call_all(dev, core, ioctl, SAA6588_CMD_READ, &cmd);
> +	saa_call_all(dev, core, command, SAA6588_CMD_READ, &cmd);
>  	mutex_unlock(&dev->lock);
>  
>  	return cmd.result;
> @@ -1216,7 +1216,7 @@ static __poll_t radio_poll(struct file *file, poll_table *wait)
>  	cmd.event_list = wait;
>  	cmd.poll_mask = 0;
>  	mutex_lock(&dev->lock);
> -	saa_call_all(dev, core, ioctl, SAA6588_CMD_POLL, &cmd);
> +	saa_call_all(dev, core, command, SAA6588_CMD_POLL, &cmd);
>  	mutex_unlock(&dev->lock);
>  
>  	return rc | cmd.poll_mask;
> diff --git a/drivers/media/platform/davinci/vpbe_display.c b/drivers/media/platform/davinci/vpbe_display.c
> index d19bad997f30..bf3c3e76b921 100644
> --- a/drivers/media/platform/davinci/vpbe_display.c
> +++ b/drivers/media/platform/davinci/vpbe_display.c
> @@ -47,7 +47,7 @@ static int venc_is_second_field(struct vpbe_display *disp_dev)
>  
>  	ret = v4l2_subdev_call(vpbe_dev->venc,
>  			       core,
> -			       ioctl,
> +			       command,
>  			       VENC_GET_FLD,
>  			       &val);
>  	if (ret < 0) {
> diff --git a/drivers/media/platform/davinci/vpbe_venc.c b/drivers/media/platform/davinci/vpbe_venc.c
> index 8caa084e5704..bde241c26d79 100644
> --- a/drivers/media/platform/davinci/vpbe_venc.c
> +++ b/drivers/media/platform/davinci/vpbe_venc.c
> @@ -521,9 +521,7 @@ static int venc_s_routing(struct v4l2_subdev *sd, u32 input, u32 output,
>  	return ret;
>  }
>  
> -static long venc_ioctl(struct v4l2_subdev *sd,
> -			unsigned int cmd,
> -			void *arg)
> +static long venc_command(struct v4l2_subdev *sd, unsigned int cmd, void *arg)
>  {
>  	u32 val;
>  
> @@ -542,7 +540,7 @@ static long venc_ioctl(struct v4l2_subdev *sd,
>  }
>  
>  static const struct v4l2_subdev_core_ops venc_core_ops = {
> -	.ioctl      = venc_ioctl,
> +	.command      = venc_command,
>  };
>  
>  static const struct v4l2_subdev_video_ops venc_video_ops = {
> diff --git a/include/media/v4l2-subdev.h b/include/media/v4l2-subdev.h
> index 42aa1f6c7c3f..115b1e41e933 100644
> --- a/include/media/v4l2-subdev.h
> +++ b/include/media/v4l2-subdev.h
> @@ -162,6 +162,9 @@ struct v4l2_subdev_io_pin_config {
>   * @s_gpio: set GPIO pins. Very simple right now, might need to be extended with
>   *	a direction argument if needed.
>   *
> + * @command: called by in-kernel drivers in order to call functions internal
> + *	   to subdev drivers driver that have a separate callback.
> + *
>   * @ioctl: called at the end of ioctl() syscall handler at the V4L2 core.
>   *	   used to provide support for private ioctls used on the driver.
>   *
> @@ -193,6 +196,7 @@ struct v4l2_subdev_core_ops {
>  	int (*load_fw)(struct v4l2_subdev *sd);
>  	int (*reset)(struct v4l2_subdev *sd, u32 val);
>  	int (*s_gpio)(struct v4l2_subdev *sd, u32 val);
> +	long (*command)(struct v4l2_subdev *sd, unsigned int cmd, void *arg);
>  	long (*ioctl)(struct v4l2_subdev *sd, unsigned int cmd, void *arg);
>  #ifdef CONFIG_COMPAT
>  	long (*compat_ioctl32)(struct v4l2_subdev *sd, unsigned int cmd, void *arg);

-- 
Regards,

Laurent Pinchart