[libvirt] [PATCH] qemu: Fix FD leak in qemudStartVMDaemon
The logfile FD is dup2'ed in __virExec in the child. The FD needs to be closed in the parent, otherwise it leaks. --- src/qemu/qemu_driver.c | 3 +++ 1 files changed, 3 insertions(+), 0 deletions(-) diff --git a/src/qemu/qemu_driver.c b/src/qemu/qemu_driver.c index c8f3a15..fbb1275 100644 --- a/src/qemu/qemu_driver.c +++ b/src/qemu/qemu_driver.c @@ -2963,6 +2963,9 @@ static int qemudStartVMDaemon(virConnectPtr conn, if (virDomainSaveStatus(driver->caps, driver->stateDir, vm) < 0) goto abort; + if (logfile != -1) + close(logfile); + return 0; cleanup: -- 1.6.3.3
On Wed, Mar 17, 2010 at 10:35:51PM +0100, Matthias Bolte wrote:
The logfile FD is dup2'ed in __virExec in the child. The FD needs to be closed in the parent, otherwise it leaks. --- src/qemu/qemu_driver.c | 3 +++ 1 files changed, 3 insertions(+), 0 deletions(-)
diff --git a/src/qemu/qemu_driver.c b/src/qemu/qemu_driver.c index c8f3a15..fbb1275 100644 --- a/src/qemu/qemu_driver.c +++ b/src/qemu/qemu_driver.c @@ -2963,6 +2963,9 @@ static int qemudStartVMDaemon(virConnectPtr conn, if (virDomainSaveStatus(driver->caps, driver->stateDir, vm) < 0) goto abort;
+ if (logfile != -1) + close(logfile); + return 0;
cleanup:
ACK, but we test if ((logfile = ...) < 0) goto cleanup; so the logical counterpart would be if (logfile >= 0) close(logfile); Daniel -- Daniel Veillard | libxml Gnome XML XSLT toolkit http://xmlsoft.org/ daniel@veillard.com | Rpmfind RPM search engine http://rpmfind.net/ http://veillard.com/ | virtualization library http://libvirt.org/
2010/3/18 Daniel Veillard <veillard@redhat.com>:
On Wed, Mar 17, 2010 at 10:35:51PM +0100, Matthias Bolte wrote:
The logfile FD is dup2'ed in __virExec in the child. The FD needs to be closed in the parent, otherwise it leaks. --- src/qemu/qemu_driver.c | 3 +++ 1 files changed, 3 insertions(+), 0 deletions(-)
diff --git a/src/qemu/qemu_driver.c b/src/qemu/qemu_driver.c index c8f3a15..fbb1275 100644 --- a/src/qemu/qemu_driver.c +++ b/src/qemu/qemu_driver.c @@ -2963,6 +2963,9 @@ static int qemudStartVMDaemon(virConnectPtr conn, if (virDomainSaveStatus(driver->caps, driver->stateDir, vm) < 0) goto abort;
+ if (logfile != -1) + close(logfile); + return 0;
cleanup:
ACK, but we test
if ((logfile = ...) < 0) goto cleanup;
so the logical counterpart would be
if (logfile >= 0) close(logfile);
Daniel
True. I just copied the the close call from the cleanup block. Both blocks (cleanup and abort) check for != 1, so one could argue to change them to >= 0 too. Matthias
On Thu, Mar 18, 2010 at 02:31:56PM +0100, Matthias Bolte wrote:
2010/3/18 Daniel Veillard <veillard@redhat.com>:
On Wed, Mar 17, 2010 at 10:35:51PM +0100, Matthias Bolte wrote:
The logfile FD is dup2'ed in __virExec in the child. The FD needs to be closed in the parent, otherwise it leaks. --- src/qemu/qemu_driver.c | 3 +++ 1 files changed, 3 insertions(+), 0 deletions(-)
diff --git a/src/qemu/qemu_driver.c b/src/qemu/qemu_driver.c index c8f3a15..fbb1275 100644 --- a/src/qemu/qemu_driver.c +++ b/src/qemu/qemu_driver.c @@ -2963,6 +2963,9 @@ static int qemudStartVMDaemon(virConnectPtr conn, if (virDomainSaveStatus(driver->caps, driver->stateDir, vm) < 0) goto abort;
+ if (logfile != -1) + close(logfile); + return 0;
cleanup:
ACK, but we test
if ((logfile = ...) < 0) goto cleanup;
so the logical counterpart would be
if (logfile >= 0) close(logfile);
Daniel
True. I just copied the the close call from the cleanup block. Both blocks (cleanup and abort) check for != 1, so one could argue to change them to >= 0 too.
Either way, let's plug the leak :-) thanks ! Daniel -- Daniel Veillard | libxml Gnome XML XSLT toolkit http://xmlsoft.org/ daniel@veillard.com | Rpmfind RPM search engine http://rpmfind.net/ http://veillard.com/ | virtualization library http://libvirt.org/
2010/3/18 Daniel Veillard <veillard@redhat.com>:
On Thu, Mar 18, 2010 at 02:31:56PM +0100, Matthias Bolte wrote:
2010/3/18 Daniel Veillard <veillard@redhat.com>:
On Wed, Mar 17, 2010 at 10:35:51PM +0100, Matthias Bolte wrote:
The logfile FD is dup2'ed in __virExec in the child. The FD needs to be closed in the parent, otherwise it leaks. --- src/qemu/qemu_driver.c | 3 +++ 1 files changed, 3 insertions(+), 0 deletions(-)
diff --git a/src/qemu/qemu_driver.c b/src/qemu/qemu_driver.c index c8f3a15..fbb1275 100644 --- a/src/qemu/qemu_driver.c +++ b/src/qemu/qemu_driver.c @@ -2963,6 +2963,9 @@ static int qemudStartVMDaemon(virConnectPtr conn, if (virDomainSaveStatus(driver->caps, driver->stateDir, vm) < 0) goto abort;
+ if (logfile != -1) + close(logfile); + return 0;
cleanup:
ACK, but we test
if ((logfile = ...) < 0) goto cleanup;
so the logical counterpart would be
if (logfile >= 0) close(logfile);
Daniel
True. I just copied the the close call from the cleanup block. Both blocks (cleanup and abort) check for != 1, so one could argue to change them to >= 0 too.
Either way, let's plug the leak :-)
thanks !
Daniel
Yep, pushed. Matthias
participants (2)
-
Daniel Veillard -
Matthias Bolte