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

[Jason Wang]

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 <address@hidden>
> ---
>  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.

Probably you need to fail during TUNGETIFF, which can make sure it was
not a tap fd.
>  
>          vnet_hdr = tap_probe_vnet_hdr(fd);
>  
> @@ -761,7 +766,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;
> +            }
>  
>              if (i == 0) {
>                  vnet_hdr = tap_probe_vnet_hdr(fd);