[libvirt PATCH 05/11] qemu_command: fix FD usage in qemuBuildInterfaceCommandLine

[Pavel Hrdina]

On Mon, Nov 16, 2020 at 05:18:51PM +0100, Peter Krempa wrote:
> On Mon, Nov 16, 2020 at 16:38:52 +0100, Pavel Hrdina wrote:
> > If virCommandPassFD() is called with VIR_COMMAND_PASS_FD_CLOSE_PARENT
> > the passed FD is closed and should not be used after that call.
> 
> This description doesn't seem to make sense. The filedescriptor itself
> isn't really used in the caller. The only thing that is used is the
> number of the filedescriptor which is used to format the string  being
> passed to qemu which is should be correct.

Right, the description was not accurate. By "should not be used" I meant
that we close it again in the cleanup section.

> > 
> > Signed-off-by: Pavel Hrdina <phrdina at redhat.com>
> > ---
> >  src/qemu/qemu_command.c | 2 +-
> >  1 file changed, 1 insertion(+), 1 deletion(-)
> > 
> > diff --git a/src/qemu/qemu_command.c b/src/qemu/qemu_command.c
> > index 0eec35da16..0580feb475 100644
> > --- a/src/qemu/qemu_command.c
> > +++ b/src/qemu/qemu_command.c
> > @@ -8142,7 +8142,7 @@ qemuBuildInterfaceCommandLine(virQEMUDriverPtr driver,
> >          g_autofree char *fdset = NULL;
> >          g_autofree char *addfdarg = NULL;
> >  
> > -        virCommandPassFD(cmd, vdpafd, VIR_COMMAND_PASS_FD_CLOSE_PARENT);
> > +        virCommandPassFD(cmd, vdpafd, 0);
> >          fdset = qemuVirCommandGetFDSet(cmd, vdpafd);
> 
> A proper fix will be to move 'vdpafd = -1;' here or copy it's value
> somewhere to prevent the double close ...

Moving the 'vdpafd = -1;' here would introduce a different bug because
'vdpafd' is used after that condition as well. So I guess the only
correct solution is to copy the value to a different variable.

Thanks for the review, I'll send a v2.

Pavel
-------------- next part --------------
A non-text attachment was scrubbed...
Name: signature.asc
Type: application/pgp-signature
Size: 833 bytes
Desc: not available
URL: <http://listman.redhat.com/archives/libvir-list/attachments/20201116/e4748f1e/attachment-0001.sig>