[v4,0/8] y2038 safety in v4l2

  I'm in the process of finishing up the last bits on y2038-unsafe
code in the kernel, this series is for v4l2, which has no problem
with overflow, but has multiple ioctls that break with user space
built against a new 32-bit libc.

I posted similar patches as part of a series back in 2015, the
new version was rewritten from scratch and I double-checked with
the old version to make sure I did not miss anything I had already
taken care of before.

Hans Verkuil worked on a different patch set in 2017, but this
also did not get to the point of being merged.

My new version contains compat-ioctl support, which the old one
did not and should be complete, but given its size likely contains
bugs. I did randconfig build tests, but no runtime test, so
careful review as well as testing would be much appreciated.

With this version, the newly added code takes care of the existing
ABI, while the existing code got moved to the 64-bit time_t
interface and is used internally. This means that testing with
existing binaries should exercise most of the modifications
and if that works and doesn't get shot down in review, we can
probably live without testing the new ABI explicitly.

I'm not entirely happy with the compat-ioctl implementation that
adds quite a bit of code duplication, but I hope this is
acceptable anyway, as a better implementation would likely
require a larger refactoring of the compat-ioctl file, while
my approach simply adds support for the additional data structure
variants.

This is a minor update compared to version 3 of this series,
with bugfixes for small mistakes that I found or that were
reported by automated build bots. I updated the tree at [2]
to this version now.

      Arnd

[1] https://lwn.net/Articles/657754/
[2] https://git.kernel.org/pub/scm/linux/kernel/git/arnd/playground.git/log/?h=y2038-v4l2

Arnd Bergmann (8):
  media: documentation: fix video_event description
  media: v4l2: abstract timeval handling in v4l2_buffer
  media: v4l2-core: compat: ignore native command codes
  media: v4l2-core: split out data copy from video_usercopy
  media: v4l2-core: fix VIDIOC_DQEVENT for time64 ABI
  media: v4l2-core: fix v4l2_buffer handling for time64 ABI
  media: v4l2-core: fix compat VIDIOC_DQEVENT for time64 ABI
  media: v4l2-core: fix compat v4l2_buffer handling for time64 ABI

 .../media/uapi/dvb/video-get-event.rst        |   2 +-
 Documentation/media/uapi/dvb/video_types.rst  |   2 +-
 .../media/common/videobuf2/videobuf2-v4l2.c   |   4 +-
 drivers/media/pci/meye/meye.c                 |   4 +-
 drivers/media/usb/cpia2/cpia2_v4l.c           |   4 +-
 drivers/media/usb/stkwebcam/stk-webcam.c      |   2 +-
 drivers/media/usb/usbvision/usbvision-video.c |   4 +-
 drivers/media/v4l2-core/v4l2-compat-ioctl32.c | 470 +++++++++++++++---
 drivers/media/v4l2-core/v4l2-event.c          |   5 +-
 drivers/media/v4l2-core/v4l2-ioctl.c          | 188 +++++--
 drivers/media/v4l2-core/v4l2-subdev.c         |  20 +-
 drivers/media/v4l2-core/videobuf-core.c       |   4 +-
 include/linux/videodev2.h                     |  17 +-
 include/trace/events/v4l2.h                   |   2 +-
 include/uapi/linux/videodev2.h                |  77 +++
 15 files changed, 669 insertions(+), 136 deletions(-)
  

Hi Arnd,

On 11/11/19 9:38 PM, Arnd Bergmann wrote:
> I'm in the process of finishing up the last bits on y2038-unsafe
> code in the kernel, this series is for v4l2, which has no problem
> with overflow, but has multiple ioctls that break with user space
> built against a new 32-bit libc.

Thank you for working on this. Much appreciated!

I've reviewed this v4 series and found a few issues (mostly things
that ended up in videodev2.h that shouldn't be there).

Once a v5 is posted I'll try to test this more.

Is there an easy(-ish) way to test this with a time64 ABI? Do you
have such an environment set up for testing?

