[v4,5/8] media: v4l2-core: fix VIDIOC_DQEVENT for time64 ABI

Message ID 20191111203835.2260382-6-arnd@arndb.de (mailing list archive)
State Changes Requested, archived
Delegated to: Hans Verkuil
Headers
Series y2038 safety in v4l2 |

Commit Message

Arnd Bergmann Nov. 11, 2019, 8:38 p.m. UTC
  The v4l2_event structure contains a 'struct timespec' member that is
defined by the user space C library, creating an ABI incompatibility
when that gets updated to a 64-bit time_t.

While passing a 32-bit time_t here would be sufficient for CLOCK_MONOTONIC
timestamps, simply redefining the structure to use the kernel's
__kernel_old_timespec would not work for any library that uses a copy
of the linux/videodev2.h header file rather than including the copy from
the latest kernel headers.

This means the kernel has to be changed to handle both versions of the
structure layout on a 32-bit architecture. The easiest way to do this
is during the copy from/to user space.

Signed-off-by: Arnd Bergmann <arnd@arndb.de>
---
 drivers/media/v4l2-core/v4l2-event.c  |  5 ++++-
 drivers/media/v4l2-core/v4l2-ioctl.c  | 24 ++++++++++++++++++++-
 drivers/media/v4l2-core/v4l2-subdev.c | 20 +++++++++++++++++-
 include/uapi/linux/videodev2.h        | 30 +++++++++++++++++++++++++++
 4 files changed, 76 insertions(+), 3 deletions(-)
  

Comments

