* [Qemu-devel] [PATCH] io: some fixes to handling of /dev/null when running commands
@ 2016-01-11 13:05 Daniel P. Berrange
2016-01-11 21:02 ` Paolo Bonzini
0 siblings, 1 reply; 2+ messages in thread
From: Daniel P. Berrange @ 2016-01-11 13:05 UTC (permalink / raw)
To: qemu-devel; +Cc: Paolo Bonzini
The /dev/null file handle was leaked in a couple of places.
There is also the possibility that both readfd and writefd
point to the same /dev/null file handle, so care must be
taken not to close the same file handle twice.
Signed-off-by: Daniel P. Berrange <berrange@redhat.com>
---
io/channel-command.c | 22 ++++++++++++++++------
1 file changed, 16 insertions(+), 6 deletions(-)
diff --git a/io/channel-command.c b/io/channel-command.c
index a220fe8..a9c67aa 100644
--- a/io/channel-command.c
+++ b/io/channel-command.c
@@ -66,7 +66,7 @@ qio_channel_command_new_spawn(const char *const argv[],
if (stdinnull || stdoutnull) {
devnull = open("/dev/null", O_RDWR);
- if (!devnull) {
+ if (devnull < 0) {
error_setg_errno(errp, errno,
"Unable to open /dev/null");
goto error;
@@ -98,6 +98,9 @@ qio_channel_command_new_spawn(const char *const argv[],
close(stdoutfd[0]);
close(stdoutfd[1]);
}
+ if (devnull != -1) {
+ close(devnull);
+ }
execv(argv[0], (char * const *)argv);
_exit(1);
@@ -117,6 +120,9 @@ qio_channel_command_new_spawn(const char *const argv[],
return ioc;
error:
+ if (devnull != -1) {
+ close(devnull);
+ }
if (stdinfd[0] != -1) {
close(stdinfd[0]);
}
@@ -202,12 +208,12 @@ static void qio_channel_command_finalize(Object *obj)
QIOChannelCommand *ioc = QIO_CHANNEL_COMMAND(obj);
if (ioc->readfd != -1) {
close(ioc->readfd);
- ioc->readfd = -1;
}
- if (ioc->writefd != -1) {
+ if (ioc->writefd != -1 &&
+ ioc->writefd != ioc->readfd) {
close(ioc->writefd);
- ioc->writefd = -1;
}
+ ioc->writefd = ioc->readfd = -1;
if (ioc->pid > 0) {
#ifndef WIN32
qio_channel_command_abort(ioc, NULL);
@@ -299,12 +305,16 @@ static int qio_channel_command_close(QIOChannel *ioc,
/* We close FDs before killing, because that
* gives a better chance of clean shutdown
*/
- if (close(cioc->writefd) < 0) {
+ if (cioc->readfd != -1 &&
+ close(cioc->readfd) < 0) {
rv = -1;
}
- if (close(cioc->readfd) < 0) {
+ if (cioc->writefd != -1 &&
+ cioc->writefd != cioc->readfd &&
+ close(cioc->writefd) < 0) {
rv = -1;
}
+ cioc->writefd = cioc->readfd = -1;
#ifndef WIN32
if (qio_channel_command_abort(cioc, errp) < 0) {
return -1;
--
2.5.0
^ permalink raw reply related [flat|nested] 2+ messages in thread
* Re: [Qemu-devel] [PATCH] io: some fixes to handling of /dev/null when running commands
2016-01-11 13:05 [Qemu-devel] [PATCH] io: some fixes to handling of /dev/null when running commands Daniel P. Berrange
@ 2016-01-11 21:02 ` Paolo Bonzini
0 siblings, 0 replies; 2+ messages in thread
From: Paolo Bonzini @ 2016-01-11 21:02 UTC (permalink / raw)
To: Daniel P. Berrange, qemu-devel
On 11/01/2016 14:05, Daniel P. Berrange wrote:
> The /dev/null file handle was leaked in a couple of places.
> There is also the possibility that both readfd and writefd
> point to the same /dev/null file handle, so care must be
> taken not to close the same file handle twice.
>
> Signed-off-by: Daniel P. Berrange <berrange@redhat.com>
> ---
> io/channel-command.c | 22 ++++++++++++++++------
> 1 file changed, 16 insertions(+), 6 deletions(-)
>
> diff --git a/io/channel-command.c b/io/channel-command.c
> index a220fe8..a9c67aa 100644
> --- a/io/channel-command.c
> +++ b/io/channel-command.c
> @@ -66,7 +66,7 @@ qio_channel_command_new_spawn(const char *const argv[],
>
> if (stdinnull || stdoutnull) {
> devnull = open("/dev/null", O_RDWR);
> - if (!devnull) {
> + if (devnull < 0) {
> error_setg_errno(errp, errno,
> "Unable to open /dev/null");
> goto error;
> @@ -98,6 +98,9 @@ qio_channel_command_new_spawn(const char *const argv[],
> close(stdoutfd[0]);
> close(stdoutfd[1]);
> }
> + if (devnull != -1) {
> + close(devnull);
> + }
>
> execv(argv[0], (char * const *)argv);
> _exit(1);
> @@ -117,6 +120,9 @@ qio_channel_command_new_spawn(const char *const argv[],
> return ioc;
>
> error:
> + if (devnull != -1) {
> + close(devnull);
> + }
> if (stdinfd[0] != -1) {
> close(stdinfd[0]);
> }
> @@ -202,12 +208,12 @@ static void qio_channel_command_finalize(Object *obj)
> QIOChannelCommand *ioc = QIO_CHANNEL_COMMAND(obj);
> if (ioc->readfd != -1) {
> close(ioc->readfd);
> - ioc->readfd = -1;
> }
> - if (ioc->writefd != -1) {
> + if (ioc->writefd != -1 &&
> + ioc->writefd != ioc->readfd) {
> close(ioc->writefd);
> - ioc->writefd = -1;
> }
> + ioc->writefd = ioc->readfd = -1;
> if (ioc->pid > 0) {
> #ifndef WIN32
> qio_channel_command_abort(ioc, NULL);
> @@ -299,12 +305,16 @@ static int qio_channel_command_close(QIOChannel *ioc,
> /* We close FDs before killing, because that
> * gives a better chance of clean shutdown
> */
> - if (close(cioc->writefd) < 0) {
> + if (cioc->readfd != -1 &&
> + close(cioc->readfd) < 0) {
> rv = -1;
> }
> - if (close(cioc->readfd) < 0) {
> + if (cioc->writefd != -1 &&
> + cioc->writefd != cioc->readfd &&
> + close(cioc->writefd) < 0) {
> rv = -1;
> }
> + cioc->writefd = cioc->readfd = -1;
> #ifndef WIN32
> if (qio_channel_command_abort(cioc, errp) < 0) {
> return -1;
>
Reviewed-by: Paolo Bonzini <pbonzini@redhat.com>
^ permalink raw reply [flat|nested] 2+ messages in thread
end of thread, other threads:[~2016-01-11 21:02 UTC | newest]
Thread overview: 2+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
2016-01-11 13:05 [Qemu-devel] [PATCH] io: some fixes to handling of /dev/null when running commands Daniel P. Berrange
2016-01-11 21:02 ` Paolo Bonzini
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).