After an evaluation, GNOME has moved from Bugzilla to GitLab. Learn more about GitLab.
No new issues can be reported in GNOME Bugzilla anymore.
To report an issue in a GNOME project, go to GNOME GitLab.
Do not go to GNOME Gitlab for: Bluefish, Doxygen, GnuCash, GStreamer, java-gnome, LDTP, NetworkManager, Tomboy.
Bug 759520 - pnmenc: Fix memory leaks/mishandling
pnmenc: Fix memory leaks/mishandling
Status: RESOLVED FIXED
Product: GStreamer
Classification: Platform
Component: gst-plugins-bad
git master
Other Linux
: Normal normal
: 1.7.1
Assigned To: GStreamer Maintainers
GStreamer Maintainers
Depends on:
Blocks:
 
 
Reported: 2015-12-16 00:06 UTC by Vineeth
Modified: 2015-12-17 09:28 UTC
See Also:
GNOME target: ---
GNOME version: ---


Attachments
pnmenc: Fix string leak (1008 bytes, patch)
2015-12-16 00:07 UTC, Vineeth
committed Details | Review
Fix logic to remove memory mishandlings. (2.11 KB, patch)
2015-12-16 01:46 UTC, Vineeth
none Details | Review
Fix logic to remove memory mishandlings. (2.11 KB, patch)
2015-12-16 04:06 UTC, Vineeth
none Details | Review
Fix logic to remove memory mishandlings. (1.35 KB, patch)
2015-12-16 23:54 UTC, Vineeth
committed Details | Review

Description Vineeth 2015-12-16 00:06:20 UTC
Fix memory leaks found when testing sample pipelines with valgrind
Comment 1 Vineeth 2015-12-16 00:07:14 UTC
Created attachment 317465 [details] [review]
pnmenc: Fix string leak

==17823== Thread 1:
==17823== 16 bytes in 1 blocks are definitely lost in loss record 966 of 2,112
==17823==    at 0x402C17C: malloc (in /usr/lib/valgrind/vgpreload_memcheck-x86-linux.so)
==17823==    by 0x43F32AF: __vasprintf_chk (vasprintf_chk.c:80)
==17823==    by 0x425B689: g_vasprintf (in /lib/i386-linux-gnu/libglib-2.0.so.0.4002.0)
==17823==    by 0x4234FF2: g_strdup_vprintf (in /lib/i386-linux-gnu/libglib-2.0.so.0.4002.0)
==17823==    by 0x4235022: g_strdup_printf (in /lib/i386-linux-gnu/libglib-2.0.so.0.4002.0)
==17823==    by 0x41758FF: gst_pnmenc_handle_frame (gstpnmenc.c:199)
==17823==    by 0x4CBC11A: gst_video_encoder_chain (gstvideoencoder.c:1480)
==17823==    by 0x40A2BA7: gst_pad_push_data (gstpad.c:4153)
==17823==    by 0x40AB866: gst_pad_push (gstpad.c:4524)
==17823==    by 0x616F1A5: gst_base_transform_chain (gstbasetransform.c:2369)
==17823==    by 0x40A2BA7: gst_pad_push_data (gstpad.c:4153)
==17823==    by 0x40AB866: gst_pad_push (gstpad.c:4524)
Comment 2 Vineeth 2015-12-16 01:46:38 UTC
Created attachment 317466 [details] [review]
Fix logic to remove memory mishandlings.
Comment 3 Vineeth 2015-12-16 01:47:36 UTC
The 2nd patch is to fix the below errors

