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

[Michal Privoznik]

On 01.07.2015 18:11, Daniel P. Berrange wrote:
> 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 at redhat.com>
>>> ---
>>>
>>> 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.
> 

Done.

Michal