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. > > Signed-off-by: Tetsuya Mukawa <[email protected]> > --- > hw/virtio/vhost-user.c | 17 ++++++++++------- > include/sysemu/char.h | 7 +++++++ > qemu-char.c | 15 +++++++++++++++ > 3 files changed, 32 insertions(+), 7 deletions(-) > > 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, > VhostUserMsg *msg, > static int vhost_user_call(struct vhost_dev *dev, unsigned long int request, > void *arg) > { > + CharDriverState *chr = dev->opaque; > VhostUserMsg msg; > VhostUserRequest msg_request; > struct vhost_vring_file *file = 0; > @@ -237,7 +238,7 @@ static int vhost_user_call(struct vhost_dev *dev, > unsigned long int request, > if (!fd_num) { > error_report("Failed initializing vhost-user memory map, " > "consider using -object memory-backend-file share=on"); > - return -1; > + goto close; > } > > msg.size = sizeof(m.memory.nregions); > @@ -281,7 +282,7 @@ static int vhost_user_call(struct vhost_dev *dev, > unsigned long int request, > break; > default: > error_report("vhost-user trying to send unhandled ioctl"); > - return -1; > + goto close; > break; > } > > @@ -297,32 +298,34 @@ static int vhost_user_call(struct vhost_dev *dev, > unsigned long int request, > if (msg_request != msg.request) { > error_report("Received unexpected msg type." > " Expected %d received %d", msg_request, msg.request); > - return -1; > + goto close; > } > > switch (msg_request) { > case VHOST_USER_GET_FEATURES: > if (msg.size != sizeof(m.u64)) { > error_report("Received bad msg size."); > - return -1; > + goto close; > } > *((__u64 *) arg) = msg.u64; > break; > case VHOST_USER_GET_VRING_BASE: > if (msg.size != 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; > } > } > > 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 *opts,
> */
> 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);
>
> /**
> * @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 *chr)
> }
> }
>
> +static void tcp_chr_shutdown(CharDriverState *chr, int how)
> +{
> + TCPCharDriver *s = chr->opaque;
> +
> + shutdown(s->fd, how);
> +}
> +
> static gboolean tcp_chr_read(GIOChannel *chan, GIOCondition cond, void
> *opaque)
> {
> CharDriverState *chr = opaque;
> @@ -3836,6 +3843,13 @@ void qemu_chr_fe_release(CharDriverState *s)
> s->avail_connections++;
> }
>
> +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(ChardevSocket *sock,
> chr->chr_write = tcp_chr_write;
> chr->chr_sync_read = tcp_chr_sync_read;
> chr->chr_close = tcp_chr_close;
> + chr->chr_shutdown = tcp_chr_shutdown;
> chr->get_msgfds = tcp_get_msgfds;
> chr->set_msgfds = tcp_set_msgfds;
> chr->chr_add_client = 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.
pgpqJph5DS1rc.pgp
Description: PGP signature
