rpms/usbredir
7
0
Fork 0
Code Issues Pull Requests Releases Activity

import usbredir-0.12.0-3.el9

Browse Source
This commit is contained in:
CentOS Sources 2022-09-27 09:38:14 -04:00 committed by Stepan Oksanichenko
parent b406af9232
commit e622f8812b
6 changed files with 288 additions and 88 deletions
Show Stats Download Patch File Download Diff File Expand all files Collapse all files

2
.gitignore vendored
View File

@ -1 +1 @@
SOURCES/usbredir-0.8.0.tar.bz2
SOURCES/usbredir-0.12.0.tar.xz

2
.usbredir.metadata
View File

@ -1 +1 @@
953126d58071d4e06d7c7f72a73e3e89e49c1ee4 SOURCES/usbredir-0.8.0.tar.bz2
70940f6dc409b3bdb9ee98f24690c438f1ae999e SOURCES/usbredir-0.12.0.tar.xz

74
SOURCES/0001-Avoid-use-after-free-in-serialization.patch
View File

@ -1,74 +0,0 @@
From 3b7e51c005a55f89f8a2e168853a96674ea52c11 Mon Sep 17 00:00:00 2001
From: Michael Hanselmann <public@hansmi.ch>
Date: Sun, 8 Aug 2021 15:35:58 +0200
Subject: [PATCH 1/1] Avoid use-after-free in serialization
Serializing parsers with large amounts of buffered write data (e.g. in case of
a slow or blocked write destination) would cause "serialize_data" to reallocate
the state buffer whose default size is 64kB (USBREDIRPARSER_SERIALIZE_BUF_SIZE).
The pointer to the position for the write buffer count would then point to
a location outside the buffer where the number of write buffers would be written
as a 32-bit value.
As of QEMU 5.2.0 the serializer is invoked for migrations. Serializations for
migrations may happen regularily such as when using the COLO feature[1].
Serialization happens under QEMU's I/O lock. The guest can't control the state
while the serialization is happening. The value written is the number of
outstanding buffers which would be suceptible to timing and host system system
load. The guest would have to continously groom the write buffers. A useful
value needs to be allocated in the exact position freed during the buffer size
increase, but before the buffer count is written. The author doesn't consider it
realistic to exploit this use-after-free reliably.
[1] https://wiki.qemu.org/Features/COLO
Signed-off-by: Michael Hanselmann <public@hansmi.ch>
(cherry picked from commit 03c519ff5831ba75120e00ebebbf1d5a1f7220ab)
---
usbredirparser/usbredirparser.c | 8 +++++---
1 file changed, 5 insertions(+), 3 deletions(-)
diff --git a/usbredirparser/usbredirparser.c b/usbredirparser/usbredirparser.c
index 8f239cc..edfdfd2 100644
--- a/usbredirparser/usbredirparser.c
+++ b/usbredirparser/usbredirparser.c
@@ -20,6 +20,7 @@
*/
#include "config.h"
+#include <stddef.h>
#include <stdio.h>
#include <stdlib.h>
#include <stdarg.h>
@@ -1581,8 +1582,9 @@ int usbredirparser_serialize(struct usbredirparser *parser_pub,
struct usbredirparser_priv *parser =
(struct usbredirparser_priv *)parser_pub;
struct usbredirparser_buf *wbuf;
- uint8_t *write_buf_count_pos, *state = NULL, *pos = NULL;
+ uint8_t *state = NULL, *pos = NULL;
uint32_t write_buf_count = 0, len, remain = 0;
+ ptrdiff_t write_buf_count_pos;
*state_dest = NULL;
*state_len = 0;
@@ -1627,7 +1629,7 @@ int usbredirparser_serialize(struct usbredirparser *parser_pub,
parser->data, parser->data_read, "packet-data"))
return -1;
- write_buf_count_pos = pos;
+ write_buf_count_pos = pos - state;
/* To be replaced with write_buf_count later */
if (serialize_int(parser, &state, &pos, &remain, 0, "write_buf_count"))
return -1;
@@ -1642,7 +1644,7 @@ int usbredirparser_serialize(struct usbredirparser *parser_pub,
wbuf = wbuf->next;
}
/* Patch in write_buf_count */
- memcpy(write_buf_count_pos, &write_buf_count, sizeof(int32_t));
+ memcpy(state + write_buf_count_pos, &write_buf_count, sizeof(int32_t));
/* Patch in length */
len = pos - state;
--
2.31.1

193
SOURCES/0001-usbredirparser-Fix-unserialize-on-pristine-check.patch Normal file
View File