==19310== Thread 2 videotestsrc0:sr:
==19310== Syscall param writev(vector[...]) points to uninitialised byte(s)
==19310==    at 0x43DAEE3: writev (writev.c:54)
==19310==    by 0x65ACC77: gst_writev (gstelements_private.c:116)
==19310==    by 0x65AD127: gst_writev_buffers (gstelements_private.c:239)
==19310==    by 0x65B4830: gst_file_sink_render_buffers (gstfilesink.c:665)
==19310==    by 0x65B4D68: gst_file_sink_render (gstfilesink.c:731)
==19310==    by 0x655E673: gst_base_sink_chain_unlocked.isra.12 (gstbasesink.c:3532)
==19310==    by 0x65605F7: gst_base_sink_chain_main (gstbasesink.c:3655)
==19310==    by 0x40A2BA7: gst_pad_push_data (gstpad.c:4153)
==19310==    by 0x40AB866: gst_pad_push (gstpad.c:4524)
==19310==    by 0x4CBD9CE: gst_video_encoder_finish_frame (gstvideoencoder.c:2196)
==19310==    by 0x4175AC7: gst_pnmenc_handle_frame (gstpnmenc.c:283)
==19310==    by 0x4CBC11A: gst_video_encoder_chain (gstvideoencoder.c:1480)
==19310==  Address 0x6380f17 is 311,135 bytes inside a block of size 322,658 alloc'd
==19310==    at 0x402C17C: malloc (in /usr/lib/valgrind/vgpreload_memcheck-x86-linux.so)
==19310==    by 0x421CBE2: g_malloc (in /lib/i386-linux-gnu/libglib-2.0.so.0.4002.0)
==19310==    by 0x4233281: g_slice_alloc (in /lib/i386-linux-gnu/libglib-2.0.so.0.4002.0)
==19310==    by 0x405CCB3: _sysmem_new_block (gstallocator.c:414)
==19310==    by 0x405D38A: gst_allocator_alloc (gstallocator.c:311)
==19310==    by 0x40691B5: gst_buffer_new_allocate (gstbuffer.c:755)
==19310==    by 0x4CBCBA5: gst_video_encoder_allocate_output_buffer (gstvideoencoder.c:1789)
==19310==    by 0x4175960: gst_pnmenc_handle_frame (gstpnmenc.c:212)
==19310==    by 0x4CBC11A: gst_video_encoder_chain (gstvideoencoder.c:1480)
==19310==    by 0x40A2BA7: gst_pad_push_data (gstpad.c:4153)
==19310==    by 0x40AB866: gst_pad_push (gstpad.c:4524)
==19310==    by 0x656F1A5: gst_base_transform_chain (gstbasetransform.c:2369)


This happens because, header length is also included in the calculations. Fixing the same
Comment 4 Vineeth 2015-12-16 04:06:41 UTC
Created attachment 317467 [details] [review]
Fix logic to remove memory mishandlings.

fix spelling mistake in commit description. pnmdec-->pnmenc
Comment 5 Sebastian Dröge (slomo) 2015-12-16 08:56:12 UTC
Comment on attachment 317465 [details] [review]
pnmenc: Fix string leak

commit 6cb6903f82316aa4b00d00001a918e9949d9fd4e
Author: Vineeth TM <vineeth.tm@samsung.com>
Date:   Wed Dec 16 09:05:42 2015 +0900

    pnmenc: Fix string memory leak
    
    header being allocated is not freed resulting in leak
    
    https://bugzilla.gnome.org/show_bug.cgi?id=759520
Comment 6 Sebastian Dröge (slomo) 2015-12-16 08:56:54 UTC
Review of attachment 317467 [details] [review]:

::: gst/pnm/gstpnmenc.c
@@ +203,3 @@
   if (pnmenc->info.encoding == GST_PNM_ENCODING_ASCII) {
     /* Per component 4 bytes are used in case of ASCII encoding */
+    size = size * (4 + 1 / 20);

This does not seem correct. 1/20 is always 0, so this is always size*4.
Comment 7 Vineeth 2015-12-16 23:54:39 UTC
Created attachment 317538 [details] [review]
Fix logic to remove memory mishandlings.

Indeed it was wrong :)..

updated the patch properly now.
Comment 8 Sebastian Dröge (slomo) 2015-12-17 09:28:04 UTC
commit 10ed707b6085e3edc14e897ba53dbd321e4f557b
Author: Vineeth TM <vineeth.tm@samsung.com>
Date:   Thu Dec 17 08:51:48 2015 +0900

    pnmenc: Fix wrong logic leading to memory mishandling
    
    While encoding the frame in ASCII mode, per component four bytes are needed
    and after every 20 bytes, a \n will be added. So the calculation should be
    size = size * (4 + 1 / 20). This should exclude the header being written.
    Since header is also being included in the calculations, memory mishandlings
    are happening.
    
    https://bugzilla.gnome.org/show_bug.cgi?id=759520