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

On Wed, Jul 01, 2015 at 05:48:32PM +0200, Peter Krempa wrote: > 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. Wow, how did this work so well for so long. We need to apply this to the stable branches from 1.2.13 onwards.