| Message ID | 20191111203835.2260382-6-arnd@arndb.de (mailing list archive) |
|---|---|
| State | New, archived |
| Headers | show |
| Series | y2038 safety in v4l2 | expand |
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) >
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
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
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
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)
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(-)