-----BEGIN PGP SIGNATURE-----

iQJQBAABCAA6FiEEh6m9kz+HxgbSdvYt2ujhCXWWnOUFAl8MGfccHG1hcmNhbmRy
 ZS5sdXJlYXVAcmVkaGF0LmNvbQAKCRDa6OEJdZac5TrvD/9GwEzrEJrvngF5w9Qx
 sZBu2ZdLewQuKrIXzG7c/PVxwSjcUl9Dn69sbI6f3h5QXdP9T0en2zgeSCb3qrHJ
 dsoLsprL89R44T6ZTvDO1fe1m/44UuXfdF/p8kkyQrnPwBXFAw3ZHKkNRtcIrihn
 lu06pGUmzL8OFvPC6B1gnN1d/oqB1mqoKs7UATz+UTxrEHQgPqQbEg7Hi+VGSA6e
 rBxogPoLrFrRrLPDDsdKp7Ylj3JaD5IF5A2E9Vv2LCMkNG/YgCuA6EqGuur7lHpL
 w8H2LbSwpNWu5vZNg3BfR9hMHrM1n//gwPwjhp1GM3MrvYjhTOIGASM9Ysav7tkY
 lB+wkutdNTE4boFILMRr2GXa7O+vByEOEV4FS8jXcZ3+hK2rfzHg6Yc0/ZThhL6O
 cwQuJTgeq/+7HycIq70yE7iLabKqE0akINAH/b6DmO+oeHrQPoHFS3ULjp6a4H1y
 Nk+y6pbmyw4Rjz8TQX90azKUkV/xVI/yCJZfqoDkYD3XzJCxekeabzT9GZ2VH0Wg
 BqPWrfEmYcGTkYLwOqC/48nngIcPmhh70BJ+r2NGfmLAaYYsDSX9a49fAwLTFAUG
 +VItzwRBqtf3ZFMOcsAtIDCtFjWPU3r7J76dJZdTJafe9TFAZxRSBLtdtL9OQ6S9
 91mSJry9zGH3+2fZs6jv6PtgSg==
 =Z07J
 -----END PGP SIGNATURE-----

Merge remote-tracking branch 'remotes/elmarco/tags/chardev-pull-request' into staging

# gpg: Signature made Mon 13 Jul 2020 09:23:19 BST
# gpg:                using RSA key 87A9BD933F87C606D276F62DDAE8E10975969CE5
# gpg:                issuer "marcandre.lureau@redhat.com"
# gpg: Good signature from "Marc-André Lureau <marcandre.lureau@redhat.com>" [full]
# gpg:                 aka "Marc-André Lureau <marcandre.lureau@gmail.com>" [full]
# Primary key fingerprint: 87A9 BD93 3F87 C606 D276  F62D DAE8 E109 7596 9CE5

* remotes/elmarco/tags/chardev-pull-request:
  chardev: Extract system emulation specific code
  chardev: Reduce "char-mux.h" scope, rename it "chardev-internal.h"
  chardev: Restrict msmouse / wctablet / testdev to system emulation
  tests/test-char: Remove unused "chardev/char-mux.h" include
  monitor/misc: Remove unused "chardev/char-mux.h" include
  char: fix use-after-free with dup chardev & reconnect
  chardev: don't abort on attempt to add duplicated chardev
  char-socket: initialize reconnect timer only when the timer doesn't start

Signed-off-by: Peter Maydell <peter.maydell@linaro.org>
This commit is contained in:
Peter Maydell 2020-07-13 09:34:24 +01:00
commit 6c87d9f311
9 changed files with 195 additions and 62 deletions

View file

@ -1,4 +1,5 @@
chardev-obj-y += char.o
chardev-obj-$(CONFIG_SOFTMMU) += chardev-sysemu.o
chardev-obj-$(CONFIG_WIN32) += char-console.o
chardev-obj-$(CONFIG_POSIX) += char-fd.o
chardev-obj-y += char-fe.o
@ -17,7 +18,7 @@ chardev-obj-y += char-udp.o
chardev-obj-$(CONFIG_WIN32) += char-win.o
chardev-obj-$(CONFIG_WIN32) += char-win-stdio.o
common-obj-y += msmouse.o wctablet.o testdev.o
common-obj-$(CONFIG_SOFTMMU) += msmouse.o wctablet.o testdev.o
ifeq ($(CONFIG_BRLAPI),y)
common-obj-m += baum.o

