Re: [Qemu-devel] [PATCH 1/3] char: fix vhost-user socket full

["Michael S. Tsirkin" <mst@redhat.com>]

On Thu, Dec 03, 2015 at 10:53:17AM +0100, Didier Pallard wrote:
> unix_send_msgfds is used by vhost-user control socket. qemu_chr_fe_write_all
> is used to send a message and retries as long as EAGAIN errno is set,
> but write_msgfds buffer is freed after first EAGAIN failure, causing
> message to be sent without proper fds attachment.
> 
> In case unix_send_msgfds is called through qemu_chr_fe_write, it will be
> user responsability to resend message as is or to free write_msgfds
> using set_msgfds(0)
> 
> Signed-off-by: Didier Pallard <didier.pallard@6wind.com>
> Reviewed-by: Thibaut Collet <thibaut.collet@6wind.com>
> ---
>  qemu-char.c | 10 ++++++++++
>  1 file changed, 10 insertions(+)
> 
> diff --git a/qemu-char.c b/qemu-char.c
> index 5448b0f..26d5f2e 100644
> --- a/qemu-char.c
> +++ b/qemu-char.c
> @@ -2614,6 +2614,16 @@ static int unix_send_msgfds(CharDriverState *chr, const uint8_t *buf, int len)
>          r = sendmsg(s->fd, &msgh, 0);
>      } while (r < 0 && errno == EINTR);
>  
> +    /* Ancillary data are not sent if no byte is written
> +     * so don't free msgfds buffer if return value is EAGAIN
> +     * If called from qemu_chr_fe_write_all retry will come soon
> +     * If called from qemu_chr_fe_write, it is the user responsibility
> +     * to resend message or free fds using set_msgfds(0)

Problem is, if ever anyone tries to send messages
with qemu_chr_fe_write and does not retry, there will
be a memory leak.

Rather than this, how about adding an assert in qemu_chr_fe_write
to make sure its not used with msgfds?


> +     */
> +    if (r < 0 && errno == EAGAIN) {
> +        return r;
> +    }
> +
>      /* free the written msgfds, no matter what */
>      if (s->write_msgfds_num) {
>          g_free(s->write_msgfds);
> -- 
> 2.1.4
>