Hans Verkuil Nov. 25, 2019, 2:40 p.m. UTC | #1
On 11/11/19 9:38 PM, Arnd Bergmann wrote:
> The v4l2_event structure contains a 'struct timespec' member that is
> defined by the user space C library, creating an ABI incompatibility
> when that gets updated to a 64-bit time_t.
> 
> While passing a 32-bit time_t here would be sufficient for CLOCK_MONOTONIC
> timestamps, simply redefining the structure to use the kernel's
> __kernel_old_timespec would not work for any library that uses a copy
> of the linux/videodev2.h header file rather than including the copy from
> the latest kernel headers.
> 
> This means the kernel has to be changed to handle both versions of the
> structure layout on a 32-bit architecture. The easiest way to do this
> is during the copy from/to user space.
> 
> Signed-off-by: Arnd Bergmann <arnd@arndb.de>
> ---
>  drivers/media/v4l2-core/v4l2-event.c  |  5 ++++-
>  drivers/media/v4l2-core/v4l2-ioctl.c  | 24 ++++++++++++++++++++-
>  drivers/media/v4l2-core/v4l2-subdev.c | 20 +++++++++++++++++-
>  include/uapi/linux/videodev2.h        | 30 +++++++++++++++++++++++++++
>  4 files changed, 76 insertions(+), 3 deletions(-)
> 
> diff --git a/drivers/media/v4l2-core/v4l2-event.c b/drivers/media/v4l2-core/v4l2-event.c
> index 9d673d113d7a..290c6b213179 100644
> --- a/drivers/media/v4l2-core/v4l2-event.c
> +++ b/drivers/media/v4l2-core/v4l2-event.c
> @@ -27,6 +27,7 @@ static unsigned sev_pos(const struct v4l2_subscribed_event *sev, unsigned idx)
>  static int __v4l2_event_dequeue(struct v4l2_fh *fh, struct v4l2_event *event)
>  {
>  	struct v4l2_kevent *kev;
> +	struct timespec64 ts;
>  	unsigned long flags;
>  
>  	spin_lock_irqsave(&fh->vdev->fh_lock, flags);
> @@ -44,7 +45,9 @@ static int __v4l2_event_dequeue(struct v4l2_fh *fh, struct v4l2_event *event)
>  
>  	kev->event.pending = fh->navailable;
>  	*event = kev->event;
> -	event->timestamp = ns_to_timespec(kev->ts);
> +	ts = ns_to_timespec64(kev->ts);
> +	event->timestamp.tv_sec = ts.tv_sec;
> +	event->timestamp.tv_nsec = ts.tv_nsec;
>  	kev->sev->first = sev_pos(kev->sev, 1);
>  	kev->sev->in_use--;
>  
> diff --git a/drivers/media/v4l2-core/v4l2-ioctl.c b/drivers/media/v4l2-core/v4l2-ioctl.c
> index 693f9eb8e01b..1de939d11628 100644
> --- a/drivers/media/v4l2-core/v4l2-ioctl.c
> +++ b/drivers/media/v4l2-core/v4l2-ioctl.c
> @@ -821,7 +821,7 @@ static void v4l_print_event(const void *arg, bool write_only)
>  	const struct v4l2_event *p = arg;
>  	const struct v4l2_event_ctrl *c;
>  
> -	pr_cont("type=0x%x, pending=%u, sequence=%u, id=%u, timestamp=%lu.%9.9lu\n",
> +	pr_cont("type=0x%x, pending=%u, sequence=%u, id=%u, timestamp=%llu.%9.9llu\n",
>  			p->type, p->pending, p->sequence, p->id,
>  			p->timestamp.tv_sec, p->timestamp.tv_nsec);
>  	switch (p->type) {
> @@ -3010,6 +3010,13 @@ 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
> +	case VIDIOC_DQEVENT_TIME32:
> +		return VIDIOC_DQEVENT;
> +#endif
> +	}
> +
>  	return cmd;
>  }
>  
> @@ -3059,6 +3066,21 @@ static int video_put_user(void __user *arg, void *parg, unsigned int cmd)
>  		return 0;
>  
>  	switch (cmd) {
> +#ifdef CONFIG_COMPAT_32BIT_TIME
> +	case VIDIOC_DQEVENT_TIME32: {
> +		struct v4l2_event_time32 ev32;
> +		struct v4l2_event *ev = parg;
> +
> +	        memcpy(&ev32, ev, offsetof(struct v4l2_event, timestamp));
> +	        ev32.timestamp.tv_sec = ev->timestamp.tv_sec;
> +	        ev32.timestamp.tv_nsec = ev->timestamp.tv_nsec;
> +	        memcpy(&ev32.id, &ev->id, sizeof(*ev) - offsetof(struct v4l2_event, id));

This looks dangerous: due to 64-bit alignment requirements the
v4l2_event struct may end with a 4-byte hole at the end of the struct,
which you do not want to copy to ev32.

I think it is safer to just copy id and reserved separately:

		ev32.id = ev->id;
		memcpy(ev32.reserved, ev->reserved, sizeof(ev->reserved));

> +
> +		if (copy_to_user(arg, &ev32, sizeof(ev32)))
> +			return -EFAULT;
> +		break;
> +	}
> +#endif
>  	default:
>  		/*  Copy results into user buffer  */
>  		if (copy_to_user(arg, parg, _IOC_SIZE(cmd)))
> diff --git a/drivers/media/v4l2-core/v4l2-subdev.c b/drivers/media/v4l2-core/v4l2-subdev.c
> index f725cd9b66b9..45454a628e45 100644
> --- a/drivers/media/v4l2-core/v4l2-subdev.c
> +++ b/drivers/media/v4l2-core/v4l2-subdev.c
> @@ -331,8 +331,8 @@ static long subdev_do_ioctl(struct file *file, unsigned int cmd, void *arg)
>  	struct v4l2_fh *vfh = file->private_data;
>  #if defined(CONFIG_VIDEO_V4L2_SUBDEV_API)
>  	struct v4l2_subdev_fh *subdev_fh = to_v4l2_subdev_fh(vfh);
> -	int rval;
>  #endif
> +	int rval;
>  
>  	switch (cmd) {
>  	case VIDIOC_QUERYCTRL:
> @@ -392,6 +392,24 @@ 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);
> +
> +		memcpy(ev32, &ev, offsetof(struct v4l2_event, timestamp));
> +		ev32->timestamp.tv_sec = ev.timestamp.tv_sec;
> +		ev32->timestamp.tv_nsec = ev.timestamp.tv_nsec;
> +		memcpy(&ev32->id, &ev.id,
> +		       sizeof(ev) - offsetof(struct v4l2_event, id));

Ditto.

> +
> +		return rval;
> +	}
> +
>  	case VIDIOC_SUBSCRIBE_EVENT:
>  		return v4l2_subdev_call(sd, core, subscribe_event, vfh, arg);
>  
> diff --git a/include/uapi/linux/videodev2.h b/include/uapi/linux/videodev2.h
> index 74d3d522f3db..1d2553d4ed5b 100644
> --- a/include/uapi/linux/videodev2.h
> +++ b/include/uapi/linux/videodev2.h
> @@ -2329,11 +2329,40 @@ struct v4l2_event {
>  	} u;
>  	__u32				pending;
>  	__u32				sequence;
> +#ifdef __KERNEL__
> +	struct __kernel_timespec	timestamp;
> +#else
>  	struct timespec			timestamp;
> +#endif
>  	__u32				id;
>  	__u32				reserved[8];
>  };
>  
> +#ifdef __KERNEL__
> +/*
> + * The user space interpretation of the 'v4l2_event' differs
> + * based on the 'time_t' definition on 32-bit architectures, so
> + * the kernel has to handle both.
> + * This is the old version for 32-bit architectures.
> + */
> +struct v4l2_event_time32 {
> +	__u32				type;
> +	union {
> +		struct v4l2_event_vsync		vsync;
> +		struct v4l2_event_ctrl		ctrl;
> +		struct v4l2_event_frame_sync	frame_sync;
> +		struct v4l2_event_src_change	src_change;
> +		struct v4l2_event_motion_det	motion_det;
> +		__u8				data[64];
> +	} u;
> +	__u32				pending;
> +	__u32				sequence;
> +	struct old_timespec32		timestamp;
> +	__u32				id;
> +	__u32				reserved[8];
> +};
> +#endif
> +
>  #define V4L2_EVENT_SUB_FL_SEND_INITIAL		(1 << 0)
>  #define V4L2_EVENT_SUB_FL_ALLOW_FEEDBACK	(1 << 1)
>  
> @@ -2486,6 +2515,7 @@ struct v4l2_create_buffers {
>  #define	VIDIOC_S_DV_TIMINGS	_IOWR('V', 87, struct v4l2_dv_timings)
>  #define	VIDIOC_G_DV_TIMINGS	_IOWR('V', 88, struct v4l2_dv_timings)
>  #define	VIDIOC_DQEVENT		 _IOR('V', 89, struct v4l2_event)
> +#define	VIDIOC_DQEVENT_TIME32	 _IOR('V', 89, struct v4l2_event_time32)

Shouldn't this be under #ifdef __KERNEL__?

And should this be in the public header at all? media/v4l2-ioctl.h might be a better
place.

Regards,

	Hans

>  #define	VIDIOC_SUBSCRIBE_EVENT	 _IOW('V', 90, struct v4l2_event_subscription)
>  #define	VIDIOC_UNSUBSCRIBE_EVENT _IOW('V', 91, struct v4l2_event_subscription)
>  #define VIDIOC_CREATE_BUFS	_IOWR('V', 92, struct v4l2_create_buffers)
>
  
Arnd Bergmann Nov. 26, 2019, 2:43 p.m. UTC | #2
On Mon, Nov 25, 2019 at 3:40 PM Hans Verkuil <hverkuil@xs4all.nl> wrote:
> On 11/11/19 9:38 PM, Arnd Bergmann wrote:

> >       switch (cmd) {
> > +#ifdef CONFIG_COMPAT_32BIT_TIME
> > +     case VIDIOC_DQEVENT_TIME32: {
> > +             struct v4l2_event_time32 ev32;
> > +             struct v4l2_event *ev = parg;
> > +
> > +             memcpy(&ev32, ev, offsetof(struct v4l2_event, timestamp));
> > +             ev32.timestamp.tv_sec = ev->timestamp.tv_sec;
> > +             ev32.timestamp.tv_nsec = ev->timestamp.tv_nsec;
> > +             memcpy(&ev32.id, &ev->id, sizeof(*ev) - offsetof(struct v4l2_event, id));
>
> This looks dangerous: due to 64-bit alignment requirements the
> v4l2_event struct may end with a 4-byte hole at the end of the struct,
> which you do not want to copy to ev32.
>
> I think it is safer to just copy id and reserved separately:
>
>                 ev32.id = ev->id;
>                 memcpy(ev32.reserved, ev->reserved, sizeof(ev->reserved));

Actually I think it's that's also bad: The padding in *ev must already be
cleared here (otherwise there is a leak of stack data in the kernel
already), so  *not* copying the padding requires at least adding a memset
upfront.

I would do the per-member copy like I did for v4l2_buffer in my
other reply:

                struct v4l2_event *ev = parg;
                struct v4l2_event_time32 ev32 = {
                        .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));

                if (copy_to_user(arg, &ev32, sizeof(ev32)))
                        return -EFAULT;

Unfortunately this is a little uglier because it still requires the two
memcpy() for the arrays, but I think it's good enough.

Any other ideas? Let me know if I should do a memset()
plus individual member copy instead.
> > +             if (!(sd->flags & V4L2_SUBDEV_FL_HAS_EVENTS))
> > +                     return -ENOIOCTLCMD;
> > +
> > +             rval = v4l2_event_dequeue(vfh, &ev, file->f_flags & O_NONBLOCK);
> > +
> > +             memcpy(ev32, &ev, offsetof(struct v4l2_event, timestamp));
> > +             ev32->timestamp.tv_sec = ev.timestamp.tv_sec;
> > +             ev32->timestamp.tv_nsec = ev.timestamp.tv_nsec;
> > +             memcpy(&ev32->id, &ev.id,
> > +                    sizeof(ev) - offsetof(struct v4l2_event, id));
>
> Ditto.

Using the corresponding code here as well.

> > +
> >  #define V4L2_EVENT_SUB_FL_SEND_INITIAL               (1 << 0)
> >  #define V4L2_EVENT_SUB_FL_ALLOW_FEEDBACK     (1 << 1)
> >
> > @@ -2486,6 +2515,7 @@ struct v4l2_create_buffers {
> >  #define      VIDIOC_S_DV_TIMINGS     _IOWR('V', 87, struct v4l2_dv_timings)
> >  #define      VIDIOC_G_DV_TIMINGS     _IOWR('V', 88, struct v4l2_dv_timings)
> >  #define      VIDIOC_DQEVENT           _IOR('V', 89, struct v4l2_event)
> > +#define      VIDIOC_DQEVENT_TIME32    _IOR('V', 89, struct v4l2_event_time32)
>
> Shouldn't this be under #ifdef __KERNEL__?
>
> And should this be in the public header at all? media/v4l2-ioctl.h might be a better
> place.

Done.

       Arnd
  
Hans Verkuil Nov. 26, 2019, 3:10 p.m. UTC | #3
On 11/26/19 3:43 PM, Arnd Bergmann wrote:
> On Mon, Nov 25, 2019 at 3:40 PM Hans Verkuil <hverkuil@xs4all.nl> wrote:
>> On 11/11/19 9:38 PM, Arnd Bergmann wrote:
> 
>>>       switch (cmd) {
>>> +#ifdef CONFIG_COMPAT_32BIT_TIME
>>> +     case VIDIOC_DQEVENT_TIME32: {
>>> +             struct v4l2_event_time32 ev32;
>>> +             struct v4l2_event *ev = parg;
>>> +
>>> +             memcpy(&ev32, ev, offsetof(struct v4l2_event, timestamp));
>>> +             ev32.timestamp.tv_sec = ev->timestamp.tv_sec;
>>> +             ev32.timestamp.tv_nsec = ev->timestamp.tv_nsec;
>>> +             memcpy(&ev32.id, &ev->id, sizeof(*ev) - offsetof(struct v4l2_event, id));
>>
>> This looks dangerous: due to 64-bit alignment requirements the
>> v4l2_event struct may end with a 4-byte hole at the end of the struct,
>> which you do not want to copy to ev32.
>>
>> I think it is safer to just copy id and reserved separately:
>>
>>                 ev32.id = ev->id;
>>                 memcpy(ev32.reserved, ev->reserved, sizeof(ev->reserved));
> 
> Actually I think it's that's also bad: The padding in *ev must already be
> cleared here (otherwise there is a leak of stack data in the kernel
> already), so  *not* copying the padding requires at least adding a memset
> upfront.
> 
> I would do the per-member copy like I did for v4l2_buffer in my
> other reply:
> 
>                 struct v4l2_event *ev = parg;
>                 struct v4l2_event_time32 ev32 = {
>                         .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));
> 
>                 if (copy_to_user(arg, &ev32, sizeof(ev32)))
>                         return -EFAULT;
> 
> Unfortunately this is a little uglier because it still requires the two
> memcpy() for the arrays, but I think it's good enough.

I agree.

Hmm, can't you do .u = ev->u ? Or is that not allowed by this syntax?

> 
> Any other ideas? Let me know if I should do a memset()
> plus individual member copy instead.

I think we have to, unless you have a better solution. This is leaking
information from the holes in the struct.

>>> +             if (!(sd->flags & V4L2_SUBDEV_FL_HAS_EVENTS))
>>> +                     return -ENOIOCTLCMD;
>>> +
>>> +             rval = v4l2_event_dequeue(vfh, &ev, file->f_flags & O_NONBLOCK);
>>> +
>>> +             memcpy(ev32, &ev, offsetof(struct v4l2_event, timestamp));
>>> +             ev32->timestamp.tv_sec = ev.timestamp.tv_sec;
>>> +             ev32->timestamp.tv_nsec = ev.timestamp.tv_nsec;
>>> +             memcpy(&ev32->id, &ev.id,
>>> +                    sizeof(ev) - offsetof(struct v4l2_event, id));
>>
>> Ditto.
> 
> Using the corresponding code here as well.
> 
>>> +
>>>  #define V4L2_EVENT_SUB_FL_SEND_INITIAL               (1 << 0)
>>>  #define V4L2_EVENT_SUB_FL_ALLOW_FEEDBACK     (1 << 1)
>>>
>>> @@ -2486,6 +2515,7 @@ struct v4l2_create_buffers {
>>>  #define      VIDIOC_S_DV_TIMINGS     _IOWR('V', 87, struct v4l2_dv_timings)
>>>  #define      VIDIOC_G_DV_TIMINGS     _IOWR('V', 88, struct v4l2_dv_timings)
>>>  #define      VIDIOC_DQEVENT           _IOR('V', 89, struct v4l2_event)
>>> +#define      VIDIOC_DQEVENT_TIME32    _IOR('V', 89, struct v4l2_event_time32)
>>
>> Shouldn't this be under #ifdef __KERNEL__?
>>
>> And should this be in the public header at all? media/v4l2-ioctl.h might be a better
>> place.
> 
> Done.
> 
>        Arnd
> 

Regards,

	Hans
  
Arnd Bergmann Nov. 26, 2019, 3:19 p.m. UTC | #4
On Tue, Nov 26, 2019 at 4:10 PM Hans Verkuil <hverkuil@xs4all.nl> wrote:
>
> On 11/26/19 3:43 PM, Arnd Bergmann wrote:
> > On Mon, Nov 25, 2019 at 3:40 PM Hans Verkuil <hverkuil@xs4all.nl> wrote:
> >> On 11/11/19 9:38 PM, Arnd Bergmann wrote:
> >                 struct v4l2_event *ev = parg;
> >                 struct v4l2_event_time32 ev32 = {
> >                         .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));
> >
> >                 if (copy_to_user(arg, &ev32, sizeof(ev32)))
> >                         return -EFAULT;
> >
> > Unfortunately this is a little uglier because it still requires the two
> > memcpy() for the arrays, but I think it's good enough.
>
> I agree.
>
> Hmm, can't you do .u = ev->u ? Or is that not allowed by this syntax?

No, that doesn't work here since the two unions are considered
different types despite being defined identically. It would work by
giving the union a name, but that name would also become visible
to user space without adding more hacks.

       Arnd
  

Patch

diff --git a/drivers/media/v4l2-core/v4l2-event.c b/drivers/media/v4l2-core/v4l2-event.c
index 9d673d113d7a..290c6b213179 100644
--- a/drivers/media/v4l2-core/v4l2-event.c
+++ b/drivers/media/v4l2-core/v4l2-event.c
@@ -27,6 +27,7 @@  static unsigned sev_pos(const struct v4l2_subscribed_event *sev, unsigned idx)
 static int __v4l2_event_dequeue(struct v4l2_fh *fh, struct v4l2_event *event)
 {
 	struct v4l2_kevent *kev;
+	struct timespec64 ts;
 	unsigned long flags;
 
 	spin_lock_irqsave(&fh->vdev->fh_lock, flags);
@@ -44,7 +45,9 @@  static int __v4l2_event_dequeue(struct v4l2_fh *fh, struct v4l2_event *event)
 
 	kev->event.pending = fh->navailable;
 	*event = kev->event;
-	event->timestamp = ns_to_timespec(kev->ts);
+	ts = ns_to_timespec64(kev->ts);
+	event->timestamp.tv_sec = ts.tv_sec;
+	event->timestamp.tv_nsec = ts.tv_nsec;
 	kev->sev->first = sev_pos(kev->sev, 1);
 	kev->sev->in_use--;
 
diff --git a/drivers/media/v4l2-core/v4l2-ioctl.c b/drivers/media/v4l2-core/v4l2-ioctl.c
index 693f9eb8e01b..1de939d11628 100644
--- a/drivers/media/v4l2-core/v4l2-ioctl.c
+++ b/drivers/media/v4l2-core/v4l2-ioctl.c
@@ -821,7 +821,7 @@  static void v4l_print_event(const void *arg, bool write_only)
 	const struct v4l2_event *p = arg;
 	const struct v4l2_event_ctrl *c;
 
-	pr_cont("type=0x%x, pending=%u, sequence=%u, id=%u, timestamp=%lu.%9.9lu\n",
+	pr_cont("type=0x%x, pending=%u, sequence=%u, id=%u, timestamp=%llu.%9.9llu\n",
 			p->type, p->pending, p->sequence, p->id,
 			p->timestamp.tv_sec, p->timestamp.tv_nsec);
 	switch (p->type) {
@@ -3010,6 +3010,13 @@  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
+	case VIDIOC_DQEVENT_TIME32:
+		return VIDIOC_DQEVENT;
+#endif
+	}
+
 	return cmd;
 }
 
@@ -3059,6 +3066,21 @@  static int video_put_user(void __user *arg, void *parg, unsigned int cmd)
 		return 0;
 
 	switch (cmd) {
+#ifdef CONFIG_COMPAT_32BIT_TIME
+	case VIDIOC_DQEVENT_TIME32: {
+		struct v4l2_event_time32 ev32;
+		struct v4l2_event *ev = parg;
+
+	        memcpy(&ev32, ev, offsetof(struct v4l2_event, timestamp));
+	        ev32.timestamp.tv_sec = ev->timestamp.tv_sec;
+	        ev32.timestamp.tv_nsec = ev->timestamp.tv_nsec;
+	        memcpy(&ev32.id, &ev->id, sizeof(*ev) - offsetof(struct v4l2_event, id));
+
+		if (copy_to_user(arg, &ev32, sizeof(ev32)))
+			return -EFAULT;
+		break;
+	}
+#endif
 	default:
 		/*  Copy results into user buffer  */
 		if (copy_to_user(arg, parg, _IOC_SIZE(cmd)))
diff --git a/drivers/media/v4l2-core/v4l2-subdev.c b/drivers/media/v4l2-core/v4l2-subdev.c
index f725cd9b66b9..45454a628e45 100644
--- a/drivers/media/v4l2-core/v4l2-subdev.c
+++ b/drivers/media/v4l2-core/v4l2-subdev.c
@@ -331,8 +331,8 @@  static long subdev_do_ioctl(struct file *file, unsigned int cmd, void *arg)
 	struct v4l2_fh *vfh = file->private_data;
 #if defined(CONFIG_VIDEO_V4L2_SUBDEV_API)
 	struct v4l2_subdev_fh *subdev_fh = to_v4l2_subdev_fh(vfh);
-	int rval;
 #endif
+	int rval;
 
 	switch (cmd) {
 	case VIDIOC_QUERYCTRL:
@@ -392,6 +392,24 @@  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);
+
+		memcpy(ev32, &ev, offsetof(struct v4l2_event, timestamp));
+		ev32->timestamp.tv_sec = ev.timestamp.tv_sec;
+		ev32->timestamp.tv_nsec = ev.timestamp.tv_nsec;
+		memcpy(&ev32->id, &ev.id,
+		       sizeof(ev) - offsetof(struct v4l2_event, id));
+
+		return rval;
+	}
+
 	case VIDIOC_SUBSCRIBE_EVENT:
 		return v4l2_subdev_call(sd, core, subscribe_event, vfh, arg);
 
diff --git a/include/uapi/linux/videodev2.h b/include/uapi/linux/videodev2.h
index 74d3d522f3db..1d2553d4ed5b 100644
--- a/include/uapi/linux/videodev2.h
+++ b/include/uapi/linux/videodev2.h
@@ -2329,11 +2329,40 @@  struct v4l2_event {
 	} u;
 	__u32				pending;
 	__u32				sequence;
+#ifdef __KERNEL__
+	struct __kernel_timespec	timestamp;
+#else
 	struct timespec			timestamp;
+#endif
 	__u32				id;
 	__u32				reserved[8];
 };
 
+#ifdef __KERNEL__
+/*
+ * The user space interpretation of the 'v4l2_event' differs
+ * based on the 'time_t' definition on 32-bit architectures, so
+ * the kernel has to handle both.
+ * This is the old version for 32-bit architectures.
+ */
+struct v4l2_event_time32 {
+	__u32				type;
+	union {
+		struct v4l2_event_vsync		vsync;
+		struct v4l2_event_ctrl		ctrl;
+		struct v4l2_event_frame_sync	frame_sync;
+		struct v4l2_event_src_change	src_change;
+		struct v4l2_event_motion_det	motion_det;
+		__u8				data[64];
+	} u;
+	__u32				pending;
+	__u32				sequence;
+	struct old_timespec32		timestamp;
+	__u32				id;
+	__u32				reserved[8];
+};
+#endif
+
 #define V4L2_EVENT_SUB_FL_SEND_INITIAL		(1 << 0)
 #define V4L2_EVENT_SUB_FL_ALLOW_FEEDBACK	(1 << 1)
 
@@ -2486,6 +2515,7 @@  struct v4l2_create_buffers {
 #define	VIDIOC_S_DV_TIMINGS	_IOWR('V', 87, struct v4l2_dv_timings)
 #define	VIDIOC_G_DV_TIMINGS	_IOWR('V', 88, struct v4l2_dv_timings)
 #define	VIDIOC_DQEVENT		 _IOR('V', 89, struct v4l2_event)
+#define	VIDIOC_DQEVENT_TIME32	 _IOR('V', 89, struct v4l2_event_time32)
 #define	VIDIOC_SUBSCRIBE_EVENT	 _IOW('V', 90, struct v4l2_event_subscription)
 #define	VIDIOC_UNSUBSCRIBE_EVENT _IOW('V', 91, struct v4l2_event_subscription)
 #define VIDIOC_CREATE_BUFS	_IOWR('V', 92, struct v4l2_create_buffers)