@ -0,0 +1,193 @@
From 6bf41a231b445ac5190c32e281b698b1ee5379b4 Mon Sep 17 00:00:00 2001
From: Victor Toso <victortoso@redhat.com>
Date: Fri, 24 Jun 2022 23:29:08 +0200
Subject: [PATCH 1/2] usbredirparser: Fix unserialize on pristine check
Content-type: text/plain
As mentioned in the bug below, the user is trying to migrate QEMU and
it is failing on the unserialization of usbredirparser at the target
host. The user does not have USB attached to the VM at all.
I've added a test that shows that serialization is currently broken.
It fails at the 'pristine' check in usbredirparser_unserialize().
This check was added with e37d86c "Skip empty write buffers when
unserializing parser" and restricted further with 186c4c7 "Avoid
memory leak from ill-formatted serialization data"
The issue here is that usbredirparser's initialization sets some
fields and thus it isn't guaranteed to be pristine.
The parser's basic data is:
| write_buf_count ... : 1
| write_buf ........ : 0xbc03e0
| write_buf_total_size: 80
| data ............. : (nil)
| header_read: ...... : 0
| type_header_read .. : 0
| data_read: ........ : 0
The current fix is to to ignore write_buf checks as, again, they are
not guaranteed to be pristine. usbredirparser library should properly
overwrite them when unserializing the data and if there were pending
buffers, they should be freed.
Related: https://bugzilla.redhat.com/show_bug.cgi?id=2096008
Signed-off-by: Victor Toso <victortoso@redhat.com>
---
tests/meson.build | 1 +
tests/serializer.c | 113 ++++++++++++++++++++++++++++++++
usbredirparser/usbredirparser.c | 4 +-
3 files changed, 115 insertions(+), 3 deletions(-)
create mode 100644 tests/serializer.c
diff --git a/tests/meson.build b/tests/meson.build
index 0d4397b..2a179c9 100644
--- a/tests/meson.build
+++ b/tests/meson.build
@@ -1,5 +1,6 @@
tests = [
'filter',
+ 'serializer',
]
deps = dependency('glib-2.0')
diff --git a/tests/serializer.c b/tests/serializer.c
new file mode 100644
index 0000000..4bd669e
--- /dev/null
+++ b/tests/serializer.c
@@ -0,0 +1,113 @@
+/*
+ * Copyright 2022 Red Hat, Inc.
+ *
+ * This library is free software; you can redistribute it and/or
+ * modify it under the terms of the GNU Lesser General Public
+ * License as published by the Free Software Foundation; either
+ * version 2.1 of the License, or (at your option) any later version.
+ *
+ * This library is distributed in the hope that it will be useful,
+ * but WITHOUT ANY WARRANTY; without even the implied warranty of
+ * MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the GNU
+ * Lesser General Public License for more details.
+ *
+ * You should have received a copy of the GNU Lesser General Public
+ * License along with this library; if not, see <http://www.gnu.org/licenses/>.
+*/
+#include "config.h"
+
+#define G_LOG_DOMAIN "serializer"
+#define G_LOG_USE_STRUCTURED
+
+#include "usbredirparser.h"
+
+#include <errno.h>
+#include <locale.h>
+#include <glib.h>
+#include <stdlib.h>
+
+
+static void
+log_cb(void *priv, int level, const char *msg)
+{
+ GLogLevelFlags glog_level;
+
+ switch(level) {
+ case usbredirparser_error:
+ glog_level = G_LOG_LEVEL_ERROR;
+ break;
+ case usbredirparser_warning:
+ glog_level = G_LOG_LEVEL_WARNING;
+ break;
+ case usbredirparser_info:
+ glog_level = G_LOG_LEVEL_INFO;
+ break;
+ case usbredirparser_debug:
+ case usbredirparser_debug_data:
+ glog_level = G_LOG_LEVEL_DEBUG;
+ break;
+ default:
+ g_warn_if_reached();
+ return;
+ }
+ g_log_structured(G_LOG_DOMAIN, glog_level, "MESSAGE", msg);
+}
+
+static struct usbredirparser *
+get_usbredirparser(void)
+{
+ struct usbredirparser *parser = usbredirparser_create();
+ g_assert_nonnull(parser);
+
+ uint32_t caps[USB_REDIR_CAPS_SIZE] = { 0, };
+ /* Typical caps set by usbredirhost */
+ usbredirparser_caps_set_cap(caps, usb_redir_cap_connect_device_version);
+ usbredirparser_caps_set_cap(caps, usb_redir_cap_filter);
+ usbredirparser_caps_set_cap(caps, usb_redir_cap_device_disconnect_ack);
+ usbredirparser_caps_set_cap(caps, usb_redir_cap_ep_info_max_packet_size);
+ usbredirparser_caps_set_cap(caps, usb_redir_cap_64bits_ids);
+ usbredirparser_caps_set_cap(caps, usb_redir_cap_32bits_bulk_length);
+ usbredirparser_caps_set_cap(caps, usb_redir_cap_bulk_receiving);
+#if LIBUSBX_API_VERSION >= 0x01000103
+ usbredirparser_caps_set_cap(caps, usb_redir_cap_bulk_streams);
+#endif
+ int parser_flags = usbredirparser_fl_usb_host;
+
+ parser->log_func = log_cb;
+ usbredirparser_init(parser,
+ PACKAGE_STRING,
+ caps,
+ USB_REDIR_CAPS_SIZE,
+ parser_flags);
+ return parser;
+}
+
+static void
+simple (gconstpointer user_data)
+{
+ uint8_t *state = NULL;
+ int ret, len = -1;
+
+ struct usbredirparser *source = get_usbredirparser();
+ ret = usbredirparser_serialize(source, &state, &len);
+ g_assert_cmpint(ret, ==, 0);
+
+ struct usbredirparser *target = get_usbredirparser();
+ ret = usbredirparser_unserialize(target, state, len);
+ g_assert_cmpint(ret, ==, 0);
+
+ g_clear_pointer(&state, free);
+ usbredirparser_destroy(source);
+ usbredirparser_destroy(target);
+}
+
+int
+main(int argc, char **argv)
+{
+ setlocale(LC_ALL, "");
+ g_test_init(&argc, &argv, NULL);
+
+ g_test_add_data_func("/serializer/serialize-and-unserialize", NULL, simple);
+
+ return g_test_run();
+}
diff --git a/usbredirparser/usbredirparser.c b/usbredirparser/usbredirparser.c
index cd1136b..a5dd0e7 100644
--- a/usbredirparser/usbredirparser.c
+++ b/usbredirparser/usbredirparser.c
@@ -1816,9 +1816,7 @@ int usbredirparser_unserialize(struct usbredirparser *parser_pub,
return -1;
}
- if (!(parser->write_buf_count == 0 && parser->write_buf == NULL &&
- parser->write_buf_total_size == 0 &&
- parser->data == NULL && parser->header_read == 0 &&
+ if (!(parser->data == NULL && parser->header_read == 0 &&
parser->type_header_read == 0 && parser->data_read == 0)) {
ERROR("unserialization must use a pristine parser");
usbredirparser_assert_invariants(parser);
--
2.37.1

63
SOURCES/0002-usbredirparser-reset-parser-s-fields-on-unserialize.patch Normal file
View File

@ -0,0 +1,63 @@
From b93c4cae1aebda786a478677d6364308e4579ade Mon Sep 17 00:00:00 2001
From: Victor Toso <victortoso@redhat.com>
Date: Sat, 25 Jun 2022 00:29:12 +0200
Subject: [PATCH 2/2] usbredirparser: reset parser's fields on unserialize
Content-type: text/plain
This is a followup from previous commit and fixes the following leak.
| 104 (24 direct, 80 indirect) bytes in 1 blocks are definitely lost in loss record 15 of 19
| at 0x484A464: calloc (vg_replace_malloc.c:1328)
| by 0x485A238: usbredirparser_queue (usbredirparser.c:1235)
| by 0x485A571: usbredirparser_init (usbredirparser.c:227)
| by 0x40130B: get_usbredirparser (serializer.c:77)
| by 0x401379: simple (serializer.c:95)
| by 0x48FA3DD: ??? (in /usr/lib64/libglib-2.0.so.0.7200.2)
| by 0x48FA144: ??? (in /usr/lib64/libglib-2.0.so.0.7200.2)
| by 0x48FA8E1: g_test_run_suite (in /usr/lib64/libglib-2.0.so.0.7200.2)
| by 0x48FA94C: g_test_run (in /usr/lib64/libglib-2.0.so.0.7200.2)
| by 0x401161: main (serializer.c:112)
|
| LEAK SUMMARY:
| definitely lost: 24 bytes in 1 blocks
| indirectly lost: 80 bytes in 1 blocks
| possibly lost: 0 bytes in 0 blocks
| still reachable: 25,500 bytes in 17 blocks
| suppressed: 0 bytes in 0 blocks
| Reachable blocks (those to which a pointer was found) are not shown.
| To see them, rerun with: --leak-check=full --show-leak-kinds=all
Signed-off-by: Victor Toso <victortoso@redhat.com>
---
usbredirparser/usbredirparser.c | 15 +++++++++++++++
1 file changed, 15 insertions(+)
diff --git a/usbredirparser/usbredirparser.c b/usbredirparser/usbredirparser.c
index a5dd0e7..9bfc27c 100644
--- a/usbredirparser/usbredirparser.c
+++ b/usbredirparser/usbredirparser.c
@@ -1823,6 +1823,21 @@ int usbredirparser_unserialize(struct usbredirparser *parser_pub,
return -1;
}
+ {
+ /* We need to reset parser's state to receive unserialized
+ * data. */
+ struct usbredirparser_buf *wbuf = parser->write_buf;
+ while (wbuf) {
+ struct usbredirparser_buf *next_wbuf = wbuf->next;
+ free(wbuf->buf);
+ free(wbuf);
+ wbuf = next_wbuf;
+ }
+ parser->write_buf = NULL;
+ parser->write_buf_count = 0;
+ parser->write_buf_total_size = 0;
+ }
+
if (unserialize_int(parser, &state, &remain, &i, "length")) {
usbredirparser_assert_invariants(parser);
return -1;
--
2.37.1

42
SPECS/usbredir.spec
View File

@ -1,15 +1,17 @@
Name: usbredir
Version: 0.8.0
Release: 9%{?dist}
Version: 0.12.0
Release: 3%{?dist}
Summary: USB network redirection protocol libraries
License: LGPLv2+
URL: http://spice-space.org/page/UsbRedir
Source0: http://spice-space.org/download/%{name}/%{name}-%{version}.tar.bz2
Patch0001: 0001-Avoid-use-after-free-in-serialization.patch
BuildRequires: make
URL: https://spice-space.org/usbredir.html
Source0: http://spice-space.org/download/%{name}/%{name}-%{version}.tar.xz
Patch0001: 0001-usbredirparser-Fix-unserialize-on-pristine-check.patch
Patch0002: 0002-usbredirparser-reset-parser-s-fields-on-unserialize.patch
BuildRequires: gcc
BuildRequires: git-core
BuildRequires: glib2-devel
BuildRequires: libusb1-devel >= 1.0.9
BuildRequires: git-core
BuildRequires: meson
%description
The usbredir libraries allow USB devices to be used on remote and/or virtual
@ -49,13 +51,15 @@ A simple USB host TCP server, using libusbredirhost.
%build
%configure --disable-static
make %{?_smp_mflags} V=1
%meson \
-Dgit_werror=disabled \
-Dtools=enabled \
-Dfuzzing=disabled
%meson_build
%install
%make_install
rm $RPM_BUILD_ROOT%{_libdir}/libusbredir*.la
%meson_install
%ldconfig_scriptlets
@ -67,7 +71,7 @@ rm $RPM_BUILD_ROOT%{_libdir}/libusbredir*.la
%{_libdir}/libusbredir*.so.*
%files devel
%doc usb-redirection-protocol.txt README.multi-thread ChangeLog TODO
%doc docs/usb-redirection-protocol.md docs/multi-thread.md ChangeLog.md TODO
%{_includedir}/usbredir*.h
%{_libdir}/libusbredir*.so
%{_libdir}/pkgconfig/libusbredir*.pc
@ -75,11 +79,25 @@ rm $RPM_BUILD_ROOT%{_libdir}/libusbredir*.la
%files server
%{!?_licensedir:%global license %%doc}
%license COPYING
%{_bindir}/usbredirect
%{_sbindir}/usbredirserver
%{_mandir}/man1/usbredirect.1*
%{_mandir}/man1/usbredirserver.1*
%changelog
* Thu Jul 28 2022 Victor Toso <victortoso@redhat.com> - 0.12.0-3
- Fix unserialization (migration regression)
Related: rhbz#2111368
* Wed Jan 19 2022 Victor Toso <victortoso@redhat.com> - 0.12.0-2
- Fix gating process
Related: rhbz#2020215
* Mon Nov 15 2021 Victor Toso <victortoso@redhat.com> - 0.12.0-1
- Rebase to latest upstream: 0.12.0
Related: rhbz#2020215
* Wed Sep 15 2021 Victor Toso <victortoso@redhat.com> - 0.8.0-9
- Avoid use-after-free in serialization
Related: rhbz#1992873