[libvirt] [PATCHv2] command: avoid fd leak on failure
Eric Blake
eblake at redhat.com
Sat Jul 16 03:58:39 UTC 2011
On 07/14/2011 11:04 AM, Eric Blake wrote:
> virCommandTransferFD promises that the fd is no longer owned by
> the caller. Normally, we want the fd to remain open until the
> child runs, but in error situations, we must close it earlier.
>
> To avoid any risks of double-close due to misunderstanding about
> this interface, change it to take a pointer which gets set to
> -1 in the caller to record that the transfer was successful.
>
> * src/util/command.h (virCommandTransferFD): Alter signature.
> * src/util/command.c (virCommandTransferFD): Close fd now if we
> can't track it to close later.
> (virCommandKeepFD): Adjust helper to make this easier.
> (virCommandRequireHandshake): Adjust callers.
> * src/qemu/qemu_command.c (qemuBuildCommandLine): Likewise.
> * src/uml/uml_conf.c (umlBuildCommandLineChr): Likewise.
> * tests/commandtest.c (test3): Likewise.
> * docs/internals/command.html.in: Update the docs.
> ---
>
> v2: alter the signature, and adjust all callers, to make it foolproof
> at avoiding a double-close
Another flaw in v2, that v1 did not have:
> @@ -3698,7 +3698,7 @@ qemuBuildCommandLine(virConnectPtr conn,
> goto error;
>
> last_good_net = i;
> - virCommandTransferFD(cmd, tapfd);
> + virCommandTransferFD(cmd,&tapfd);
>
> if (snprintf(tapfd_name, sizeof(tapfd_name), "%d",
> tapfd)>= sizeof(tapfd_name))
Oops - this prints tapfd_name as -1 instead of the fd that the child
will be inheriting.
But rearranging the lines, and doing snprintf prior to
virCommandTransferFD, is also problematic - if the snprintf fails, then
we go to error without doing the virCommandTransferFD, then the
virCommandPtr no longer closes the fd and we have a leak.
I'm starting to think that resetting fd to -1 as a safety valve causes
more harm than good, and hope that we can go with v1 after all.
--
Eric Blake eblake at redhat.com +1-801-349-2682
Libvirt virtualization library http://libvirt.org
More information about the libvir-list
mailing list