From mboxrd@z Thu Jan 1 00:00:00 1970 Received: from eggs.gnu.org ([2001:4830:134:3::10]:45189) by lists.gnu.org with esmtp (Exim 4.71) (envelope-from ) id 1Z4UdE-0007YT-Ht for qemu-devel@nongnu.org; Mon, 15 Jun 2015 09:40:57 -0400 Received: from Debian-exim by eggs.gnu.org with spam-scanned (Exim 4.71) (envelope-from ) id 1Z4UdB-0006yy-FP for qemu-devel@nongnu.org; Mon, 15 Jun 2015 09:40:56 -0400 Received: from mx1.redhat.com ([209.132.183.28]:47210) by eggs.gnu.org with esmtp (Exim 4.71) (envelope-from ) id 1Z4UdA-0006w3-Q3 for qemu-devel@nongnu.org; Mon, 15 Jun 2015 09:40:53 -0400 Date: Mon, 15 Jun 2015 14:40:22 +0100 From: Stefan Hajnoczi Message-ID: <20150615134022.GB9410@stefanha-thinkpad.redhat.com> References: <1432538908-26298-5-git-send-email-mukawa@igel.co.jp> <1432874550-10921-1-git-send-email-mukawa@igel.co.jp> <1432874550-10921-3-git-send-email-mukawa@igel.co.jp> MIME-Version: 1.0 Content-Type: multipart/signed; micalg=pgp-sha1; protocol="application/pgp-signature"; boundary="bCsyhTFzCvuiizWE" Content-Disposition: inline In-Reply-To: <1432874550-10921-3-git-send-email-mukawa@igel.co.jp> Subject: Re: [Qemu-devel] [PATCH v1 2/4] vhost-user: Shutdown vhost-user connection when wrong messages are passed List-Id: List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , To: Tetsuya Mukawa Cc: jasowang@redhat.com, qemu-devel@nongnu.org, n.nikolaev@virtualopensystems.com --bCsyhTFzCvuiizWE Content-Type: text/plain; charset=us-ascii Content-Disposition: inline Content-Transfer-Encoding: quoted-printable On Fri, May 29, 2015 at 01:42:28PM +0900, Tetsuya Mukawa wrote: > When wrong vhost-user message are passed, the connection should be > shutdown. >=20 > Signed-off-by: Tetsuya Mukawa > --- > hw/virtio/vhost-user.c | 17 ++++++++++------- > include/sysemu/char.h | 7 +++++++ > qemu-char.c | 15 +++++++++++++++ > 3 files changed, 32 insertions(+), 7 deletions(-) >=20 > diff --git a/hw/virtio/vhost-user.c b/hw/virtio/vhost-user.c > index e7ab829..4d7e3ba 100644 > --- a/hw/virtio/vhost-user.c > +++ b/hw/virtio/vhost-user.c > @@ -183,6 +183,7 @@ static int vhost_user_write(struct vhost_dev *dev, Vh= ostUserMsg *msg, > static int vhost_user_call(struct vhost_dev *dev, unsigned long int requ= est, > void *arg) > { > + CharDriverState *chr =3D dev->opaque; > VhostUserMsg msg; > VhostUserRequest msg_request; > struct vhost_vring_file *file =3D 0; > @@ -237,7 +238,7 @@ static int vhost_user_call(struct vhost_dev *dev, uns= igned long int request, > if (!fd_num) { > error_report("Failed initializing vhost-user memory map, " > "consider using -object memory-backend-file share=3D= on"); > - return -1; > + goto close; > } > =20 > msg.size =3D sizeof(m.memory.nregions); > @@ -281,7 +282,7 @@ static int vhost_user_call(struct vhost_dev *dev, uns= igned long int request, > break; > default: > error_report("vhost-user trying to send unhandled ioctl"); > - return -1; > + goto close; > break; > } > =20 > @@ -297,32 +298,34 @@ static int vhost_user_call(struct vhost_dev *dev, u= nsigned long int request, > if (msg_request !=3D msg.request) { > error_report("Received unexpected msg type." > " Expected %d received %d", msg_request, msg.request= ); > - return -1; > + goto close; > } > =20 > switch (msg_request) { > case VHOST_USER_GET_FEATURES: > if (msg.size !=3D sizeof(m.u64)) { > error_report("Received bad msg size."); > - return -1; > + goto close; > } > *((__u64 *) arg) =3D msg.u64; > break; > case VHOST_USER_GET_VRING_BASE: > if (msg.size !=3D sizeof(m.state)) { > error_report("Received bad msg size."); > - return -1; > + goto close; > } > memcpy(arg, &msg.state, sizeof(struct vhost_vring_state)); > break; > default: > error_report("Received unexpected msg type."); > - return -1; > - break; > } > } > =20 > return 0; > + > +close: > + qemu_chr_shutdown(chr, SHUT_RDWR); > + return -1; > } Why is shutdown(2) necessary? We're aborting the connection and don't expect to process any more data, so we could just close it. > diff --git a/include/sysemu/char.h b/include/sysemu/char.h > index 832b7fe..d944ded 100644 > --- a/include/sysemu/char.h > +++ b/include/sysemu/char.h > @@ -70,6 +70,7 @@ struct CharDriverState { > IOReadHandler *chr_read; > void *handler_opaque; > void (*chr_close)(struct CharDriverState *chr); > + void (*chr_shutdown)(struct CharDriverState *chr, int how); > void (*chr_accept_input)(struct CharDriverState *chr); > void (*chr_set_echo)(struct CharDriverState *chr, bool echo); > void (*chr_set_fe_open)(struct CharDriverState *chr, int fe_open); > @@ -124,6 +125,12 @@ CharDriverState *qemu_chr_new_from_opts(QemuOpts *op= ts, > */ > CharDriverState *qemu_chr_new(const char *label, const char *filename, > void (*init)(struct CharDriverState *s)); > +/** > + * @qemu_chr_shutdown: > + * > + * Shutdown a fd of character backend. > + */ > +void qemu_chr_shutdown(CharDriverState *chr, int how); > =20 > /** > * @qemu_chr_delete: > diff --git a/qemu-char.c b/qemu-char.c > index d0c1564..2b28808 100644 > --- a/qemu-char.c > +++ b/qemu-char.c > @@ -2839,6 +2839,13 @@ static void tcp_chr_disconnect(CharDriverState *ch= r) > } > } > =20 > +static void tcp_chr_shutdown(CharDriverState *chr, int how) > +{ > + TCPCharDriver *s =3D chr->opaque; > + > + shutdown(s->fd, how); > +} > + > static gboolean tcp_chr_read(GIOChannel *chan, GIOCondition cond, void *= opaque) > { > CharDriverState *chr =3D opaque; > @@ -3836,6 +3843,13 @@ void qemu_chr_fe_release(CharDriverState *s) > s->avail_connections++; > } > =20 > +void qemu_chr_shutdown(CharDriverState *chr, int how) > +{ > + if (chr->chr_shutdown) { > + chr->chr_shutdown(chr, how); > + } > +} > + > void qemu_chr_delete(CharDriverState *chr) > { > QTAILQ_REMOVE(&chardevs, chr, next); > @@ -4154,6 +4168,7 @@ static CharDriverState *qmp_chardev_open_socket(Cha= rdevSocket *sock, > chr->chr_write =3D tcp_chr_write; > chr->chr_sync_read =3D tcp_chr_sync_read; > chr->chr_close =3D tcp_chr_close; > + chr->chr_shutdown =3D tcp_chr_shutdown; > chr->get_msgfds =3D tcp_get_msgfds; > chr->set_msgfds =3D tcp_set_msgfds; > chr->chr_add_client =3D tcp_chr_add_client; Please split this into a separate qemu-char.c patch. I hesitate to add a shutdown(2) interface since it adds a new state that the rest of the qemu-char.c code doesn't know about. --bCsyhTFzCvuiizWE Content-Type: application/pgp-signature -----BEGIN PGP SIGNATURE----- Version: GnuPG v1 iQEcBAEBAgAGBQJVftXGAAoJEJykq7OBq3PIfeEH/0UXUoY9jyBBm0YqSBOmVeR/ RLOyOTYmCnVSMldDCJbLkXEKm8rcu6XdvnagItAykHklKBHBkDXWLyvg+Kbe/09k CJO2AK0p11MNdrCNPPu5C3jJbZW8xRVHs7uKjA64qi19Sb0UPKQHxQb0KoG+NMDi cXan9oblodZ39IOS2dFHlf5GSU4K0/GPsEQMB80HaFdUNow5EG+3xzRxChuJGE2k W8QSox/bgJ6W8A970jvjkfqO9H/ns4lLqq2O7GLVw6kx9MWBp+qG1FCorlhZB64u ghjQrjdxga3suwgPLOQvfbf1dvgObvFS0HaM+RvFHwLq2QIdkm4mCXGoenTM0jw= =xMA5 -----END PGP SIGNATURE----- --bCsyhTFzCvuiizWE--