Fix VIDIOC_QBUF compat ioctl32

PATCH In review Arnaud Patard 3 participants 4 messages Jan 25, 2010 – Jul 14, 2010 Media (V4L/DVB)

Share & tools

Arnaud Patard

author

2010-01-25 15:32 UTC #0 lore raw

PATCH 1/1 Fix VIDIOC_QBUF compat ioctl32

When using VIDIOC_QBUF with memory type set to V4L2_MEMORY_MMAP, the v4l2_buffer buffer gets unmodified on drivers like uvc (well, only bytesused field is modified). Then some apps like gstreamer are reusing the same buffer later to call munmap (eg passing the buffer "length" field as 2nd parameter of munmap).

It's working fine on full 32bits but on 32bits systems with 64bit kernel, the get_v4l2_buffer32() doesn't copy length/m.offset values and then copy garbage to userspace in put_v4l2_buffer32().

This has for consequence things like that in the libv4l2 logs:

libv4l2: v4l2 unknown munmap 0x2e2b0000, -2145144908
libv4l2: v4l2 unknown munmap 0x2e530000, -2145144908

The buffer are not unmap'ed and then if the application close and open again the device, it won't work and logs will show something like:

libv4l2: error setting pixformat: Device or resource busy

The easy solution is to read length and m.offset in get_v4l2_buffer32().

Signed-off-by Arnaud Patard <[email protected]>

---

---
drivers/media/video/v4l2-compat-ioctl32.c | 5 5 + 0 - 0 !

 1 file changed, 5 insertions(+)

drivers/media/video/v4l2-compat-ioctl32.c +3 -0

