Re: [PATCH 2/2] net/tap: fix error reporting when bridge helper forgets to send an FD

[Philippe Mathieu-Daudé]

On 5/5/21 2:12 PM, Daniel P. Berrangé wrote:
> The recv_fd() method returns -1 on error, 0 on end of file, or an FD
> number on success.
> 
> Technically 0 is also a valid FD number, so the return value is
> ambiguous.
> 
> The caller doesn't even consider the possibility of 0 meaning end
> of file, so just blindly assume it is a valid FD.
> 
> IOW if the bridge helper exits with 0 status code and forgets to
> send an FD, QEMU will accidentally try to use STDIN as a TAP FD.
> 
> Fortunately we get an error shortly thereafter
> 
>  $ qemu-system-x86_64 -netdev bridge,br=br99,helper=/bin/true,id=ns0
>  qemu-system-x86_64: -netdev bridge,br=br99,helper=/bin/true,id=ns0:
>    Unable to query TUNGETIFF on FD 0: Inappropriate ioctl for device
> 
> It is better if we correctly diagnose this broken bridge helper
> though. To do this we need to return the FD in an output parameter
> to remove the ambiguity and then fix the caller to check for the
> end of file condition. With this done we now get
> 
>   $ qemu-system-x86_64 -netdev bridge,br=br99,helper=/bin/true,id=ns0
>   qemu-system-x86_64: -netdev bridge,br=br99,helper=/bin/true,id=ns0:
>     bridge helper did not send a file descriptor
> 
> Fixes: https://gitlab.com/qemu-project/qemu/-/issues/166
> Signed-off-by: Daniel P. Berrangé <berrange@redhat.com>
> ---
>  net/tap.c | 32 ++++++++++++++++++++++----------
>  1 file changed, 22 insertions(+), 10 deletions(-)

> +        /*
> +         * ret == 0 means EOF, and if status == 0 then helper
> +         * exited cleanly but forgot to send us an FD. Opps...

Opps or Oops?

Otherwise:
Reviewed-by: Philippe Mathieu-Daudé <philmd@redhat.com>