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

On Wed, Jul 01, 2015 at 17:36:06 +0200, Michal Privoznik wrote: > 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(a)redhat.com&gt; > --- > > 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. ACK in freeze. BTW it was caused by commit e1de5521.