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 <muk...@igel.co.jp>
> ---
> 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
