* [Qemu-devel] [PATCH] vhost-user: don't poke at chardev internal QemuOpts
@ 2016-10-07 9:04 Daniel P. Berrange
2016-10-07 9:10 ` Marc-André Lureau
0 siblings, 1 reply; 4+ messages in thread
From: Daniel P. Berrange @ 2016-10-07 9:04 UTC (permalink / raw)
To: qemu-devel
Cc: Michal Privoznik, marcandre.lureau, Markus Armbruster,
Paolo Bonzini, Michael S. Tsirkin, Daniel P. Berrange
The vhost-user code is poking at the QemuOpts instance
in the CharDriverState struct, not realizing that it is
valid for this to be NULL. e.g. the following crash
shows a codepath where it will be NULL:
Program terminated with signal SIGSEGV, Segmentation fault.
#0 0x000055baf6ab4adc in qemu_opt_foreach (opts=0x0, func=0x55baf696b650 <net_vhost_chardev_opts>, opaque=0x7ffc51368c00, errp=0x7ffc51368e48) at util/qemu-option.c:617
617 QTAILQ_FOREACH(opt, &opts->head, next) {
[Current thread is 1 (Thread 0x7f1d4970bb40 (LWP 6603))]
(gdb) bt
#0 0x000055baf6ab4adc in qemu_opt_foreach (opts=0x0, func=0x55baf696b650 <net_vhost_chardev_opts>, opaque=0x7ffc51368c00, errp=0x7ffc51368e48) at util/qemu-option.c:617
#1 0x000055baf696b7da in net_vhost_parse_chardev (opts=0x55baf8ff9260, errp=0x7ffc51368e48) at net/vhost-user.c:314
#2 0x000055baf696b985 in net_init_vhost_user (netdev=0x55baf8ff9250, name=0x55baf879d270 "hostnet2", peer=0x0, errp=0x7ffc51368e48) at net/vhost-user.c:360
#3 0x000055baf6960216 in net_client_init1 (object=0x55baf8ff9250, is_netdev=true, errp=0x7ffc51368e48) at net/net.c:1051
#4 0x000055baf6960518 in net_client_init (opts=0x55baf776e7e0, is_netdev=true, errp=0x7ffc51368f00) at net/net.c:1108
#5 0x000055baf696083f in netdev_add (opts=0x55baf776e7e0, errp=0x7ffc51368f00) at net/net.c:1186
#6 0x000055baf69608c7 in qmp_netdev_add (qdict=0x55baf7afaf60, ret=0x7ffc51368f50, errp=0x7ffc51368f48) at net/net.c:1205
#7 0x000055baf6622135 in handle_qmp_command (parser=0x55baf77fb590, tokens=0x7f1d24011960) at /path/to/qemu.git/monitor.c:3978
#8 0x000055baf6a9d099 in json_message_process_token (lexer=0x55baf77fb598, input=0x55baf75acd20, type=JSON_RCURLY, x=113, y=19) at qobject/json-streamer.c:105
#9 0x000055baf6abf7aa in json_lexer_feed_char (lexer=0x55baf77fb598, ch=125 '}', flush=false) at qobject/json-lexer.c:319
#10 0x000055baf6abf8f2 in json_lexer_feed (lexer=0x55baf77fb598, buffer=0x7ffc51369170 "}R\204\367\272U", size=1) at qobject/json-lexer.c:369
#11 0x000055baf6a9d13c in json_message_parser_feed (parser=0x55baf77fb590, buffer=0x7ffc51369170 "}R\204\367\272U", size=1) at qobject/json-streamer.c:124
#12 0x000055baf66221f7 in monitor_qmp_read (opaque=0x55baf77fb530, buf=0x7ffc51369170 "}R\204\367\272U", size=1) at /path/to/qemu.git/monitor.c:3994
#13 0x000055baf6757014 in qemu_chr_be_write_impl (s=0x55baf7610a40, buf=0x7ffc51369170 "}R\204\367\272U", len=1) at qemu-char.c:387
#14 0x000055baf6757076 in qemu_chr_be_write (s=0x55baf7610a40, buf=0x7ffc51369170 "}R\204\367\272U", len=1) at qemu-char.c:399
#15 0x000055baf675b3b0 in tcp_chr_read (chan=0x55baf90244b0, cond=G_IO_IN, opaque=0x55baf7610a40) at qemu-char.c:2927
#16 0x000055baf6a5d655 in qio_channel_fd_source_dispatch (source=0x55baf7610df0, callback=0x55baf675b25a <tcp_chr_read>, user_data=0x55baf7610a40) at io/channel-watch.c:84
#17 0x00007f1d3e80cbbd in g_main_context_dispatch () from /usr/lib64/libglib-2.0.so.0
#18 0x000055baf69d3720 in glib_pollfds_poll () at main-loop.c:213
#19 0x000055baf69d37fd in os_host_main_loop_wait (timeout=126000000) at main-loop.c:258
#20 0x000055baf69d38ad in main_loop_wait (nonblocking=0) at main-loop.c:506
#21 0x000055baf676587b in main_loop () at vl.c:1908
#22 0x000055baf676d3bf in main (argc=101, argv=0x7ffc5136a6c8, envp=0x7ffc5136a9f8) at vl.c:4604
(gdb) p opts
$1 = (QemuOpts *) 0x0
The crash occurred when attaching vhost-user net via QMP:
{
"execute": "chardev-add",
"arguments": {
"id": "charnet2",
"backend": {
"type": "socket",
"data": {
"addr": {
"type": "unix",
"data": {
"path": "/var/run/openvswitch/vhost-user1"
}
},
"wait": false,
"server": false
}
}
},
"id": "libvirt-19"
}
{
"return": {
},
"id": "libvirt-19"
}
{
"execute": "netdev_add",
"arguments": {
"type": "vhost-user",
"chardev": "charnet2",
"id": "hostnet2"
},
"id": "libvirt-20"
}
The vhost-user code should not be poking at the internals of the
CharDriverState struct. What it wants is a chardev that is operating
as a network server, so add an explicit API to allow it to validate
this feature condition without looking at chardev internals.
Signed-off-by: Daniel P. Berrange <berrange@redhat.com>
---
NB, I've not functionally tested this - just compiled and
unit tested.
include/sysemu/char.h | 1 +
net/vhost-user.c | 40 ++++------------------------------------
qemu-char.c | 7 +++++++
3 files changed, 12 insertions(+), 36 deletions(-)
diff --git a/include/sysemu/char.h b/include/sysemu/char.h
index 0d0465a..50a6603 100644
--- a/include/sysemu/char.h
+++ b/include/sysemu/char.h
@@ -437,6 +437,7 @@ int qemu_chr_add_client(CharDriverState *s, int fd);
CharDriverState *qemu_chr_find(const char *name);
bool chr_is_ringbuf(const CharDriverState *chr);
+bool qemu_chr_is_network_server(CharDriverState *chr);
QemuOpts *qemu_chr_parse_compat(const char *label, const char *filename);
void register_char_driver(const char *name, ChardevBackendKind kind,
diff --git a/net/vhost-user.c b/net/vhost-user.c
index b0595f8..2e2cf24 100644
--- a/net/vhost-user.c
+++ b/net/vhost-user.c
@@ -27,11 +27,6 @@ typedef struct VhostUserState {
bool started;
} VhostUserState;
-typedef struct VhostUserChardevProps {
- bool is_socket;
- bool is_unix;
-} VhostUserChardevProps;
-
VHostNetState *vhost_user_get_vhost_net(NetClientState *nc)
{
VhostUserState *s = DO_UPCAST(VhostUserState, nc, nc);
@@ -278,45 +273,18 @@ static int net_vhost_user_init(NetClientState *peer, const char *device,
return 0;
}
-static int net_vhost_chardev_opts(void *opaque,
- const char *name, const char *value,
- Error **errp)
-{
- VhostUserChardevProps *props = opaque;
-
- if (strcmp(name, "backend") == 0 && strcmp(value, "socket") == 0) {
- props->is_socket = true;
- } else if (strcmp(name, "path") == 0) {
- props->is_unix = true;
- } else if (strcmp(name, "server") == 0) {
- } else {
- error_setg(errp,
- "vhost-user does not support a chardev with option %s=%s",
- name, value);
- return -1;
- }
- return 0;
-}
-
-static CharDriverState *net_vhost_parse_chardev(
+static CharDriverState *net_vhost_claim_chardev(
const NetdevVhostUserOptions *opts, Error **errp)
{
CharDriverState *chr = qemu_chr_find(opts->chardev);
- VhostUserChardevProps props;
if (chr == NULL) {
error_setg(errp, "chardev \"%s\" not found", opts->chardev);
return NULL;
}
- /* inspect chardev opts */
- memset(&props, 0, sizeof(props));
- if (qemu_opt_foreach(chr->opts, net_vhost_chardev_opts, &props, errp)) {
- return NULL;
- }
-
- if (!props.is_socket || !props.is_unix) {
- error_setg(errp, "chardev \"%s\" is not a unix socket",
+ if (!qemu_chr_is_network_server(chr)) {
+ error_setg(errp, "chardev \"%s\" does not provide a network server",
opts->chardev);
return NULL;
}
@@ -357,7 +325,7 @@ int net_init_vhost_user(const Netdev *netdev, const char *name,
assert(netdev->type == NET_CLIENT_DRIVER_VHOST_USER);
vhost_user_opts = &netdev->u.vhost_user;
- chr = net_vhost_parse_chardev(vhost_user_opts, errp);
+ chr = net_vhost_claim_chardev(vhost_user_opts, errp);
if (!chr) {
return -1;
}
diff --git a/qemu-char.c b/qemu-char.c
index fb456ce..977fb1a 100644
--- a/qemu-char.c
+++ b/qemu-char.c
@@ -4596,6 +4596,13 @@ static CharDriverState *qmp_chardev_open_udp(const char *id,
return qemu_chr_open_udp(sioc, common, errp);
}
+
+bool qemu_chr_is_network_server(CharDriverState *chr)
+{
+ return chr->chr_wait_connected != NULL;
+}
+
+
ChardevReturn *qmp_chardev_add(const char *id, ChardevBackend *backend,
Error **errp)
{
--
2.7.4
^ permalink raw reply related [flat|nested] 4+ messages in thread
* Re: [Qemu-devel] [PATCH] vhost-user: don't poke at chardev internal QemuOpts
2016-10-07 9:04 [Qemu-devel] [PATCH] vhost-user: don't poke at chardev internal QemuOpts Daniel P. Berrange
@ 2016-10-07 9:10 ` Marc-André Lureau
2016-10-07 9:15 ` Daniel P. Berrange
0 siblings, 1 reply; 4+ messages in thread
From: Marc-André Lureau @ 2016-10-07 9:10 UTC (permalink / raw)
To: Daniel P. Berrange, qemu-devel
Cc: Michal Privoznik, Markus Armbruster, Paolo Bonzini, Michael S. Tsirkin
Hi
On Fri, Oct 7, 2016 at 1:04 PM Daniel P. Berrange <berrange@redhat.com>
wrote:
> The vhost-user code is poking at the QemuOpts instance
> in the CharDriverState struct, not realizing that it is
> valid for this to be NULL. e.g. the following crash
> shows a codepath where it will be NULL:
>
> Program terminated with signal SIGSEGV, Segmentation fault.
> #0 0x000055baf6ab4adc in qemu_opt_foreach (opts=0x0, func=0x55baf696b650
> <net_vhost_chardev_opts>, opaque=0x7ffc51368c00, errp=0x7ffc51368e48) at
> util/qemu-option.c:617
> 617 QTAILQ_FOREACH(opt, &opts->head, next) {
> [Current thread is 1 (Thread 0x7f1d4970bb40 (LWP 6603))]
> (gdb) bt
> #0 0x000055baf6ab4adc in qemu_opt_foreach (opts=0x0, func=0x55baf696b650
> <net_vhost_chardev_opts>, opaque=0x7ffc51368c00, errp=0x7ffc51368e48) at
> util/qemu-option.c:617
> #1 0x000055baf696b7da in net_vhost_parse_chardev (opts=0x55baf8ff9260,
> errp=0x7ffc51368e48) at net/vhost-user.c:314
> #2 0x000055baf696b985 in net_init_vhost_user (netdev=0x55baf8ff9250,
> name=0x55baf879d270 "hostnet2", peer=0x0, errp=0x7ffc51368e48) at
> net/vhost-user.c:360
> #3 0x000055baf6960216 in net_client_init1 (object=0x55baf8ff9250,
> is_netdev=true, errp=0x7ffc51368e48) at net/net.c:1051
> #4 0x000055baf6960518 in net_client_init (opts=0x55baf776e7e0,
> is_netdev=true, errp=0x7ffc51368f00) at net/net.c:1108
> #5 0x000055baf696083f in netdev_add (opts=0x55baf776e7e0,
> errp=0x7ffc51368f00) at net/net.c:1186
> #6 0x000055baf69608c7 in qmp_netdev_add (qdict=0x55baf7afaf60,
> ret=0x7ffc51368f50, errp=0x7ffc51368f48) at net/net.c:1205
> #7 0x000055baf6622135 in handle_qmp_command (parser=0x55baf77fb590,
> tokens=0x7f1d24011960) at /path/to/qemu.git/monitor.c:3978
> #8 0x000055baf6a9d099 in json_message_process_token
> (lexer=0x55baf77fb598, input=0x55baf75acd20, type=JSON_RCURLY, x=113, y=19)
> at qobject/json-streamer.c:105
> #9 0x000055baf6abf7aa in json_lexer_feed_char (lexer=0x55baf77fb598,
> ch=125 '}', flush=false) at qobject/json-lexer.c:319
> #10 0x000055baf6abf8f2 in json_lexer_feed (lexer=0x55baf77fb598,
> buffer=0x7ffc51369170 "}R\204\367\272U", size=1) at qobject/json-lexer.c:369
> #11 0x000055baf6a9d13c in json_message_parser_feed
> (parser=0x55baf77fb590, buffer=0x7ffc51369170 "}R\204\367\272U", size=1) at
> qobject/json-streamer.c:124
> #12 0x000055baf66221f7 in monitor_qmp_read (opaque=0x55baf77fb530,
> buf=0x7ffc51369170 "}R\204\367\272U", size=1) at
> /path/to/qemu.git/monitor.c:3994
> #13 0x000055baf6757014 in qemu_chr_be_write_impl (s=0x55baf7610a40,
> buf=0x7ffc51369170 "}R\204\367\272U", len=1) at qemu-char.c:387
> #14 0x000055baf6757076 in qemu_chr_be_write (s=0x55baf7610a40,
> buf=0x7ffc51369170 "}R\204\367\272U", len=1) at qemu-char.c:399
> #15 0x000055baf675b3b0 in tcp_chr_read (chan=0x55baf90244b0,
> cond=G_IO_IN, opaque=0x55baf7610a40) at qemu-char.c:2927
> #16 0x000055baf6a5d655 in qio_channel_fd_source_dispatch
> (source=0x55baf7610df0, callback=0x55baf675b25a <tcp_chr_read>,
> user_data=0x55baf7610a40) at io/channel-watch.c:84
> #17 0x00007f1d3e80cbbd in g_main_context_dispatch () from
> /usr/lib64/libglib-2.0.so.0
> #18 0x000055baf69d3720 in glib_pollfds_poll () at main-loop.c:213
> #19 0x000055baf69d37fd in os_host_main_loop_wait (timeout=126000000) at
> main-loop.c:258
> #20 0x000055baf69d38ad in main_loop_wait (nonblocking=0) at
> main-loop.c:506
> #21 0x000055baf676587b in main_loop () at vl.c:1908
> #22 0x000055baf676d3bf in main (argc=101, argv=0x7ffc5136a6c8,
> envp=0x7ffc5136a9f8) at vl.c:4604
> (gdb) p opts
> $1 = (QemuOpts *) 0x0
>
> The crash occurred when attaching vhost-user net via QMP:
>
> {
> "execute": "chardev-add",
> "arguments": {
> "id": "charnet2",
> "backend": {
> "type": "socket",
> "data": {
> "addr": {
> "type": "unix",
> "data": {
> "path": "/var/run/openvswitch/vhost-user1"
> }
> },
> "wait": false,
> "server": false
> }
> }
> },
> "id": "libvirt-19"
> }
> {
> "return": {
>
> },
> "id": "libvirt-19"
> }
> {
> "execute": "netdev_add",
> "arguments": {
> "type": "vhost-user",
> "chardev": "charnet2",
> "id": "hostnet2"
> },
> "id": "libvirt-20"
> }
>
> The vhost-user code should not be poking at the internals of the
> CharDriverState struct. What it wants is a chardev that is operating
> as a network server, so add an explicit API to allow it to validate
> this feature condition without looking at chardev internals.
>
It has to be a unix socket too, why did you drop that check?
>
> Signed-off-by: Daniel P. Berrange <berrange@redhat.com>
> ---
>
> NB, I've not functionally tested this - just compiled and
> unit tested.
>
> include/sysemu/char.h | 1 +
> net/vhost-user.c | 40 ++++------------------------------------
> qemu-char.c | 7 +++++++
> 3 files changed, 12 insertions(+), 36 deletions(-)
>
> diff --git a/include/sysemu/char.h b/include/sysemu/char.h
> index 0d0465a..50a6603 100644
> --- a/include/sysemu/char.h
> +++ b/include/sysemu/char.h
> @@ -437,6 +437,7 @@ int qemu_chr_add_client(CharDriverState *s, int fd);
> CharDriverState *qemu_chr_find(const char *name);
> bool chr_is_ringbuf(const CharDriverState *chr);
>
> +bool qemu_chr_is_network_server(CharDriverState *chr);
> QemuOpts *qemu_chr_parse_compat(const char *label, const char *filename);
>
> void register_char_driver(const char *name, ChardevBackendKind kind,
> diff --git a/net/vhost-user.c b/net/vhost-user.c
> index b0595f8..2e2cf24 100644
> --- a/net/vhost-user.c
> +++ b/net/vhost-user.c
> @@ -27,11 +27,6 @@ typedef struct VhostUserState {
> bool started;
> } VhostUserState;
>
> -typedef struct VhostUserChardevProps {
> - bool is_socket;
> - bool is_unix;
> -} VhostUserChardevProps;
> -
> VHostNetState *vhost_user_get_vhost_net(NetClientState *nc)
> {
> VhostUserState *s = DO_UPCAST(VhostUserState, nc, nc);
> @@ -278,45 +273,18 @@ static int net_vhost_user_init(NetClientState *peer,
> const char *device,
> return 0;
> }
>
> -static int net_vhost_chardev_opts(void *opaque,
> - const char *name, const char *value,
> - Error **errp)
> -{
> - VhostUserChardevProps *props = opaque;
> -
> - if (strcmp(name, "backend") == 0 && strcmp(value, "socket") == 0) {
> - props->is_socket = true;
> - } else if (strcmp(name, "path") == 0) {
> - props->is_unix = true;
> - } else if (strcmp(name, "server") == 0) {
> - } else {
> - error_setg(errp,
> - "vhost-user does not support a chardev with option
> %s=%s",
> - name, value);
> - return -1;
> - }
> - return 0;
> -}
> -
> -static CharDriverState *net_vhost_parse_chardev(
> +static CharDriverState *net_vhost_claim_chardev(
> const NetdevVhostUserOptions *opts, Error **errp)
> {
> CharDriverState *chr = qemu_chr_find(opts->chardev);
> - VhostUserChardevProps props;
>
> if (chr == NULL) {
> error_setg(errp, "chardev \"%s\" not found", opts->chardev);
> return NULL;
> }
>
> - /* inspect chardev opts */
> - memset(&props, 0, sizeof(props));
> - if (qemu_opt_foreach(chr->opts, net_vhost_chardev_opts, &props,
> errp)) {
> - return NULL;
> - }
> -
> - if (!props.is_socket || !props.is_unix) {
> - error_setg(errp, "chardev \"%s\" is not a unix socket",
> + if (!qemu_chr_is_network_server(chr)) {
> + error_setg(errp, "chardev \"%s\" does not provide a network
> server",
> opts->chardev);
> return NULL;
> }
> @@ -357,7 +325,7 @@ int net_init_vhost_user(const Netdev *netdev, const
> char *name,
> assert(netdev->type == NET_CLIENT_DRIVER_VHOST_USER);
> vhost_user_opts = &netdev->u.vhost_user;
>
> - chr = net_vhost_parse_chardev(vhost_user_opts, errp);
> + chr = net_vhost_claim_chardev(vhost_user_opts, errp);
> if (!chr) {
> return -1;
> }
> diff --git a/qemu-char.c b/qemu-char.c
> index fb456ce..977fb1a 100644
> --- a/qemu-char.c
> +++ b/qemu-char.c
> @@ -4596,6 +4596,13 @@ static CharDriverState *qmp_chardev_open_udp(const
> char *id,
> return qemu_chr_open_udp(sioc, common, errp);
> }
>
> +
> +bool qemu_chr_is_network_server(CharDriverState *chr)
> +{
> + return chr->chr_wait_connected != NULL;
> +}
> +
> +
> ChardevReturn *qmp_chardev_add(const char *id, ChardevBackend *backend,
> Error **errp)
> {
> --
> 2.7.4
>
> --
Marc-André Lureau
^ permalink raw reply [flat|nested] 4+ messages in thread
* Re: [Qemu-devel] [PATCH] vhost-user: don't poke at chardev internal QemuOpts
2016-10-07 9:10 ` Marc-André Lureau
@ 2016-10-07 9:15 ` Daniel P. Berrange
2016-10-07 9:49 ` Marc-André Lureau
0 siblings, 1 reply; 4+ messages in thread
From: Daniel P. Berrange @ 2016-10-07 9:15 UTC (permalink / raw)
To: Marc-André Lureau
Cc: qemu-devel, Michal Privoznik, Markus Armbruster, Paolo Bonzini,
Michael S. Tsirkin
On Fri, Oct 07, 2016 at 09:10:27AM +0000, Marc-André Lureau wrote:
> Hi
>
> On Fri, Oct 7, 2016 at 1:04 PM Daniel P. Berrange <berrange@redhat.com>
> wrote:
>
> > The vhost-user code is poking at the QemuOpts instance
> > in the CharDriverState struct, not realizing that it is
> > valid for this to be NULL. e.g. the following crash
> > shows a codepath where it will be NULL:
> >
> > Program terminated with signal SIGSEGV, Segmentation fault.
> > #0 0x000055baf6ab4adc in qemu_opt_foreach (opts=0x0, func=0x55baf696b650
> > <net_vhost_chardev_opts>, opaque=0x7ffc51368c00, errp=0x7ffc51368e48) at
> > util/qemu-option.c:617
> > 617 QTAILQ_FOREACH(opt, &opts->head, next) {
> > [Current thread is 1 (Thread 0x7f1d4970bb40 (LWP 6603))]
> > (gdb) bt
> > #0 0x000055baf6ab4adc in qemu_opt_foreach (opts=0x0, func=0x55baf696b650
> > <net_vhost_chardev_opts>, opaque=0x7ffc51368c00, errp=0x7ffc51368e48) at
> > util/qemu-option.c:617
> > #1 0x000055baf696b7da in net_vhost_parse_chardev (opts=0x55baf8ff9260,
> > errp=0x7ffc51368e48) at net/vhost-user.c:314
> > #2 0x000055baf696b985 in net_init_vhost_user (netdev=0x55baf8ff9250,
> > name=0x55baf879d270 "hostnet2", peer=0x0, errp=0x7ffc51368e48) at
> > net/vhost-user.c:360
> > #3 0x000055baf6960216 in net_client_init1 (object=0x55baf8ff9250,
> > is_netdev=true, errp=0x7ffc51368e48) at net/net.c:1051
> > #4 0x000055baf6960518 in net_client_init (opts=0x55baf776e7e0,
> > is_netdev=true, errp=0x7ffc51368f00) at net/net.c:1108
> > #5 0x000055baf696083f in netdev_add (opts=0x55baf776e7e0,
> > errp=0x7ffc51368f00) at net/net.c:1186
> > #6 0x000055baf69608c7 in qmp_netdev_add (qdict=0x55baf7afaf60,
> > ret=0x7ffc51368f50, errp=0x7ffc51368f48) at net/net.c:1205
> > #7 0x000055baf6622135 in handle_qmp_command (parser=0x55baf77fb590,
> > tokens=0x7f1d24011960) at /path/to/qemu.git/monitor.c:3978
> > #8 0x000055baf6a9d099 in json_message_process_token
> > (lexer=0x55baf77fb598, input=0x55baf75acd20, type=JSON_RCURLY, x=113, y=19)
> > at qobject/json-streamer.c:105
> > #9 0x000055baf6abf7aa in json_lexer_feed_char (lexer=0x55baf77fb598,
> > ch=125 '}', flush=false) at qobject/json-lexer.c:319
> > #10 0x000055baf6abf8f2 in json_lexer_feed (lexer=0x55baf77fb598,
> > buffer=0x7ffc51369170 "}R\204\367\272U", size=1) at qobject/json-lexer.c:369
> > #11 0x000055baf6a9d13c in json_message_parser_feed
> > (parser=0x55baf77fb590, buffer=0x7ffc51369170 "}R\204\367\272U", size=1) at
> > qobject/json-streamer.c:124
> > #12 0x000055baf66221f7 in monitor_qmp_read (opaque=0x55baf77fb530,
> > buf=0x7ffc51369170 "}R\204\367\272U", size=1) at
> > /path/to/qemu.git/monitor.c:3994
> > #13 0x000055baf6757014 in qemu_chr_be_write_impl (s=0x55baf7610a40,
> > buf=0x7ffc51369170 "}R\204\367\272U", len=1) at qemu-char.c:387
> > #14 0x000055baf6757076 in qemu_chr_be_write (s=0x55baf7610a40,
> > buf=0x7ffc51369170 "}R\204\367\272U", len=1) at qemu-char.c:399
> > #15 0x000055baf675b3b0 in tcp_chr_read (chan=0x55baf90244b0,
> > cond=G_IO_IN, opaque=0x55baf7610a40) at qemu-char.c:2927
> > #16 0x000055baf6a5d655 in qio_channel_fd_source_dispatch
> > (source=0x55baf7610df0, callback=0x55baf675b25a <tcp_chr_read>,
> > user_data=0x55baf7610a40) at io/channel-watch.c:84
> > #17 0x00007f1d3e80cbbd in g_main_context_dispatch () from
> > /usr/lib64/libglib-2.0.so.0
> > #18 0x000055baf69d3720 in glib_pollfds_poll () at main-loop.c:213
> > #19 0x000055baf69d37fd in os_host_main_loop_wait (timeout=126000000) at
> > main-loop.c:258
> > #20 0x000055baf69d38ad in main_loop_wait (nonblocking=0) at
> > main-loop.c:506
> > #21 0x000055baf676587b in main_loop () at vl.c:1908
> > #22 0x000055baf676d3bf in main (argc=101, argv=0x7ffc5136a6c8,
> > envp=0x7ffc5136a9f8) at vl.c:4604
> > (gdb) p opts
> > $1 = (QemuOpts *) 0x0
> >
> > The crash occurred when attaching vhost-user net via QMP:
> >
> > {
> > "execute": "chardev-add",
> > "arguments": {
> > "id": "charnet2",
> > "backend": {
> > "type": "socket",
> > "data": {
> > "addr": {
> > "type": "unix",
> > "data": {
> > "path": "/var/run/openvswitch/vhost-user1"
> > }
> > },
> > "wait": false,
> > "server": false
> > }
> > }
> > },
> > "id": "libvirt-19"
> > }
> > {
> > "return": {
> >
> > },
> > "id": "libvirt-19"
> > }
> > {
> > "execute": "netdev_add",
> > "arguments": {
> > "type": "vhost-user",
> > "chardev": "charnet2",
> > "id": "hostnet2"
> > },
> > "id": "libvirt-20"
> > }
> >
> > The vhost-user code should not be poking at the internals of the
> > CharDriverState struct. What it wants is a chardev that is operating
> > as a network server, so add an explicit API to allow it to validate
> > this feature condition without looking at chardev internals.
> >
>
> It has to be a unix socket too, why did you drop that check?
Oh does it ? I was reading the code as allowing a TCP or UNIX
server. What aspect of it requires UNIX sockets ?
Regards,
Daniel
--
|: http://berrange.com -o- http://www.flickr.com/photos/dberrange/ :|
|: http://libvirt.org -o- http://virt-manager.org :|
|: http://entangle-photo.org -o- http://search.cpan.org/~danberr/ :|
^ permalink raw reply [flat|nested] 4+ messages in thread
* Re: [Qemu-devel] [PATCH] vhost-user: don't poke at chardev internal QemuOpts
2016-10-07 9:15 ` Daniel P. Berrange
@ 2016-10-07 9:49 ` Marc-André Lureau
0 siblings, 0 replies; 4+ messages in thread
From: Marc-André Lureau @ 2016-10-07 9:49 UTC (permalink / raw)
To: Daniel P. Berrange
Cc: qemu-devel, Michal Privoznik, Markus Armbruster, Paolo Bonzini,
Michael S. Tsirkin
On Fri, Oct 7, 2016 at 1:15 PM Daniel P. Berrange <berrange@redhat.com>
wrote:
> On Fri, Oct 07, 2016 at 09:10:27AM +0000, Marc-André Lureau wrote:
> > Hi
> >
> > On Fri, Oct 7, 2016 at 1:04 PM Daniel P. Berrange <berrange@redhat.com>
> > wrote:
> >
> > > The vhost-user code is poking at the QemuOpts instance
> > > in the CharDriverState struct, not realizing that it is
> > > valid for this to be NULL. e.g. the following crash
> > > shows a codepath where it will be NULL:
> > >
> > > Program terminated with signal SIGSEGV, Segmentation fault.
> > > #0 0x000055baf6ab4adc in qemu_opt_foreach (opts=0x0,
> func=0x55baf696b650
> > > <net_vhost_chardev_opts>, opaque=0x7ffc51368c00, errp=0x7ffc51368e48)
> at
> > > util/qemu-option.c:617
> > > 617 QTAILQ_FOREACH(opt, &opts->head, next) {
> > > [Current thread is 1 (Thread 0x7f1d4970bb40 (LWP 6603))]
> > > (gdb) bt
> > > #0 0x000055baf6ab4adc in qemu_opt_foreach (opts=0x0,
> func=0x55baf696b650
> > > <net_vhost_chardev_opts>, opaque=0x7ffc51368c00, errp=0x7ffc51368e48)
> at
> > > util/qemu-option.c:617
> > > #1 0x000055baf696b7da in net_vhost_parse_chardev
> (opts=0x55baf8ff9260,
> > > errp=0x7ffc51368e48) at net/vhost-user.c:314
> > > #2 0x000055baf696b985 in net_init_vhost_user (netdev=0x55baf8ff9250,
> > > name=0x55baf879d270 "hostnet2", peer=0x0, errp=0x7ffc51368e48) at
> > > net/vhost-user.c:360
> > > #3 0x000055baf6960216 in net_client_init1 (object=0x55baf8ff9250,
> > > is_netdev=true, errp=0x7ffc51368e48) at net/net.c:1051
> > > #4 0x000055baf6960518 in net_client_init (opts=0x55baf776e7e0,
> > > is_netdev=true, errp=0x7ffc51368f00) at net/net.c:1108
> > > #5 0x000055baf696083f in netdev_add (opts=0x55baf776e7e0,
> > > errp=0x7ffc51368f00) at net/net.c:1186
> > > #6 0x000055baf69608c7 in qmp_netdev_add (qdict=0x55baf7afaf60,
> > > ret=0x7ffc51368f50, errp=0x7ffc51368f48) at net/net.c:1205
> > > #7 0x000055baf6622135 in handle_qmp_command (parser=0x55baf77fb590,
> > > tokens=0x7f1d24011960) at /path/to/qemu.git/monitor.c:3978
> > > #8 0x000055baf6a9d099 in json_message_process_token
> > > (lexer=0x55baf77fb598, input=0x55baf75acd20, type=JSON_RCURLY, x=113,
> y=19)
> > > at qobject/json-streamer.c:105
> > > #9 0x000055baf6abf7aa in json_lexer_feed_char (lexer=0x55baf77fb598,
> > > ch=125 '}', flush=false) at qobject/json-lexer.c:319
> > > #10 0x000055baf6abf8f2 in json_lexer_feed (lexer=0x55baf77fb598,
> > > buffer=0x7ffc51369170 "}R\204\367\272U", size=1) at
> qobject/json-lexer.c:369
> > > #11 0x000055baf6a9d13c in json_message_parser_feed
> > > (parser=0x55baf77fb590, buffer=0x7ffc51369170 "}R\204\367\272U",
> size=1) at
> > > qobject/json-streamer.c:124
> > > #12 0x000055baf66221f7 in monitor_qmp_read (opaque=0x55baf77fb530,
> > > buf=0x7ffc51369170 "}R\204\367\272U", size=1) at
> > > /path/to/qemu.git/monitor.c:3994
> > > #13 0x000055baf6757014 in qemu_chr_be_write_impl (s=0x55baf7610a40,
> > > buf=0x7ffc51369170 "}R\204\367\272U", len=1) at qemu-char.c:387
> > > #14 0x000055baf6757076 in qemu_chr_be_write (s=0x55baf7610a40,
> > > buf=0x7ffc51369170 "}R\204\367\272U", len=1) at qemu-char.c:399
> > > #15 0x000055baf675b3b0 in tcp_chr_read (chan=0x55baf90244b0,
> > > cond=G_IO_IN, opaque=0x55baf7610a40) at qemu-char.c:2927
> > > #16 0x000055baf6a5d655 in qio_channel_fd_source_dispatch
> > > (source=0x55baf7610df0, callback=0x55baf675b25a <tcp_chr_read>,
> > > user_data=0x55baf7610a40) at io/channel-watch.c:84
> > > #17 0x00007f1d3e80cbbd in g_main_context_dispatch () from
> > > /usr/lib64/libglib-2.0.so.0
> > > #18 0x000055baf69d3720 in glib_pollfds_poll () at main-loop.c:213
> > > #19 0x000055baf69d37fd in os_host_main_loop_wait (timeout=126000000)
> at
> > > main-loop.c:258
> > > #20 0x000055baf69d38ad in main_loop_wait (nonblocking=0) at
> > > main-loop.c:506
> > > #21 0x000055baf676587b in main_loop () at vl.c:1908
> > > #22 0x000055baf676d3bf in main (argc=101, argv=0x7ffc5136a6c8,
> > > envp=0x7ffc5136a9f8) at vl.c:4604
> > > (gdb) p opts
> > > $1 = (QemuOpts *) 0x0
> > >
> > > The crash occurred when attaching vhost-user net via QMP:
> > >
> > > {
> > > "execute": "chardev-add",
> > > "arguments": {
> > > "id": "charnet2",
> > > "backend": {
> > > "type": "socket",
> > > "data": {
> > > "addr": {
> > > "type": "unix",
> > > "data": {
> > > "path": "/var/run/openvswitch/vhost-user1"
> > > }
> > > },
> > > "wait": false,
> > > "server": false
> > > }
> > > }
> > > },
> > > "id": "libvirt-19"
> > > }
> > > {
> > > "return": {
> > >
> > > },
> > > "id": "libvirt-19"
> > > }
> > > {
> > > "execute": "netdev_add",
> > > "arguments": {
> > > "type": "vhost-user",
> > > "chardev": "charnet2",
> > > "id": "hostnet2"
> > > },
> > > "id": "libvirt-20"
> > > }
> > >
> > > The vhost-user code should not be poking at the internals of the
> > > CharDriverState struct. What it wants is a chardev that is operating
> > > as a network server, so add an explicit API to allow it to validate
> > > this feature condition without looking at chardev internals.
> > >
> >
> > It has to be a unix socket too, why did you drop that check?
>
> Oh does it ? I was reading the code as allowing a TCP or UNIX
> server. What aspect of it requires UNIX sockets ?
>
>
docs/specs/vhost-user.txt
"It implements the control plane needed
to establish virtqueue sharing with a user space process on the same host.
It
uses communication over a Unix domain socket to share file descriptors in
the
ancillary data of the message."
> Regards,
> Daniel
> --
> |: http://berrange.com -o- http://www.flickr.com/photos/dberrange/
> :|
> |: http://libvirt.org -o- http://virt-manager.org
> :|
> |: http://entangle-photo.org -o- http://search.cpan.org/~danberr/
> :|
>
--
Marc-André Lureau
^ permalink raw reply [flat|nested] 4+ messages in thread
end of thread, other threads:[~2016-10-07 9:49 UTC | newest]
Thread overview: 4+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2016-10-07 9:04 [Qemu-devel] [PATCH] vhost-user: don't poke at chardev internal QemuOpts Daniel P. Berrange
2016-10-07 9:10 ` Marc-André Lureau
2016-10-07 9:15 ` Daniel P. Berrange
2016-10-07 9:49 ` Marc-André Lureau
This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.