From mboxrd@z Thu Jan 1 00:00:00 1970 Received: from eggs.gnu.org ([2001:4830:134:3::10]:46098) by lists.gnu.org with esmtp (Exim 4.71) (envelope-from ) id 1Y2vwt-0000nd-Ri for qemu-devel@nongnu.org; Mon, 22 Dec 2014 00:54:38 -0500 Received: from Debian-exim by eggs.gnu.org with spam-scanned (Exim 4.71) (envelope-from ) id 1Y2vwm-00038r-Fu for qemu-devel@nongnu.org; Mon, 22 Dec 2014 00:54:31 -0500 Received: from mx1.redhat.com ([209.132.183.28]:50567) by eggs.gnu.org with esmtp (Exim 4.71) (envelope-from ) id 1Y2vwm-00036a-7s for qemu-devel@nongnu.org; Mon, 22 Dec 2014 00:54:24 -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 sBM5sMKe004551 (version=TLSv1/SSLv3 cipher=DHE-RSA-AES256-GCM-SHA384 bits=256 verify=FAIL) for ; Mon, 22 Dec 2014 00:54:23 -0500 Message-ID: <5497B20C.5070900@redhat.com> Date: Mon, 22 Dec 2014 13:54:20 +0800 From: Jason Wang MIME-Version: 1.0 References: <1418995502-14908-1-git-send-email-akong@redhat.com> <5497948D.5050900@redhat.com> <20141222052840.GA8597@air.redhat.com> In-Reply-To: <20141222052840.GA8597@air.redhat.com> Content-Type: text/plain; charset=windows-1252 Content-Transfer-Encoding: 7bit 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 Cc: qemu-devel@nongnu.org, stefanha@redhat.com, mst@redhat.com On 12/22/2014 01:28 PM, Amos Kong wrote: > On Mon, Dec 22, 2014 at 11:48:29AM +0800, Jason Wang wrote: >> On 12/19/2014 09:25 PM, 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 >>> fds and vhost_net fds. >>> >>> Signed-off-by: Amos Kong >>> --- >>> net/tap.c | 16 +++++++++++++--- >>> 1 file changed, 13 insertions(+), 3 deletions(-) >>> >>> diff --git a/net/tap.c b/net/tap.c >>> index bde6b58..039280a 100644 >>> --- a/net/tap.c >>> +++ b/net/tap.c >>> @@ -688,7 +688,7 @@ int net_init_tap(const NetClientOptions *opts, const char *name, >>> NetClientState *peer) >>> { >>> const NetdevTapOptions *tap; >>> - int fd, vnet_hdr = 0, i = 0, queues; >>> + int fd, vnet_hdr = 0, i = 0, queues, ret; >>> /* for the no-fd, no-helper case */ >>> const char *script = NULL; /* suppress wrong "uninit'd use" gcc warning */ >>> const char *downscript = NULL; >>> @@ -722,7 +722,12 @@ int net_init_tap(const NetClientOptions *opts, const char *name, >>> return -1; >>> } >>> >>> - 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)); >>> + return -1; >>> + } >> This may not work. There may be still some kinds of fd can pass this but >> still fail at TUNGETIFF or other tun ioctls. > Early catching the error is better. This only help to check if the fd > exists. If you just want to check the existence. Why don't you do it in monitor_handle_fd_param() to let other case to benefit also? And probably F_GETFL is better in this case. But doing this does not solve the issue you mention in the commit log. Even if fd exists, if it was not a tap fd, qemu may still abort.