From mboxrd@z Thu Jan 1 00:00:00 1970 Received: from eggs.gnu.org ([2001:4830:134:3::10]:55700) by lists.gnu.org with esmtp (Exim 4.71) (envelope-from ) id 1aRKcR-0006ow-Vl for qemu-devel@nongnu.org; Thu, 04 Feb 2016 09:10:48 -0500 Received: from Debian-exim by eggs.gnu.org with spam-scanned (Exim 4.71) (envelope-from ) id 1aRKcM-0007f1-QZ for qemu-devel@nongnu.org; Thu, 04 Feb 2016 09:10:47 -0500 Received: from mx1.redhat.com ([209.132.183.28]:60392) by eggs.gnu.org with esmtp (Exim 4.71) (envelope-from ) id 1aRKcM-0007eP-K2 for qemu-devel@nongnu.org; Thu, 04 Feb 2016 09:10:42 -0500 Date: Thu, 4 Feb 2016 16:10:38 +0200 From: "Michael S. Tsirkin" Message-ID: <20160204154232-mutt-send-email-mst@redhat.com> References: <1449136399-4158-1-git-send-email-didier.pallard@6wind.com> <1449136399-4158-2-git-send-email-didier.pallard@6wind.com> MIME-Version: 1.0 Content-Type: text/plain; charset=us-ascii Content-Disposition: inline In-Reply-To: <1449136399-4158-2-git-send-email-didier.pallard@6wind.com> Subject: Re: [Qemu-devel] [PATCH 1/3] char: fix vhost-user socket full List-Id: List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , To: Didier Pallard Cc: thibaut.collet@6wind.com, jmg@6wind.com, qemu-devel@nongnu.org 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 > Reviewed-by: Thibaut Collet > --- > 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 >