[libvirt] [PATCH] virsh: avoid uninitialized memory usage

Wen Congyang wency at cn.fujitsu.com
Thu Apr 19 08:53:41 UTC 2012


At 04/19/2012 04:40 PM, Alex Jia Wrote:
> On 04/19/2012 04:19 PM, Wen Congyang wrote:
>> At 04/19/2012 04:09 PM, Alex Jia Wrote:
>>> Detected by valgrind.
>>>
>>> * tools/virsh.c (cmdBlockPull): fix uninitialized memory usage.
>>>
>>> * How to reproduce?
>>> $ qemu-img create /var/lib/libvirt/images/test 1M
>>> $ cat>  /tmp/test.xml<<EOF
>>> <domain type='qemu'>
>>>    <name>test</name>
>>>    <memory>219200</memory>
>>>    <vcpu>1</vcpu>
>>>    <os>
>>>      <type arch='x86_64'>hvm</type>
>>>      <boot dev='hd'/>
>>>    </os>
>>>    <devices>
>>>      <disk type='file' device='disk'>
>>>        <driver name='qemu' type='raw'/>
>>>        <source file='/var/lib/libvirt/images/test'/>
>>>        <target dev='vda' bus='virtio'/>
>>>      </disk>
>>>      <input type='mouse' bus='ps2'/>
>>>      <graphics type='spice' autoport='yes' listen='0.0.0.0'/>
>>>    </devices>
>>> </domain>
>>> EOF
>>> $ virsh define /tmp/test.xml
>>> $ valgrind -v virsh blockpull test /var/lib/libvirt/images/test --wait
>>>
>>> actual result:
>>>
>>> ==10906== 1 errors in context 1 of 1:
>>> ==10906== Syscall param rt_sigaction(act->sa_flags) points to
>>> uninitialised byte(s)
>>> ==10906==    at 0x39CF80F5BE: __libc_sigaction (sigaction.c:67)
>>> ==10906==    by 0x43016C: cmdBlockPull (virsh.c:7638)
>>> ==10906==    by 0x4150D4: vshCommandRun (virsh.c:18574)
>>> ==10906==    by 0x425E73: main (virsh.c:20178)
>>> ==10906==  Address 0x7fefffae8 is on thread 1's stack
>>>
>>>
>>> Signed-off-by: Alex Jia<ajia at redhat.com>
>>> ---
>>>   tools/virsh.c |    1 +
>>>   1 files changed, 1 insertions(+), 0 deletions(-)
>>>
>>> diff --git a/tools/virsh.c b/tools/virsh.c
>>> index 95ed7bc..4e4ca57 100644
>>> --- a/tools/virsh.c
>>> +++ b/tools/virsh.c
>>> @@ -7634,6 +7634,7 @@ cmdBlockPull(vshControl *ctl, const vshCmd *cmd)
>>>
>>>           intCaught = 0;
>>>           sig_action.sa_sigaction = vshCatchInt;
>>> +        sigemptyset((sigset_t *)&sig_action.sa_flags);
>> Why using sigemptyset here? You should use 'sig_action.sa_flags = 0'.
> Yeah, I think 'sig_action.sa_flags = 0' is right, but I don't know what
> the difference are,
> could you explain more?

sigset_t is:
# define _SIGSET_NWORDS (1024 / (8 * sizeof (unsigned long int)))
typedef struct
  {
    unsigned long int __val[_SIGSET_NWORDS];
  } __sigset_t;

The length of sigset is larger than sizeof(int)

If you use sigemptyset() to clear flags, it will affect the memory after flags.
It is very dangerous!!!

Thanks
Wen Congyang

> 
> Thanks,
> Alex
>> Thanks
>> Wen Congyang
>>
>>>           sigemptyset(&sig_action.sa_mask);
>>>           sigaction(SIGINT,&sig_action,&old_sig_action);
>>>
> 
> 




More information about the libvir-list mailing list