Re: [Qemu-devel] [PATCH] check return value of fcntl() to detect invalid fd

[Eric Blake <eblake@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 <akong@redhat.com>
> ---
>  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