From a1e20feffef8be2f68877545dcdac469b05ced4d Mon Sep 17 00:00:00 2001 From: Hans de Goede Date: Mon, 9 Jul 2012 13:35:29 +0200 Subject: [PATCH] Cherry-pick 2 patches from upstream git fixing an exotic crash (rhbz#838279) --- ...e-always-set-buf-length-when-convert.patch | 125 ++++++++++++++++++ ...n-t-requeue-buffers-which-we-are-ret.patch | 41 ++++++ v4l-utils.spec | 9 +- 3 files changed, 174 insertions(+), 1 deletion(-) create mode 100644 0013-libv4l2-Ensure-we-always-set-buf-length-when-convert.patch create mode 100644 0014-libv4l2-dqbuf-Don-t-requeue-buffers-which-we-are-ret.patch diff --git a/0013-libv4l2-Ensure-we-always-set-buf-length-when-convert.patch b/0013-libv4l2-Ensure-we-always-set-buf-length-when-convert.patch new file mode 100644 index 0000000..f7f2b73 --- /dev/null +++ b/0013-libv4l2-Ensure-we-always-set-buf-length-when-convert.patch @@ -0,0 +1,125 @@ +From 6662f9f04e529ddeae3828fe18b49f7b239fdb10 Mon Sep 17 00:00:00 2001 +From: Hans de Goede +Date: Mon, 9 Jul 2012 10:13:11 +0200 +Subject: [PATCH 13/14] libv4l2: Ensure we always set buf->length when + converting + +Some apps blindly use buf->length when calling munmap, even if a previous +call (ie dqbuf) on the buffer failed. We used to not set buf->length to +our conversion buffer length when the actual ioctl call to the device, or +the format conversion failed. + +When a dqbuf succeeded and the conversion failed this which would cause +us to leave buf->length set to the real buffer length rather then the +conversion buffer length, if the app then wrongly uses buf->length (*) +for a munmap we would not recognize this as munmap of the conversion +buffer and pass it through to the real munmap unmapping a part of +our conversion buf without being aware of this! + +*) After from the point of the app a failed dqbuf, so it should not trust the +contents of buf at all. + +This patch makes things work even for buggy apps, by always setting +buf->length when converting even if things fail. + +Signed-off-by: Hans de Goede + +Conflicts: + lib/libv4l2/libv4l2.c +--- + lib/libv4l2/libv4l2.c | 52 +++++++++++++++++++++++++++++-------------------- + 1 file changed, 31 insertions(+), 21 deletions(-) + +diff --git a/lib/libv4l2/libv4l2.c b/lib/libv4l2/libv4l2.c +index 089fc38..a8f17ed 100644 +--- a/lib/libv4l2/libv4l2.c ++++ b/lib/libv4l2/libv4l2.c +@@ -486,6 +486,23 @@ static int v4l2_needs_conversion(int index) + return 0; + } + ++static void v4l2_set_conversion_buf_params(int index, struct v4l2_buffer *buf) ++{ ++ if (!v4l2_needs_conversion(index)) ++ return; ++ ++ /* This may happen if the ioctl failed */ ++ if (buf->index >= devices[index].no_frames) ++ buf->index = 0; ++ ++ buf->m.offset = V4L2_MMAP_OFFSET_MAGIC | buf->index; ++ buf->length = V4L2_FRAME_BUF_SIZE; ++ if (devices[index].frame_map_count[buf->index]) ++ buf->flags |= V4L2_BUF_FLAG_MAPPED; ++ else ++ buf->flags &= ~V4L2_BUF_FLAG_MAPPED; ++} ++ + static int v4l2_buffers_mapped(int index) + { + unsigned int i; +@@ -1182,19 +1199,14 @@ int v4l2_ioctl(int fd, unsigned long int request, ...) + /* Do a real query even when converting to let the driver fill in + things like buf->field */ + result = SYS_IOCTL(devices[index].fd, VIDIOC_QUERYBUF, buf); +- if (result || !v4l2_needs_conversion(index)) +- break; + +- buf->m.offset = V4L2_MMAP_OFFSET_MAGIC | buf->index; +- buf->length = V4L2_FRAME_BUF_SIZE; +- if (devices[index].frame_map_count[buf->index]) +- buf->flags |= V4L2_BUF_FLAG_MAPPED; +- else +- buf->flags &= ~V4L2_BUF_FLAG_MAPPED; ++ v4l2_set_conversion_buf_params(index, buf); + break; + } + +- case VIDIOC_QBUF: ++ case VIDIOC_QBUF: { ++ struct v4l2_buffer *buf = arg; ++ + if (devices[index].flags & V4L2_STREAM_CONTROLLED_BY_READ) { + result = v4l2_deactivate_read_stream(index); + if (result) +@@ -1209,7 +1221,10 @@ int v4l2_ioctl(int fd, unsigned long int request, ...) + } + + result = SYS_IOCTL(devices[index].fd, VIDIOC_QBUF, arg); ++ ++ v4l2_set_conversion_buf_params(index, buf); + break; ++ } + + case VIDIOC_DQBUF: { + struct v4l2_buffer *buf = arg; +@@ -1248,19 +1263,14 @@ int v4l2_ioctl(int fd, unsigned long int request, ...) + } + } + +- result = v4l2_dequeue_and_convert(index, buf, 0, V4L2_FRAME_BUF_SIZE); +- if (result < 0) +- break; +- +- buf->bytesused = result; +- buf->m.offset = V4L2_MMAP_OFFSET_MAGIC | buf->index; +- buf->length = V4L2_FRAME_BUF_SIZE; +- if (devices[index].frame_map_count[buf->index]) +- buf->flags |= V4L2_BUF_FLAG_MAPPED; +- else +- buf->flags &= ~V4L2_BUF_FLAG_MAPPED; ++ result = v4l2_dequeue_and_convert(index, buf, 0, ++ V4L2_FRAME_BUF_SIZE); ++ if (result >= 0) { ++ buf->bytesused = result; ++ result = 0; ++ } + +- result = 0; ++ v4l2_set_conversion_buf_params(index, buf); + break; + } + +-- +1.7.10.4 + diff --git a/0014-libv4l2-dqbuf-Don-t-requeue-buffers-which-we-are-ret.patch b/0014-libv4l2-dqbuf-Don-t-requeue-buffers-which-we-are-ret.patch new file mode 100644 index 0000000..42ea6f0 --- /dev/null +++ b/0014-libv4l2-dqbuf-Don-t-requeue-buffers-which-we-are-ret.patch @@ -0,0 +1,41 @@ +From 8fcfbbc58a0b6a0eeb31c02493e164f82236a8e6 Mon Sep 17 00:00:00 2001 +From: Hans de Goede +Date: Mon, 9 Jul 2012 13:23:41 +0200 +Subject: [PATCH 14/14] libv4l2: dqbuf: Don't requeue buffers which we are + returning to the app + +We retry dqbuf when we get a short frame or a decode error, but if the retries +are exhausted and the frame is short we will return the (short) buffer to the +caller, since returning a short frame is better then not returning anything +at all. + +This means that we must not re-queue it then! Also see: +https://bugzilla.redhat.com/show_bug.cgi?id=838279 + +Signed-off-by: Hans de Goede +--- + lib/libv4l2/libv4l2.c | 8 +++++++- + 1 file changed, 7 insertions(+), 1 deletion(-) + +diff --git a/lib/libv4l2/libv4l2.c b/lib/libv4l2/libv4l2.c +index a8f17ed..070ac0d 100644 +--- a/lib/libv4l2/libv4l2.c ++++ b/lib/libv4l2/libv4l2.c +@@ -317,7 +317,13 @@ static int v4l2_dequeue_and_convert(int index, struct v4l2_buffer *buf, + V4L2_LOG_ERR("converting / decoding frame data: %s", + v4lconvert_get_error_message(devices[index].convert)); + +- v4l2_queue_read_buffer(index, buf->index); ++ /* ++ * If this is the last try, and the frame is short ++ * we will return the (short) buffer to the caller, ++ * so we must not re-queue it then! ++ */ ++ if (!(tries == 1 && errno == EPIPE)) ++ v4l2_queue_read_buffer(index, buf->index); + errno = saved_err; + } + tries--; +-- +1.7.10.4 + diff --git a/v4l-utils.spec b/v4l-utils.spec index e3ffaae..701fe72 100644 --- a/v4l-utils.spec +++ b/v4l-utils.spec @@ -1,6 +1,6 @@ Name: v4l-utils Version: 0.8.8 -Release: 1%{?dist} +Release: 2%{?dist} Summary: Utilities for video4linux and DVB devices Group: Applications/System # ir-keytable and v4l2-sysfs-path are GPLv2 only @@ -20,6 +20,8 @@ Patch9: 0009-libv4lcontrol-Add-Lenovo-Thinkpad-X220-Tablet-to-ups.patch Patch10: 0010-libv4l2-Improve-VIDIOC_-_FMT-logging.patch Patch11: 0011-libv4lconvert-replace-strndupa-with-more-portable-st.patch Patch12: 0012-libdvbv5-Add-missing-includes.patch +Patch13: 0013-libv4l2-Ensure-we-always-set-buf-length-when-convert.patch +Patch14: 0014-libv4l2-dqbuf-Don-t-requeue-buffers-which-we-are-ret.patch BuildRequires: libjpeg-devel qt4-devel kernel-headers desktop-file-utils # For /lib/udev/rules.d ownership Requires: udev @@ -102,6 +104,8 @@ developing applications that use libv4l. %patch10 -p1 %patch11 -p1 %patch12 -p1 +%patch13 -p1 +%patch14 -p1 %build @@ -175,6 +179,9 @@ gtk-update-icon-cache %{_datadir}/icons/hicolor &>/dev/null || : %changelog +* Mon Jul 9 2012 Hans de Goede - 0.8.8-2 +- Cherry-pick 2 patches from upstream git fixing an exotic crash (rhbz#838279) + * Tue May 22 2012 Hans de Goede - 0.8.8-1 - New upstream release 0.8.8 - Add patches from upstream git to improve Pixart JPEG decoding