> 
> I posted similar patches as part of a series back in 2015, the
> new version was rewritten from scratch and I double-checked with
> the old version to make sure I did not miss anything I had already
> taken care of before.
> 
> Hans Verkuil worked on a different patch set in 2017, but this
> also did not get to the point of being merged.
> 
> My new version contains compat-ioctl support, which the old one
> did not and should be complete, but given its size likely contains
> bugs. I did randconfig build tests, but no runtime test, so
> careful review as well as testing would be much appreciated.
> 
> With this version, the newly added code takes care of the existing
> ABI, while the existing code got moved to the 64-bit time_t
> interface and is used internally. This means that testing with
> existing binaries should exercise most of the modifications
> and if that works and doesn't get shot down in review, we can
> probably live without testing the new ABI explicitly.
> 
> I'm not entirely happy with the compat-ioctl implementation that
> adds quite a bit of code duplication, but I hope this is
> acceptable anyway, as a better implementation would likely
> require a larger refactoring of the compat-ioctl file, while
> my approach simply adds support for the additional data structure
> variants.

A cleanup is indeed much more work. This y2038 code is awful, but that's
really because the original structs are aligned in the worst possible
way.

Regards,

	Hans

> 
> This is a minor update compared to version 3 of this series,
> with bugfixes for small mistakes that I found or that were
> reported by automated build bots. I updated the tree at [2]
> to this version now.
> 
>       Arnd
> 
> [1] https://lwn.net/Articles/657754/
> [2] https://git.kernel.org/pub/scm/linux/kernel/git/arnd/playground.git/log/?h=y2038-v4l2
> 
> Arnd Bergmann (8):
>   media: documentation: fix video_event description
>   media: v4l2: abstract timeval handling in v4l2_buffer
>   media: v4l2-core: compat: ignore native command codes
>   media: v4l2-core: split out data copy from video_usercopy
>   media: v4l2-core: fix VIDIOC_DQEVENT for time64 ABI
>   media: v4l2-core: fix v4l2_buffer handling for time64 ABI
>   media: v4l2-core: fix compat VIDIOC_DQEVENT for time64 ABI
>   media: v4l2-core: fix compat v4l2_buffer handling for time64 ABI
> 
>  .../media/uapi/dvb/video-get-event.rst        |   2 +-
>  Documentation/media/uapi/dvb/video_types.rst  |   2 +-
>  .../media/common/videobuf2/videobuf2-v4l2.c   |   4 +-
>  drivers/media/pci/meye/meye.c                 |   4 +-
>  drivers/media/usb/cpia2/cpia2_v4l.c           |   4 +-
>  drivers/media/usb/stkwebcam/stk-webcam.c      |   2 +-
>  drivers/media/usb/usbvision/usbvision-video.c |   4 +-
>  drivers/media/v4l2-core/v4l2-compat-ioctl32.c | 470 +++++++++++++++---
>  drivers/media/v4l2-core/v4l2-event.c          |   5 +-
>  drivers/media/v4l2-core/v4l2-ioctl.c          | 188 +++++--
>  drivers/media/v4l2-core/v4l2-subdev.c         |  20 +-
>  drivers/media/v4l2-core/videobuf-core.c       |   4 +-
>  include/linux/videodev2.h                     |  17 +-
>  include/trace/events/v4l2.h                   |   2 +-
>  include/uapi/linux/videodev2.h                |  77 +++
>  15 files changed, 669 insertions(+), 136 deletions(-)
>
  

On Mon, Nov 25, 2019 at 5:02 PM Hans Verkuil <hverkuil@xs4all.nl> wrote:
>
> Hi Arnd,
>
> On 11/11/19 9:38 PM, Arnd Bergmann wrote:
> > I'm in the process of finishing up the last bits on y2038-unsafe
> > code in the kernel, this series is for v4l2, which has no problem
> > with overflow, but has multiple ioctls that break with user space
> > built against a new 32-bit libc.
>
> Thank you for working on this. Much appreciated!
>
> I've reviewed this v4 series and found a few issues (mostly things
> that ended up in videodev2.h that shouldn't be there).
>
> Once a v5 is posted I'll try to test this more.

Ok, great!

>
> Is there an easy(-ish) way to test this with a time64 ABI? Do you
> have such an environment set up for testing?

If you can build your user space with musl, you can test using
a recent snapshot from http://git.musl-libc.org/cgit/musl/.

> > I'm not entirely happy with the compat-ioctl implementation that
> > adds quite a bit of code duplication, but I hope this is
> > acceptable anyway, as a better implementation would likely
> > require a larger refactoring of the compat-ioctl file, while
> > my approach simply adds support for the additional data structure
> > variants.
>
> A cleanup is indeed much more work. This y2038 code is awful, but that's
> really because the original structs are aligned in the worst possible
> way.

Ok.

         Arnd