Index: linux-2.6/drivers/media/video/v4l2-compat-ioctl32.c
===================================================================
@@ -475,6 +475,9 @@ static int get_v4l2_buffer32(struct v4l2
 			return -EFAULT;
 	switch (kp->memory) {
 	case V4L2_MEMORY_MMAP:
+		if (get_user(kp->length, &up->length) ||
+			get_user(kp->m.offset, &up->m.offset))
+			return -EFAULT;
 		break;
 	case V4L2_MEMORY_USERPTR:
 		{

Stefan Kost

2010-01-26 08:04 UTC #1 lore raw

Quoted 18 lines · Arnaud Patard

When using VIDIOC_QBUF with memory type set to V4L2_MEMORY_MMAP, the v4l2_buffer buffer gets unmodified on drivers like uvc (well, only bytesused field is modified). Then some apps like gstreamer are reusing the same buffer later to call munmap (eg passing the buffer "length" field as 2nd parameter of munmap).
It's working fine on full 32bits but on 32bits systems with 64bit kernel, the get_v4l2_buffer32() doesn't copy length/m.offset values and then copy garbage to userspace in put_v4l2_buffer32().
This has for consequence things like that in the libv4l2 logs:
libv4l2: v4l2 unknown munmap 0x2e2b0000, -2145144908
libv4l2: v4l2 unknown munmap 0x2e530000, -2145144908
The buffer are not unmap'ed and then if the application close and open again the device, it won't work and logs will show something like:

Quoted 8 lines

libv4l2: error setting pixformat: Device or resource busy
The easy solution is to read length and m.offset in get_v4l2_buffer32().
Signed-off-by Arnaud Patard <[email protected]>
---

I am not sure it even works fine on 32bit. Just yesterday I discovered
https://bugzilla.gnome.org/show_bug.cgi?id=608042

I get this when using gstreamer with my UVC based camera

request == VIDIOC_STREAMOFF
result == 0
libv4l2: v4l2 unknown munmap 0xb6d45000, 38400
libv4l2: v4l2 unknown munmap 0xb6d3b000, 38400

I verified that buffer address and size is correct. The libv4l code for v4l2_munmap could be a bit more verbose in the case of an error ...

Stefan

Arnaud Patard

author

2010-01-26 10:10 UTC #2 lore raw

Stefan Kost <[email protected]> writes:

Hi,

Quoted 21 lines

Quoted 20 lines · Arnaud Patard
When using VIDIOC_QBUF with memory type set to V4L2_MEMORY_MMAP, the v4l2_buffer buffer gets unmodified on drivers like uvc (well, only bytesused field is modified). Then some apps like gstreamer are reusing the same buffer later to call munmap (eg passing the buffer "length" field as 2nd parameter of munmap).
It's working fine on full 32bits but on 32bits systems with 64bit kernel, the get_v4l2_buffer32() doesn't copy length/m.offset values and then copy garbage to userspace in put_v4l2_buffer32().
This has for consequence things like that in the libv4l2 logs:
libv4l2: v4l2 unknown munmap 0x2e2b0000, -2145144908
libv4l2: v4l2 unknown munmap 0x2e530000, -2145144908
The buffer are not unmap'ed and then if the application close and open again the device, it won't work and logs will show something like:
libv4l2: error setting pixformat: Device or resource busy

Quoted 8 lines

Quoted 6 lines
The easy solution is to read length and m.offset in get_v4l2_buffer32().
Signed-off-by Arnaud Patard <[email protected]>
---
I am not sure it even works fine on 32bit. Just yesterday I discovered
https://bugzilla.gnome.org/show_bug.cgi?id=608042

My test app (cheese) is working on the 2 differents 32bits systems I tried and with this patch it's working on my system, so it's possible that your problem is different. Do you get this bug with cheese too ?

Quoted 10 lines

I get this when using gstreamer with my UVC based camera
request == VIDIOC_STREAMOFF
result == 0
libv4l2: v4l2 unknown munmap 0xb6d45000, 38400
libv4l2: v4l2 unknown munmap 0xb6d3b000, 38400
I verified that buffer address and size is correct. The libv4l code for v4l2_munmap could be a bit more verbose in the case of an error ...

iirc, libv4l2 is telling you "unknown munmap" is a result of getting a mmap call handled by the driver and not by libv4l2 (the size of your buffer is 38400 and libv4l2 wants/expects the size to be 16777216 to handle it). fwiw, I'm getting "unknown munmap" but that doesn't prevent me to change the resolution in cheese.

Arnaud

Pawel Osciak

2010-07-14 16:45 UTC #3 lore raw

In reply to Arnaud Patard

Hi,

Quoted 33 lines · Arnaud Patard

When using VIDIOC_QBUF with memory type set to V4L2_MEMORY_MMAP, the v4l2_buffer buffer gets unmodified on drivers like uvc (well, only bytesused field is modified). Then some apps like gstreamer are reusing the same buffer later to call munmap (eg passing the buffer "length" field as 2nd parameter of munmap).
It's working fine on full 32bits but on 32bits systems with 64bit kernel, the get_v4l2_buffer32() doesn't copy length/m.offset values and then copy garbage to userspace in put_v4l2_buffer32().
This has for consequence things like that in the libv4l2 logs:
libv4l2: v4l2 unknown munmap 0x2e2b0000, -2145144908 libv4l2: v4l2 unknown munmap 0x2e530000, -2145144908
The buffer are not unmap'ed and then if the application close and open again the device, it won't work and logs will show something like:
libv4l2: error setting pixformat: Device or resource busy
The easy solution is to read length and m.offset in get_v4l2_buffer32().
Signed-off-by Arnaud Patard <[email protected]> ---
---
drivers/media/video/v4l2-compat-ioctl32.c | 5 5 + 0 - 0 ! 1 file
changed, 5 insertions(+)
Index: linux-2.6/drivers/media/video/v4l2-compat-ioctl32.c =================================================================== --- linux-2.6.orig/drivers/media/video/v4l2-compat-ioctl32.c +++ linux-2.6/drivers/media/video/v4l2-compat-ioctl32.c @@ -475,6 +475,9 @@ static int get_v4l2_buffer32(struct v4l2

drivers/media/video/v4l2-compat-ioctl32.c static int get_v4l2_buffer32(struct v4l2 · line 483

Quoted 9 lines

drivers/media/video/v4l2-compat-ioctl32.c +3 -0

 			return -EFAULT;
 	switch (kp->memory) {
 	case V4L2_MEMORY_MMAP:
+		if (get_user(kp->length, &up->length) ||
+			get_user(kp->m.offset, &up->m.offset))
+			return -EFAULT;
 		break;
 	case V4L2_MEMORY_USERPTR:
 		{

Could you give more details on how this helps your application? Especially, why is length needed? Length should be returned by driver, but this is the get_* function, so userspace->kernel...

Could you explain this with a bit more context please? Thanks!

Best regards,

Signature · 3 lines

Pawel Osciak
Linux Platform Group
Samsung Poland R&D Center