From mboxrd@z Thu Jan 1 00:00:00 1970 Received: from eggs.gnu.org ([208.118.235.92]:35519) by lists.gnu.org with esmtp (Exim 4.71) (envelope-from ) id 1TDg0H-0004ZA-RK for qemu-devel@nongnu.org; Mon, 17 Sep 2012 14:25:06 -0400 Received: from Debian-exim by eggs.gnu.org with spam-scanned (Exim 4.71) (envelope-from ) id 1TDg0C-0002RH-OU for qemu-devel@nongnu.org; Mon, 17 Sep 2012 14:25:05 -0400 Received: from e9.ny.us.ibm.com ([32.97.182.139]:48150) by eggs.gnu.org with esmtp (Exim 4.71) (envelope-from ) id 1TDg0C-0002Qy-KH for qemu-devel@nongnu.org; Mon, 17 Sep 2012 14:25:00 -0400 Received: from /spool/local by e9.ny.us.ibm.com with IBM ESMTP SMTP Gateway: Authorized Use Only! Violators will be prosecuted for from ; Mon, 17 Sep 2012 14:25:00 -0400 Received: from d01av02.pok.ibm.com (d01av02.pok.ibm.com [9.56.224.216]) by d01relay03.pok.ibm.com (8.13.8/8.13.8/NCO v10.0) with ESMTP id q8HIOu52124108 for ; Mon, 17 Sep 2012 14:24:56 -0400 Received: from d01av02.pok.ibm.com (loopback [127.0.0.1]) by d01av02.pok.ibm.com (8.14.4/8.13.1/NCO v10.0 AVout) with ESMTP id q8HIOtKQ015814 for ; Mon, 17 Sep 2012 15:24:55 -0300 From: Anthony Liguori In-Reply-To: <1347244257-15586-2-git-send-email-david@gibson.dropbear.id.au> References: <1347244257-15586-1-git-send-email-david@gibson.dropbear.id.au> <1347244257-15586-2-git-send-email-david@gibson.dropbear.id.au> Date: Mon, 17 Sep 2012 13:24:51 -0500 Message-ID: <87txuw4ido.fsf@codemonkey.ws> MIME-Version: 1.0 Content-Type: text/plain; charset=us-ascii Subject: Re: [Qemu-devel] [PATCH 1/2] qemu-char: BUGFIX, don't call FD_ISSET with negative fd List-Id: List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , To: David Gibson Cc: qemu-devel@nongnu.org David Gibson writes: > tcp_chr_connect(), unlike for example udp_chr_update_read_handler() does > not check if the fd it is using is valid (>= 0) before passing it to > qemu_set_fd_handler2(). If using e.g. a TCP serial port, which is not > initially connected, this can result in -1 being passed to FD_ISSET, which > has undefined behaviour. On x86 it seems to harmlessly return 0, but on > PowerPC, it causes a fortify buffer overflow error to be thrown. > > This patch fixes this by putting an extra test in tcp_chr_connect(), and > also adds an assert qemu_set_fd_handler2() to catch other such errors on > all platforms, rather than just some. > > Signed-off-by: David Gibson Applied. Thanks. Regards, Anthony Liguori > --- > iohandler.c | 2 ++ > qemu-char.c | 6 ++++-- > 2 files changed, 6 insertions(+), 2 deletions(-) > > diff --git a/iohandler.c b/iohandler.c > index dea4355..a2d871b 100644 > --- a/iohandler.c > +++ b/iohandler.c > @@ -56,6 +56,8 @@ int qemu_set_fd_handler2(int fd, > { > IOHandlerRecord *ioh; > > + assert(fd >= 0); > + > if (!fd_read && !fd_write) { > QLIST_FOREACH(ioh, &io_handlers, next) { > if (ioh->fd == fd) { > diff --git a/qemu-char.c b/qemu-char.c > index 398baf1..73e48ff 100644 > --- a/qemu-char.c > +++ b/qemu-char.c > @@ -2332,8 +2332,10 @@ static void tcp_chr_connect(void *opaque) > TCPCharDriver *s = chr->opaque; > > s->connected = 1; > - qemu_set_fd_handler2(s->fd, tcp_chr_read_poll, > - tcp_chr_read, NULL, chr); > + if (s->fd >= 0) { > + qemu_set_fd_handler2(s->fd, tcp_chr_read_poll, > + tcp_chr_read, NULL, chr); > + } > qemu_chr_generic_open(chr); > } > > -- > 1.7.10.4