From mboxrd@z Thu Jan 1 00:00:00 1970 Received: from eggs.gnu.org ([2001:4830:134:3::10]:47006) by lists.gnu.org with esmtp (Exim 4.71) (envelope-from ) id 1bsRFk-0005iC-5e for qemu-devel@nongnu.org; Fri, 07 Oct 2016 05:15:46 -0400 Received: from Debian-exim by eggs.gnu.org with spam-scanned (Exim 4.71) (envelope-from ) id 1bsRFg-0007ce-OH for qemu-devel@nongnu.org; Fri, 07 Oct 2016 05:15:39 -0400 Received: from mx1.redhat.com ([209.132.183.28]:45808) by eggs.gnu.org with esmtp (Exim 4.71) (envelope-from ) id 1bsRFg-0007cQ-7H for qemu-devel@nongnu.org; Fri, 07 Oct 2016 05:15:36 -0400 Date: Fri, 7 Oct 2016 10:15:31 +0100 From: "Daniel P. Berrange" Message-ID: <20161007091531.GE26332@redhat.com> Reply-To: "Daniel P. Berrange" References: <1475831087-4697-1-git-send-email-berrange@redhat.com> MIME-Version: 1.0 Content-Type: text/plain; charset=utf-8 Content-Disposition: inline In-Reply-To: Content-Transfer-Encoding: quoted-printable Subject: Re: [Qemu-devel] [PATCH] vhost-user: don't poke at chardev internal QemuOpts List-Id: List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , To: =?utf-8?Q?Marc-Andr=C3=A9?= Lureau Cc: qemu-devel@nongnu.org, Michal Privoznik , Markus Armbruster , Paolo Bonzini , "Michael S. Tsirkin" On Fri, Oct 07, 2016 at 09:10:27AM +0000, Marc-Andr=C3=A9 Lureau wrote: > Hi >=20 > On Fri, Oct 7, 2016 at 1:04 PM Daniel P. Berrange > wrote: >=20 > > 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=3D0x0, func=3D0x55b= af696b650 > > , opaque=3D0x7ffc51368c00, errp=3D0x7ffc51368= e48) 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=3D0x0, func=3D0x55b= af696b650 > > , opaque=3D0x7ffc51368c00, errp=3D0x7ffc51368= e48) at > > util/qemu-option.c:617 > > #1 0x000055baf696b7da in net_vhost_parse_chardev (opts=3D0x55baf8ff= 9260, > > errp=3D0x7ffc51368e48) at net/vhost-user.c:314 > > #2 0x000055baf696b985 in net_init_vhost_user (netdev=3D0x55baf8ff92= 50, > > name=3D0x55baf879d270 "hostnet2", peer=3D0x0, errp=3D0x7ffc51368e48) = at > > net/vhost-user.c:360 > > #3 0x000055baf6960216 in net_client_init1 (object=3D0x55baf8ff9250, > > is_netdev=3Dtrue, errp=3D0x7ffc51368e48) at net/net.c:1051 > > #4 0x000055baf6960518 in net_client_init (opts=3D0x55baf776e7e0, > > is_netdev=3Dtrue, errp=3D0x7ffc51368f00) at net/net.c:1108 > > #5 0x000055baf696083f in netdev_add (opts=3D0x55baf776e7e0, > > errp=3D0x7ffc51368f00) at net/net.c:1186 > > #6 0x000055baf69608c7 in qmp_netdev_add (qdict=3D0x55baf7afaf60, > > ret=3D0x7ffc51368f50, errp=3D0x7ffc51368f48) at net/net.c:1205 > > #7 0x000055baf6622135 in handle_qmp_command (parser=3D0x55baf77fb59= 0, > > tokens=3D0x7f1d24011960) at /path/to/qemu.git/monitor.c:3978 > > #8 0x000055baf6a9d099 in json_message_process_token > > (lexer=3D0x55baf77fb598, input=3D0x55baf75acd20, type=3DJSON_RCURLY, = x=3D113, y=3D19) > > at qobject/json-streamer.c:105 > > #9 0x000055baf6abf7aa in json_lexer_feed_char (lexer=3D0x55baf77fb5= 98, > > ch=3D125 '}', flush=3Dfalse) at qobject/json-lexer.c:319 > > #10 0x000055baf6abf8f2 in json_lexer_feed (lexer=3D0x55baf77fb598, > > buffer=3D0x7ffc51369170 "}R\204\367\272U", size=3D1) at qobject/json-= lexer.c:369 > > #11 0x000055baf6a9d13c in json_message_parser_feed > > (parser=3D0x55baf77fb590, buffer=3D0x7ffc51369170 "}R\204\367\272U", = size=3D1) at > > qobject/json-streamer.c:124 > > #12 0x000055baf66221f7 in monitor_qmp_read (opaque=3D0x55baf77fb530, > > buf=3D0x7ffc51369170 "}R\204\367\272U", size=3D1) at > > /path/to/qemu.git/monitor.c:3994 > > #13 0x000055baf6757014 in qemu_chr_be_write_impl (s=3D0x55baf7610a40= , > > buf=3D0x7ffc51369170 "}R\204\367\272U", len=3D1) at qemu-char.c:387 > > #14 0x000055baf6757076 in qemu_chr_be_write (s=3D0x55baf7610a40, > > buf=3D0x7ffc51369170 "}R\204\367\272U", len=3D1) at qemu-char.c:399 > > #15 0x000055baf675b3b0 in tcp_chr_read (chan=3D0x55baf90244b0, > > cond=3DG_IO_IN, opaque=3D0x55baf7610a40) at qemu-char.c:2927 > > #16 0x000055baf6a5d655 in qio_channel_fd_source_dispatch > > (source=3D0x55baf7610df0, callback=3D0x55baf675b25a , > > user_data=3D0x55baf7610a40) 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=3D12600000= 0) at > > main-loop.c:258 > > #20 0x000055baf69d38ad in main_loop_wait (nonblocking=3D0) at > > main-loop.c:506 > > #21 0x000055baf676587b in main_loop () at vl.c:1908 > > #22 0x000055baf676d3bf in main (argc=3D101, argv=3D0x7ffc5136a6c8, > > envp=3D0x7ffc5136a9f8) at vl.c:4604 > > (gdb) p opts > > $1 =3D (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. > > >=20 > 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 --=20 |: http://berrange.com -o- http://www.flickr.com/photos/dberrange= / :| |: http://libvirt.org -o- http://virt-manager.or= g :| |: http://entangle-photo.org -o- http://search.cpan.org/~danberr= / :|