From mboxrd@z Thu Jan 1 00:00:00 1970 Received: from eggs.gnu.org ([2001:4830:134:3::10]:50603) by lists.gnu.org with esmtp (Exim 4.71) (envelope-from ) id 1Y20yu-0000PZ-9i for qemu-devel@nongnu.org; Fri, 19 Dec 2014 12:04:53 -0500 Received: from Debian-exim by eggs.gnu.org with spam-scanned (Exim 4.71) (envelope-from ) id 1Y20yn-0005sS-Fx for qemu-devel@nongnu.org; Fri, 19 Dec 2014 12:04:48 -0500 Received: from mx1.redhat.com ([209.132.183.28]:49435) by eggs.gnu.org with esmtp (Exim 4.71) (envelope-from ) id 1Y20yn-0005sO-7h for qemu-devel@nongnu.org; Fri, 19 Dec 2014 12:04:41 -0500 Received: from int-mx14.intmail.prod.int.phx2.redhat.com (int-mx14.intmail.prod.int.phx2.redhat.com [10.5.11.27]) by mx1.redhat.com (8.14.4/8.14.4) with ESMTP id sBJH4eIK004247 (version=TLSv1/SSLv3 cipher=DHE-RSA-AES256-GCM-SHA384 bits=256 verify=FAIL) for ; Fri, 19 Dec 2014 12:04:40 -0500 Message-ID: <54944B29.6050501@redhat.com> Date: Fri, 19 Dec 2014 08:58:33 -0700 From: Eric Blake MIME-Version: 1.0 References: <1418995502-14908-1-git-send-email-akong@redhat.com> In-Reply-To: <1418995502-14908-1-git-send-email-akong@redhat.com> Content-Type: text/plain; charset=utf-8 Content-Transfer-Encoding: 8bit Subject: Re: [Qemu-devel] [PATCH] check return value of fcntl() to detect invalid fd List-Id: List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , To: Amos Kong , qemu-devel@nongnu.org Cc: jasowang@redhat.com, stefanha@redhat.com, mst@redhat.com On 12/19/2014 06:25 AM, Amos Kong wrote: > Passing some invalid fds in QEMU commandline, the fds don't exist. > QEMU will get error "TUNGETIFF ioctl() failed: Bad file descriptor", > and coredump in setting queues. > > This patch checked return value of first operate to fd, QEMU will > report error and exit without coredump. It's effected for both netdev s/effected/needed/ > fds and vhost_net fds. > > Signed-off-by: Amos Kong > --- > net/tap.c | 16 +++++++++++++--- > 1 file changed, 13 insertions(+), 3 deletions(-) > > > - fcntl(fd, F_SETFL, O_NONBLOCK); > + ret = fcntl(fd, F_SETFL, O_NONBLOCK); > + if (ret < 0) { > + error_report("Fail to set file status to nonblock, " > + "%s", strerror(-ret)); Awkward grammar; I would suggest s/Fail/Failed/; s/nonblock/nonblocking/, if you still end up reporting the error here. Wrong, in multiple ways. fcntl does not return negative errno values, just -1, and strerror(1) is not what you want. Furthermore, this code is blindly clearing all other flags as part of setting O_NONBLOCK. The correct way to set O_NONBLOCK is to call F_GETFL first. Which is what qemu_set_nonblock() from oslib-posix.c already does (so reuse that instead of reinventing it here). Except _that_ function fails to report failure, so we need a bigger audit to fix it and all callers. -- Eric Blake eblake redhat com +1-919-301-3266 Libvirt virtualization library http://libvirt.org