Hi Dafna,
On 28/07/2021 12:06, Dafna Hirschfeld wrote:
> Currently, if the ioctl VIDIOC_DQEVENT_TIME32 is called on e.g. Arm-64
> the function 'v4l2_compat_translate_cmd' doesn't have a 'translation'
> for the cmd and so 'cmd' is returned as is. This cause
> a failure '-ENOTTY' in __video_do_ioctl.
I don't really see how this would happen. As far as I can tell (and admittedly,
the code is hard to follow) on such a system there is no need for any 32-bit to
64-bit conversion in v4l2-compat-ioctl32.c since struct v4l2_event_time32 is the
same for both architectures.
And in v4l2-ioctl.c VIDIOC_DQEVENT_TIME32 is handled correctly as far as I can
tell. Where exactly does the return -ENOTTY happen?
It definitely does not make sense to modify v4l2-compat-ioctl32.c if there is
nothing to convert between 32 and 64 bit. If there is a bug, then that should be
in v4l2-ioctl.c.
Note that it is perfectly possible that there is a bug for arm64 and this ioctl,
since that's the one combination I've never tested since I don't have a test setup
for that.
Regards,
Hans
> This patch fixes it by defining VIDIOC_DQEVENT32_TIME32 to be
> VIDIOC_DQEVENT_TIME32 for non x86-64 arch and translates it
> to VIDIOC_DQEVENT.
> In addition, add a call to put_v4l2_event_time32 to copy the data
> to userspace for non x86-64 arch
>
> Signed-off-by: Dafna Hirschfeld <dafna.hirschfeld@xxxxxxxxxxxxx>
> ---
> drivers/media/v4l2-core/v4l2-compat-ioctl32.c | 21 ++++++++++++-------
> include/media/v4l2-ioctl.h | 3 +++
> 2 files changed, 16 insertions(+), 8 deletions(-)
>
> diff --git a/drivers/media/v4l2-core/v4l2-compat-ioctl32.c b/drivers/media/v4l2-core/v4l2-compat-ioctl32.c
> index 5623003a9705..20a80880d9b7 100644
> --- a/drivers/media/v4l2-core/v4l2-compat-ioctl32.c
> +++ b/drivers/media/v4l2-core/v4l2-compat-ioctl32.c
> @@ -871,9 +871,13 @@ static int put_v4l2_edid32(struct v4l2_edid *p64,
> #define VIDIOC_QUERYBUF32_TIME32 _IOWR('V', 9, struct v4l2_buffer32_time32)
> #define VIDIOC_QBUF32_TIME32 _IOWR('V', 15, struct v4l2_buffer32_time32)
> #define VIDIOC_DQBUF32_TIME32 _IOWR('V', 17, struct v4l2_buffer32_time32)
> +
> #ifdef CONFIG_X86_64
> #define VIDIOC_DQEVENT32_TIME32 _IOR ('V', 89, struct v4l2_event32_time32)
> +#else
> +#define VIDIOC_DQEVENT32_TIME32 VIDIOC_DQEVENT_TIME32
> #endif
> +
> #define VIDIOC_PREPARE_BUF32_TIME32 _IOWR('V', 93, struct v4l2_buffer32_time32)
> #endif
>
> @@ -899,6 +903,8 @@ unsigned int v4l2_compat_translate_cmd(unsigned int cmd)
> return VIDIOC_DQBUF;
> case VIDIOC_PREPARE_BUF32_TIME32:
> return VIDIOC_PREPARE_BUF;
> + case VIDIOC_DQEVENT32_TIME32:
> + return VIDIOC_DQEVENT;
> #endif
> case VIDIOC_QUERYBUF32:
> return VIDIOC_QUERYBUF;
> @@ -927,10 +933,6 @@ unsigned int v4l2_compat_translate_cmd(unsigned int cmd)
> #ifdef CONFIG_X86_64
> case VIDIOC_DQEVENT32:
> return VIDIOC_DQEVENT;
> -#ifdef CONFIG_COMPAT_32BIT_TIME
> - case VIDIOC_DQEVENT32_TIME32:
> - return VIDIOC_DQEVENT;
> -#endif
> #endif
> }
> return cmd;
> @@ -996,6 +998,13 @@ int v4l2_compat_put_user(void __user *arg, void *parg, unsigned int cmd)
> case VIDIOC_DQBUF32_TIME32:
> case VIDIOC_PREPARE_BUF32_TIME32:
> return put_v4l2_buffer32_time32(parg, arg);
> + case VIDIOC_DQEVENT32_TIME32:
> +#ifdef CONFIG_X86_64
> + return put_v4l2_event32_time32(parg, arg);
> +#else
> + return put_v4l2_event_time32(parg, arg);
> +#endif
> +
> #endif
> case VIDIOC_QUERYBUF32:
> case VIDIOC_QBUF32:
> @@ -1023,10 +1032,6 @@ int v4l2_compat_put_user(void __user *arg, void *parg, unsigned int cmd)
> #ifdef CONFIG_X86_64
> case VIDIOC_DQEVENT32:
> return put_v4l2_event32(parg, arg);
> -#ifdef CONFIG_COMPAT_32BIT_TIME
> - case VIDIOC_DQEVENT32_TIME32:
> - return put_v4l2_event32_time32(parg, arg);
> -#endif
> #endif
> }
> return 0;
> diff --git a/include/media/v4l2-ioctl.h b/include/media/v4l2-ioctl.h
> index 0c209bbbc76f..3840a3ea384c 100644
> --- a/include/media/v4l2-ioctl.h
> +++ b/include/media/v4l2-ioctl.h
> @@ -786,7 +786,10 @@ struct v4l2_buffer_time32 {
> };
>
> /*
> + * For architectures other than x86_64, the struct v4l2_event32_time32
> + * is the same as v4l2_event_time32.
> * This function is used to copy the struct v4l2_event_time32 to userspace
> + * if either the kernel is not 64-bit or if the architecture is other than x86_64.
> */
> int put_v4l2_event_time32(void *parg, void __user *arg);
>
>
