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(a)redhat.com&gt; --- 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);

sigemptyset(&sig_action.sa_mask); sigaction(SIGINT, &sig_action, &old_sig_action);

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&lt;ajia(a)redhat.com&gt; >> --- >> 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?

Thanks, Alex > Thanks > Wen Congyang > >> sigemptyset(&sig_action.sa_mask); >> sigaction(SIGINT,&sig_action,&old_sig_action); >> -- libvir-list mailing list libvir-list(a)redhat.com https://www.redhat.com/mailman/listinfo/libvir-list

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&lt;ajia(a)redhat.com&gt; >>> --- >>> 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); >>> >