| Message ID | 963102742.505036.1301042754895.JavaMail.root@zmail07.collab.prod.int.phx2.redhat.com |
|---|---|
| State | New |
| Headers | show |
Ulrich Obergfell <uobergfe@redhat.com> wrote: > This is version 2 of the patch that I originally posted in: > > http://lists.gnu.org/archive/html/qemu-devel/2011-03/msg02063.html > > [Sorry, I missed to include the keyword 'PATCH' in the subject > of the original post.] > > The following commit breaks the code of the function palette_destroy(). > > http://git.kernel.org/?p=virt/kvm/qemu-kvm.git;a=commit;h=e31e3694afef58ba191cbcc6875ec243e5971268 > > The broken code causes a severe memory leak of 'VncPalette' structures > because it never frees anything: > > 70 void palette_destroy(VncPalette *palette) > 71 { > 72 if (palette == NULL) { > 73 qemu_free(palette); > 74 } > 75 } > > Version 2 of the patch calls qemu_free() unconditionally. > > Signed-off-by: Ulrich Obergfell <uobergfe@redhat.com> Ouchhhhhhhhhhhhhhh Reviewed-by: Juan Quintela <quintela@redhat.com> A new reason to never ever test if pointer is != NULL before calling free. Good catch.
On Fri, Mar 25, 2011 at 8:45 AM, Ulrich Obergfell <uobergfe@redhat.com> wrote: > > This is version 2 of the patch that I originally posted in: > > http://lists.gnu.org/archive/html/qemu-devel/2011-03/msg02063.html > > [Sorry, I missed to include the keyword 'PATCH' in the subject > of the original post.] > > The following commit breaks the code of the function palette_destroy(). > > http://git.kernel.org/?p=virt/kvm/qemu-kvm.git;a=commit;h=e31e3694afef58ba191cbcc6875ec243e5971268 > > The broken code causes a severe memory leak of 'VncPalette' structures > because it never frees anything: > > 70 void palette_destroy(VncPalette *palette) > 71 { > 72 if (palette == NULL) { > 73 qemu_free(palette); > 74 } > 75 } > > Version 2 of the patch calls qemu_free() unconditionally. > > Signed-off-by: Ulrich Obergfell <uobergfe@redhat.com> Looks good. Reviewed-by: Stefan Hajnoczi <stefanha@linux.vnet.ibm.com>
On 03/25/2011 04:31 AM, Stefan Hajnoczi wrote: > On Fri, Mar 25, 2011 at 8:45 AM, Ulrich Obergfell<uobergfe@redhat.com> wrote: >> This is version 2 of the patch that I originally posted in: >> >> http://lists.gnu.org/archive/html/qemu-devel/2011-03/msg02063.html >> >> [Sorry, I missed to include the keyword 'PATCH' in the subject >> of the original post.] >> >> The following commit breaks the code of the function palette_destroy(). >> >> http://git.kernel.org/?p=virt/kvm/qemu-kvm.git;a=commit;h=e31e3694afef58ba191cbcc6875ec243e5971268 >> >> The broken code causes a severe memory leak of 'VncPalette' structures >> because it never frees anything: >> >> 70 void palette_destroy(VncPalette *palette) >> 71 { >> 72 if (palette == NULL) { >> 73 qemu_free(palette); >> 74 } >> 75 } >> >> Version 2 of the patch calls qemu_free() unconditionally. >> >> Signed-off-by: Ulrich Obergfell<uobergfe@redhat.com> > Looks good. Applied. Thanks. Regards, Anthony Liguori > Reviewed-by: Stefan Hajnoczi<stefanha@linux.vnet.ibm.com>
diff -up ./ui/vnc-palette.c.orig0 ./ui/vnc-palette.c --- ./ui/vnc-palette.c.orig0 2011-03-15 03:53:22.000000000 +0100 +++ ./ui/vnc-palette.c 2011-03-21 20:19:02.736948725 +0100 @@ -69,9 +69,7 @@ void palette_init(VncPalette *palette, s void palette_destroy(VncPalette *palette) { - if (palette == NULL) { - qemu_free(palette); - } + qemu_free(palette); } int palette_put(VncPalette *palette, uint32_t color)
This is version 2 of the patch that I originally posted in: http://lists.gnu.org/archive/html/qemu-devel/2011-03/msg02063.html [Sorry, I missed to include the keyword 'PATCH' in the subject of the original post.] The following commit breaks the code of the function palette_destroy(). http://git.kernel.org/?p=virt/kvm/qemu-kvm.git;a=commit;h=e31e3694afef58ba191cbcc6875ec243e5971268 The broken code causes a severe memory leak of 'VncPalette' structures because it never frees anything: 70 void palette_destroy(VncPalette *palette) 71 { 72 if (palette == NULL) { 73 qemu_free(palette); 74 } 75 } Version 2 of the patch calls qemu_free() unconditionally. Signed-off-by: Ulrich Obergfell <uobergfe@redhat.com>