View file

@ -29,7 +29,7 @@
#include "chardev/char-fe.h"
#include "chardev/char-io.h"
#include "chardev/char-mux.h"
#include "chardev-internal.h"
int qemu_chr_fe_write(CharBackend *be, const uint8_t *buf, int len)
{

View file

@ -29,7 +29,7 @@
#include "chardev/char.h"
#include "sysemu/block-backend.h"
#include "sysemu/sysemu.h"
#include "chardev/char-mux.h"
#include "chardev-internal.h"
/* MUX driver for serial I/O splitting */

View file

@ -490,7 +490,7 @@ static void tcp_chr_disconnect_locked(Chardev *chr)
if (emit_close) {
qemu_chr_be_event(chr, CHR_EVENT_CLOSED);
}
if (s->reconnect_time) {
if (s->reconnect_time && !s->reconnect_timer) {
qemu_chr_socket_restart_timer(chr);
}
}
@ -1129,7 +1129,8 @@ static void tcp_chr_connect_client_async(Chardev *chr)
*/
s->connect_task = qio_task_new(OBJECT(sioc),
qemu_chr_socket_connected,
chr, NULL);
object_ref(OBJECT(chr)),
(GDestroyNotify)object_unref);
qio_task_run_in_thread(s->connect_task,
tcp_chr_connect_client_task,
s->addr,

View file

@ -40,12 +40,12 @@
#include "qemu/id.h"
#include "qemu/coroutine.h"
#include "chardev/char-mux.h"
#include "chardev-internal.h"
/***********************************************************/
/* character device */
static Object *get_chardevs_root(void)
Object *get_chardevs_root(void)
{
return container_get(object_get_root(), "/chardevs");
}
@ -305,33 +305,6 @@ static const TypeInfo char_type_info = {
.class_init = char_class_init,
};
static int chardev_machine_done_notify_one(Object *child, void *opaque)
{
Chardev *chr = (Chardev *)child;
ChardevClass *class = CHARDEV_GET_CLASS(chr);
if (class->chr_machine_done) {
return class->chr_machine_done(chr);
}
return 0;
}
static void chardev_machine_done_hook(Notifier *notifier, void *unused)
{
int ret = object_child_foreach(get_chardevs_root(),
chardev_machine_done_notify_one, NULL);
if (ret) {
error_report("Failed to call chardev machine_done hooks");
exit(1);
}
}
static Notifier chardev_machine_done_notify = {
.notify = chardev_machine_done_hook,
};
static bool qemu_chr_is_busy(Chardev *s)
{
if (CHARDEV_IS_MUX(s)) {
@ -996,7 +969,11 @@ static Chardev *chardev_new(const char *id, const char *typename,
}
if (id) {
object_property_add_child(get_chardevs_root(), id, obj);
object_property_try_add_child(get_chardevs_root(), id, obj,
&local_err);
if (local_err) {
goto end;
}
object_unref(obj);
}
@ -1194,12 +1171,6 @@ void qemu_chr_cleanup(void)
static void register_types(void)
{
type_register_static(&char_type_info);
/* this must be done after machine init, since we register FEs with muxes
* as part of realize functions like serial_isa_realizefn when -nographic
* is specified
*/
qemu_add_machine_init_done_notifier(&chardev_machine_done_notify);
}
type_init(register_types);

View file

@ -1,5 +1,5 @@
/*
* QEMU System Emulator
* QEMU Character device internals
*
* Copyright (c) 2003-2008 Fabrice Bellard
*
@ -21,15 +21,17 @@
* OUT OF OR IN CONNECTION WITH THE SOFTWARE OR THE USE OR OTHER DEALINGS IN
* THE SOFTWARE.
*/
#ifndef CHAR_MUX_H
#define CHAR_MUX_H
#ifndef CHARDEV_INTERNAL_H
#define CHARDEV_INTERNAL_H
#include "chardev/char.h"
#include "chardev/char-fe.h"
#include "qom/object.h"
#define MAX_MUX 4
#define MUX_BUFFER_SIZE 32 /* Must be a power of 2. */
#define MUX_BUFFER_MASK (MUX_BUFFER_SIZE - 1)
typedef struct MuxChardev {
Chardev parent;
CharBackend *backends[MAX_MUX];
@ -58,4 +60,6 @@ typedef struct MuxChardev {
void mux_set_focus(Chardev *chr, int focus);
void mux_chr_send_all_event(Chardev *chr, QEMUChrEvent event);
Object *get_chardevs_root(void);
#endif /* CHAR_MUX_H */

69
chardev/chardev-sysemu.c Normal file
View file

@ -0,0 +1,69 @@
/*
* QEMU System Emulator
*
* Copyright (c) 2003-2008 Fabrice Bellard
*
* Permission is hereby granted, free of charge, to any person obtaining a copy
* of this software and associated documentation files (the "Software"), to deal
* in the Software without restriction, including without limitation the rights
* to use, copy, modify, merge, publish, distribute, sublicense, and/or sell
* copies of the Software, and to permit persons to whom the Software is
* furnished to do so, subject to the following conditions:
*
* The above copyright notice and this permission notice shall be included in
* all copies or substantial portions of the Software.
*
* THE SOFTWARE IS PROVIDED "AS IS", WITHOUT WARRANTY OF ANY KIND, EXPRESS OR
* IMPLIED, INCLUDING BUT NOT LIMITED TO THE WARRANTIES OF MERCHANTABILITY,
* FITNESS FOR A PARTICULAR PURPOSE AND NONINFRINGEMENT. IN NO EVENT SHALL
* THE AUTHORS OR COPYRIGHT HOLDERS BE LIABLE FOR ANY CLAIM, DAMAGES OR OTHER
* LIABILITY, WHETHER IN AN ACTION OF CONTRACT, TORT OR OTHERWISE, ARISING FROM,
* OUT OF OR IN CONNECTION WITH THE SOFTWARE OR THE USE OR OTHER DEALINGS IN
* THE SOFTWARE.
*/
#include "qemu/osdep.h"
#include "sysemu/sysemu.h"
#include "chardev/char.h"
#include "qemu/error-report.h"
#include "chardev-internal.h"
static int chardev_machine_done_notify_one(Object *child, void *opaque)
{
Chardev *chr = (Chardev *)child;
ChardevClass *class = CHARDEV_GET_CLASS(chr);
if (class->chr_machine_done) {
return class->chr_machine_done(chr);
}
return 0;
}
static void chardev_machine_done_hook(Notifier *notifier, void *unused)
{
int ret = object_child_foreach(get_chardevs_root(),
chardev_machine_done_notify_one, NULL);
if (ret) {
error_report("Failed to call chardev machine_done hooks");
exit(1);
}
}
static Notifier chardev_machine_done_notify = {
.notify = chardev_machine_done_hook,
};
static void register_types(void)
{
/*
* This must be done after machine init, since we register FEs with muxes
* as part of realize functions like serial_isa_realizefn when -nographic
* is specified.
*/
qemu_add_machine_init_done_notifier(&chardev_machine_done_notify);
}
type_init(register_types);

View file

@ -33,7 +33,6 @@
#include "exec/gdbstub.h"
#include "net/net.h"
#include "net/slirp.h"
#include "chardev/char-mux.h"
#include "ui/qemu-spice.h"
#include "qemu/config-file.h"
#include "qemu/ctype.h"

View file

@ -6,7 +6,6 @@
#include "qemu/option.h"
#include "qemu/sockets.h"
#include "chardev/char-fe.h"
#include "chardev/char-mux.h"
#include "sysemu/sysemu.h"
#include "qapi/error.h"
#include "qapi/qapi-commands-char.h"
@ -625,12 +624,14 @@ static void char_udp_test(void)
typedef struct {
int event;
bool got_pong;
CharBackend *be;
} CharSocketTestData;
#define SOCKET_PING "Hello"
#define SOCKET_PONG "World"
typedef void (*char_socket_cb)(void *opaque, QEMUChrEvent event);
static void
char_socket_event(void *opaque, QEMUChrEvent event)
@ -639,6 +640,27 @@ char_socket_event(void *opaque, QEMUChrEvent event)
data->event = event;
}
static void
char_socket_event_with_error(void *opaque, QEMUChrEvent event)
{
static bool first_error;
CharSocketTestData *data = opaque;
CharBackend *be = data->be;
data->event = event;
switch (event) {
case CHR_EVENT_OPENED:
if (!first_error) {
first_error = true;
qemu_chr_fe_disconnect(be);
}
return;
case CHR_EVENT_CLOSED:
return;
default:
return;
}
}
static void
char_socket_read(void *opaque, const uint8_t *buf, int size)
@ -699,19 +721,24 @@ char_socket_addr_to_opt_str(SocketAddress *addr, bool fd_pass,
}
static void
char_socket_ping_pong(QIOChannel *ioc)
static int
char_socket_ping_pong(QIOChannel *ioc, Error **errp)
{
char greeting[sizeof(SOCKET_PING)];
const char *response = SOCKET_PONG;
qio_channel_read_all(ioc, greeting, sizeof(greeting), &error_abort);
int ret;
ret = qio_channel_read_all(ioc, greeting, sizeof(greeting), errp);
if (ret != 0) {
object_unref(OBJECT(ioc));
return -1;
}
g_assert(memcmp(greeting, SOCKET_PING, sizeof(greeting)) == 0);
qio_channel_write_all(ioc, response, sizeof(SOCKET_PONG), &error_abort);
qio_channel_write_all(ioc, response, sizeof(SOCKET_PONG), errp);
object_unref(OBJECT(ioc));
return 0;
}
@ -723,7 +750,7 @@ char_socket_server_client_thread(gpointer data)
qio_channel_socket_connect_sync(ioc, addr, &error_abort);
char_socket_ping_pong(QIO_CHANNEL(ioc));
char_socket_ping_pong(QIO_CHANNEL(ioc), &error_abort);
return NULL;
}
@ -783,6 +810,7 @@ static void char_socket_server_test(gconstpointer opaque)
reconnect:
data.event = -1;
data.be = &be;
qemu_chr_fe_set_handlers(&be, NULL, NULL,
char_socket_event, NULL,
&data, NULL, true);
@ -855,10 +883,13 @@ char_socket_client_server_thread(gpointer data)
QIOChannelSocket *ioc = data;
QIOChannelSocket *cioc;
retry:
cioc = qio_channel_socket_accept(ioc, &error_abort);
g_assert_nonnull(cioc);
char_socket_ping_pong(QIO_CHANNEL(cioc));
if (char_socket_ping_pong(QIO_CHANNEL(cioc), NULL) != 0) {
goto retry;
}
return NULL;
}
@ -869,12 +900,59 @@ typedef struct {
const char *reconnect;
bool wait_connected;
bool fd_pass;
char_socket_cb event_cb;
} CharSocketClientTestConfig;
static void char_socket_client_dupid_test(gconstpointer opaque)
{
const CharSocketClientTestConfig *config = opaque;
QIOChannelSocket *ioc;
char *optstr;
Chardev *chr1, *chr2;
SocketAddress *addr;
QemuOpts *opts;
Error *local_err = NULL;
/*
* Setup a listener socket and determine get its address
* so we know the TCP port for the client later
*/
ioc = qio_channel_socket_new();
g_assert_nonnull(ioc);
qio_channel_socket_listen_sync(ioc, config->addr, 1, &error_abort);
addr = qio_channel_socket_get_local_address(ioc, &error_abort);
g_assert_nonnull(addr);
/*
* Populate the chardev address based on what the server
* is actually listening on
*/
optstr = char_socket_addr_to_opt_str(addr,
config->fd_pass,
config->reconnect,
false);
opts = qemu_opts_parse_noisily(qemu_find_opts("chardev"),
optstr, true);
g_assert_nonnull(opts);
chr1 = qemu_chr_new_from_opts(opts, NULL, &error_abort);
g_assert_nonnull(chr1);
chr2 = qemu_chr_new_from_opts(opts, NULL, &local_err);
g_assert_null(chr2);
error_free_or_abort(&local_err);
object_unref(OBJECT(ioc));
qemu_opts_del(opts);
object_unparent(OBJECT(chr1));
qapi_free_SocketAddress(addr);
g_free(optstr);
}
static void char_socket_client_test(gconstpointer opaque)
{
const CharSocketClientTestConfig *config = opaque;
const char_socket_cb event_cb = config->event_cb;
QIOChannelSocket *ioc;
char *optstr;
Chardev *chr;
@ -938,8 +1016,9 @@ static void char_socket_client_test(gconstpointer opaque)
reconnect:
data.event = -1;
data.be = &be;
qemu_chr_fe_set_handlers(&be, NULL, NULL,
char_socket_event, NULL,
event_cb, NULL,
&data, NULL, true);
if (config->reconnect) {
g_assert(data.event == -1);
@ -977,7 +1056,7 @@ static void char_socket_client_test(gconstpointer opaque)
/* Setup a callback to receive the reply to our greeting */
qemu_chr_fe_set_handlers(&be, char_socket_can_read,
char_socket_read,
char_socket_event, NULL,
event_cb, NULL,
&data, NULL, true);
g_assert(data.event == CHR_EVENT_OPENED);
data.event = -1;
@ -1422,17 +1501,22 @@ int main(int argc, char **argv)
#define SOCKET_CLIENT_TEST(name, addr) \
static CharSocketClientTestConfig client1 ## name = \
{ addr, NULL, false, false }; \
{ addr, NULL, false, false, char_socket_event }; \
static CharSocketClientTestConfig client2 ## name = \
{ addr, NULL, true, false }; \
{ addr, NULL, true, false, char_socket_event }; \
static CharSocketClientTestConfig client3 ## name = \
{ addr, ",reconnect=1", false }; \
{ addr, ",reconnect=1", false, false, char_socket_event }; \
static CharSocketClientTestConfig client4 ## name = \
{ addr, ",reconnect=1", true }; \
{ addr, ",reconnect=1", true, false, char_socket_event }; \
static CharSocketClientTestConfig client5 ## name = \
{ addr, NULL, false, true }; \
{ addr, NULL, false, true, char_socket_event }; \
static CharSocketClientTestConfig client6 ## name = \
{ addr, NULL, true, true }; \
{ addr, NULL, true, true, char_socket_event }; \
static CharSocketClientTestConfig client7 ## name = \
{ addr, ",reconnect=1", true, false, \
char_socket_event_with_error }; \
static CharSocketClientTestConfig client8 ## name = \
{ addr, ",reconnect=1", false, false, char_socket_event }; \
g_test_add_data_func("/char/socket/client/mainloop/" # name, \
&client1 ##name, char_socket_client_test); \
g_test_add_data_func("/char/socket/client/wait-conn/" # name, \
@ -1444,7 +1528,11 @@ int main(int argc, char **argv)
g_test_add_data_func("/char/socket/client/mainloop-fdpass/" # name, \
&client5 ##name, char_socket_client_test); \
g_test_add_data_func("/char/socket/client/wait-conn-fdpass/" # name, \
&client6 ##name, char_socket_client_test)
&client6 ##name, char_socket_client_test); \
g_test_add_data_func("/char/socket/client/reconnect-error/" # name, \
&client7 ##name, char_socket_client_test); \
g_test_add_data_func("/char/socket/client/dupid-reconnect/" # name, \
&client8 ##name, char_socket_client_dupid_test)
if (has_ipv4) {
SOCKET_SERVER_TEST(tcp, &tcpaddr);