[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]
Re: [PATCH 2/2] net/tap: fix error reporting when bridge helper forgets
From: |
Daniel P . Berrangé |
Subject: |
Re: [PATCH 2/2] net/tap: fix error reporting when bridge helper forgets to send an FD |
Date: |
Wed, 12 May 2021 16:36:27 +0100 |
User-agent: |
Mutt/2.0.6 (2021-03-06) |
On Wed, May 05, 2021 at 02:32:04PM +0200, Philippe Mathieu-Daudé wrote:
> 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?
Oops, it should be Oops :-)
>
> Otherwise:
> Reviewed-by: Philippe Mathieu-Daudé <philmd@redhat.com>
>
Regards,
Daniel
--
|: https://berrange.com -o- https://www.flickr.com/photos/dberrange :|
|: https://libvirt.org -o- https://fstop138.berrange.com :|
|: https://entangle-photo.org -o- https://www.instagram.com/dberrange :|