On Thu, Dec 12, 2019 at 4:43 PM Hans Verkuil hverkuil@xs4all.nl wrote:
On 11/26/19 5:18 PM, Arnd Bergmann wrote:
switch (cmd) {+#ifdef COMPAT_32BIT_TIME
COMPAT_32BIT_TIME -> CONFIG_COMPAT_32BIT_TIME
Fixed.
*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,You forgot to copy sequence.
.memory = vb32.memory,.m.userptr = vb32.m.usercopy,usercopy -> userptr
Fixed.
.length = vb32.length,.request_fd = vb32.request_fd,};if (cmd == VIDIOC_QUERYBUF_TIME32)memset(&vb->length, 0, sizeof(*vb) -offsetof(struct v4l2_buffer, length));It's from the field AFTER vb->length that this needs to be zeroed. It's best to use the CLEAR_AFTER_FIELD macro here.
I'm a bit lost about this one: the fields that are not explicitly uninitialized here are already set to zero by the assignment above. Should this simply be a
if (cmd == VIDIOC_QUERYBUF_TIME32) vb->request_fd = 0;
then? I don't remember where that memset() originally came from or why request_fd has to be cleared here.
@@ -3100,6 +3141,30 @@ static int video_put_user(void __user *arg, void *parg, unsigned int cmd) return -EFAULT; break; }
case VIDIOC_QUERYBUF_TIME32:case VIDIOC_QBUF_TIME32:case VIDIOC_DQBUF_TIME32:case VIDIOC_PREPARE_BUF_TIME32: {struct v4l2_buffer *vb = parg;struct v4l2_buffer_time32 vb32 = {.index = vb->index,.type = vb->type,.bytesused = vb->bytesused,.flags = vb->flags,.field = vb->field,.timestamp.tv_sec = vb->timestamp.tv_sec,.timestamp.tv_usec = vb->timestamp.tv_usec,.timecode = vb->timecode,You forgot to copy sequence.
Fixed.
With these changes this patch series passed both the 64 and 32 bit compliance tests (in fact, all the issues mentioned above were found with these compliance tests).
Yay compliance tests!
I am unable to test with musl since v4l2-ctl and v4l2-compliance are C++ programs, and there doesn't appear to be an easy way to compile a C++ program with musl.
If you happen to have a test environment where you can compile C++ with musl, then let me know and I can give instructions on how to run the compliance tests.
If you can't test that, then I can merge this regardless, and hope for the best once the Y2038 fixes end up in glibc. But ideally I'd like to have this tested.
I've heard good things about the prebuilt toolchains from http://musl.cc/. These seems to come with a libstdc++, but I have not tried that myself.
I've folded the change below into this patch in my y2038-v4l2-v6 branch but have not been able to update the copy on git.kernel.org yet because of server-side issues today.
Arnd
8<----- diff --git a/drivers/media/v4l2-core/v4l2-ioctl.c b/drivers/media/v4l2-core/v4l2-ioctl.c index c416870a3166..667225712343 100644 --- a/drivers/media/v4l2-core/v4l2-ioctl.c +++ b/drivers/media/v4l2-core/v4l2-ioctl.c @@ -3055,7 +3055,7 @@ static int video_get_user(void __user *arg, void *parg, unsigned int cmd, }
switch (cmd) { -#ifdef COMPAT_32BIT_TIME +#ifdef CONFIG_COMPAT_32BIT_TIME case VIDIOC_QUERYBUF_TIME32: case VIDIOC_QBUF_TIME32: case VIDIOC_DQBUF_TIME32: @@ -3075,15 +3075,15 @@ static int video_get_user(void __user *arg, void *parg, unsigned int cmd, .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.usercopy, + .m.userptr = vb32.m.userptr, .length = vb32.length, .request_fd = vb32.request_fd, };
if (cmd == VIDIOC_QUERYBUF_TIME32) - memset(&vb->length, 0, sizeof(*vb) - - offsetof(struct v4l2_buffer, length)); + vb->request_fd = 0;
break; } @@ -3155,6 +3155,7 @@ static int video_put_user(void __user *arg, void *parg, unsigned int cmd) .timestamp.tv_sec = vb->timestamp.tv_sec, .timestamp.tv_usec = vb->timestamp.tv_usec, .timecode = vb->timecode, + .sequence = vb->sequence, .memory = vb->memory, .m.userptr = vb->m.userptr, .length = vb->length,