[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]
Re: [Qemu-devel] [PATCH] check return value of fcntl() to detect invalid
From: |
Eric Blake |
Subject: |
Re: [Qemu-devel] [PATCH] check return value of fcntl() to detect invalid fd |
Date: |
Fri, 19 Dec 2014 08:58:33 -0700 |
User-agent: |
Mozilla/5.0 (X11; Linux x86_64; rv:31.0) Gecko/20100101 Thunderbird/31.3.0 |
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 <address@hidden>
> ---
> 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