Re: [Qemu-devel] [PATCH 25/38] ivshmem: Rely on server sending the ID right after the version

[Marc-André Lureau]

On Mon, Feb 29, 2016 at 7:40 PM, Markus Armbruster <address@hidden> wrote:
> The protocol specification (ivshmem-spec.txt, formerly
> ivshmem_device_spec.txt) has always required the ID message to be sent
> right at the beginning, and ivshmem-server has always complied.  The
> device, however, accepts it out of order.  If an interrupt setup
> arrived before it, though, it would be misinterpreted as connect
> notification.  Fix the latent bug by relying on the spec and
> ivshmem-server's actual behavior.
>
> Signed-off-by: Markus Armbruster <address@hidden>
> ---

Reviewed-by: Marc-André Lureau <address@hidden>


>  hw/misc/ivshmem.c | 27 ++++++++++++++++++++++++---
>  1 file changed, 24 insertions(+), 3 deletions(-)
>
> diff --git a/hw/misc/ivshmem.c b/hw/misc/ivshmem.c
> index 831da53..8f976ca 100644
> --- a/hw/misc/ivshmem.c
> +++ b/hw/misc/ivshmem.c
> @@ -653,8 +653,6 @@ static void process_msg(IVShmemState *s, int64_t msg, int 
> fd, Error **errp)
>
>      if (fd >= 0) {
>          process_msg_connect(s, msg, fd, errp);
> -    } else if (s->vm_id == -1) {
> -        s->vm_id = msg;
>      } else {
>          process_msg_disconnect(s, msg, errp);
>      }
> @@ -723,6 +721,30 @@ static void ivshmem_recv_setup(IVShmemState *s, Error 
> **errp)
>      }
>
>      /*
> +     * ivshmem-server sends the remaining initial messages in a fixed
> +     * order, but the device has always accepted them in any order.
> +     * Stay as compatible as practical, just in case people use
> +     * servers that behave differently.
> +     */
> +
> +    /*
> +     * ivshmem_device_spec.txt has always required the ID message
> +     * right here, and ivshmem-server has always complied.  However,
> +     * older versions of the device accepted it out of order, but
> +     * broke when an interrupt setup message arrived before it.
> +     */
> +    msg = ivshmem_recv_msg(s, &fd, &err);
> +    if (err) {
> +        error_propagate(errp, err);
> +        return;
> +    }
> +    if (fd != -1 || msg < 0 || msg > IVSHMEM_MAX_PEERS) {
> +        error_setg(errp, "server sent invalid ID message");
> +        return;
> +    }
> +    s->vm_id = msg;
> +
> +    /*
>       * Receive more messages until we got shared memory.
>       */
>      do {
> @@ -953,7 +975,6 @@ static void pci_ivshmem_realize(PCIDevice *dev, Error 
> **errp)
>
>          /* we allocate enough space for 16 peers and grow as needed */
>          resize_peers(s, 16);
> -        s->vm_id = -1;
>
>          pci_register_bar(dev, 2, attr, &s->bar);
>
> --
> 2.4.3
>
>



-- 
Marc-André Lureau