[PATCH] lxc: Don't pass a local variable address randomly

[Michal Privoznik <mprivozn@xxxxxxxxxx>]

So, recently I was testing the LXC driver. You know, startup some
domains. But to my surprise, I was not able to start a single one:

  virsh # start --console test
  error: Reconnected to the hypervisor
  error: Failed to start domain test
  error: internal error: guest failed to start: unexpected exit status 125

So I've start digging. It turns out, that in virExec(), when I printed
out the @cmd, I got strange values: *(cmd->outfdptr) was certainly not
valid FD number: it has random value of several millions. This
obviously made prepareStdFd(childout, STDOUT_FILENO) fail (line 611).
But outfdptr is set in virCommandSetOutputFD(). The only place within
LXC driver where the function is called is in
virLXCProcessBuildControllerCmd(). If you take a closer look at the
function it looks like this:

static virCommandPtr
virLXCProcessBuildControllerCmd(virLXCDriverPtr driver,
                                ..
                                int logfd,
                                const char *pidfile)
{
    ...
    virCommandSetOutputFD(cmd, &logfd);
    virCommandSetErrorFD(cmd, &logfd);
    ...
}

Yes, you guessed it. @logfd is passed into the function by value.
However, in the function we try to get its address (an address of a
local variable) which is no longer valid once function is finished and
stack is cleaned. Therefore when cmd->outfdptr is evaluated at any
point after this function, we may get a random number, depending on
what's currently on the stack. Of course, this may work sometimes too
- it depends on the compiler how it arranges the code, when the stack
is wiped out.

In order to fix this, lets pass a pointer to @logfd instead of
figuring out (wrong) its value in a function.

Signed-off-by: Michal Privoznik <mprivozn@xxxxxxxxxx>
---

I'd love to target this into the current release - in my case LXC
driver is unusable without this patch. There might be other users
affected too.


 src/lxc/lxc_process.c | 8 ++++----
 1 file changed, 4 insertions(+), 4 deletions(-)

diff --git a/src/lxc/lxc_process.c b/src/lxc/lxc_process.c
index 6c23a0b..2bdce3b 100644
--- a/src/lxc/lxc_process.c
+++ b/src/lxc/lxc_process.c
@@ -750,7 +750,7 @@ virLXCProcessBuildControllerCmd(virLXCDriverPtr driver,
                                 int *files,
                                 size_t nfiles,
                                 int handshakefd,
-                                int logfd,
+                                int * const logfd,
                                 const char *pidfile)
 {
     size_t i;
@@ -820,8 +820,8 @@ virLXCProcessBuildControllerCmd(virLXCDriverPtr driver,
     virCommandPassFD(cmd, handshakefd, 0);
     virCommandDaemonize(cmd);
     virCommandSetPidFile(cmd, pidfile);
-    virCommandSetOutputFD(cmd, &logfd);
-    virCommandSetErrorFD(cmd, &logfd);
+    virCommandSetOutputFD(cmd, logfd);
+    virCommandSetErrorFD(cmd, logfd);
     /* So we can pause before exec'ing the controller to
      * write the live domain status XML with the PID */
     virCommandRequireHandshake(cmd);
@@ -1208,7 +1208,7 @@ int virLXCProcessStart(virConnectPtr conn,
                                                 ttyFDs, nttyFDs,
                                                 files, nfiles,
                                                 handshakefds[1],
-                                                logfd,
+                                                &logfd,
                                                 pidfile)))
         goto cleanup;
 
-- 
2.3.6

--
libvir-list mailing list
libvir-list@xxxxxxxxxx
https://www.redhat.com/mailman/listinfo/libvir-list