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 758039 - gstgl: fix memory leaks and memory mishandlings
gstgl: fix memory leaks and memory mishandlings
Status: RESOLVED FIXED
Product: GStreamer
Classification: Platform
Component: gst-plugins-bad
git master
Other Linux
: Normal normal
: git master
Assigned To: GStreamer Maintainers
GStreamer Maintainers
Depends on:
Blocks:
 
 
Reported: 2015-11-13 01:41 UTC by Vineeth
Modified: 2015-11-13 06:10 UTC
See Also:
GNOME target: ---
GNOME version: ---


Attachments
glsl: fix wrong memory access (1.40 KB, patch)
2015-11-13 01:44 UTC, Vineeth
committed Details | Review
tests: glsl version_profile_s string leak (820 bytes, patch)
2015-11-13 01:45 UTC, Vineeth
committed Details | Review
fix string leak (935 bytes, patch)
2015-11-13 01:57 UTC, Vineeth
committed Details | Review
glupload: fix caps memory leak (914 bytes, patch)
2015-11-13 02:05 UTC, Vineeth
committed Details | Review
glshader: Fix illegal access of memory (1.09 KB, patch)
2015-11-13 02:25 UTC, Vineeth
rejected Details | Review

Description Vineeth 2015-11-13 01:41:05 UTC
Fixing few leaks and mishandling in gl
Comment 1 Vineeth 2015-11-13 01:44:01 UTC
Created attachment 315379 [details] [review]
glsl: fix wrong memory access

trying to access string location which does not exist.

==3492== Invalid read of size 1
==3492==    at 0x402F3E8: strlen (in /usr/lib/valgrind/vgpreload_memcheck-x86-linux.so)
==3492==    by 0x425035F: g_strchomp (in /lib/i386-linux-gnu/libglib-2.0.so.0.4002.0)
==3492==    by 0x4056F3E: gst_glsl_version_profile_from_string (gstglsl.c:279)
==3492==    by 0x8049109: test_serialization (gstglsl.c:284)
==3492==    by 0x409CBAA: srunner_run (check_run.c:450)
==3492==    by 0x409CC62: srunner_run_all (check_run.c:674)
==3492==    by 0x408FA97: gst_check_run_suite (gstcheck.c:825)
==3492==    by 0x8048BEC: main (gstglsl.c:308)
==3492==  Address 0x514642c is 0 bytes after a block of size 4 alloc'd
==3492==    at 0x402C17C: malloc (in /usr/lib/valgrind/vgpreload_memcheck-x86-linux.so)
==3492==    by 0x4236BE2: g_malloc (in /lib/i386-linux-gnu/libglib-2.0.so.0.4002.0)
==3492==    by 0x424EE1C: g_strdup (in /lib/i386-linux-gnu/libglib-2.0.so.0.4002.0)
==3492==    by 0x4056E75: gst_glsl_version_profile_from_string (gstglsl.c:255)
==3492==    by 0x8049109: test_serialization (gstglsl.c:284)
==3492==    by 0x409CBAA: srunner_run (check_run.c:450)
==3492==    by 0x409CC62: srunner_run_all (check_run.c:674)
==3492==    by 0x408FA97: gst_check_run_suite (gstcheck.c:825)
==3492==    by 0x8048BEC: main (gstglsl.c:308)
Comment 2 Vineeth 2015-11-13 01:45:27 UTC
Created attachment 315380 [details] [review]
tests: glsl version_profile_s string leak

==5039== 210 bytes in 17 blocks are definitely lost in loss record 1,445 of 1,475
==5039==    at 0x402C17C: malloc (in /usr/lib/valgrind/vgpreload_memcheck-x86-linux.so)
==5039==    by 0x440E2AF: __vasprintf_chk (vasprintf_chk.c:80)
==5039==    by 0x4275689: g_vasprintf (in /lib/i386-linux-gnu/libglib-2.0.so.0.4002.0)
==5039==    by 0x424EFF2: g_strdup_vprintf (in /lib/i386-linux-gnu/libglib-2.0.so.0.4002.0)
==5039==    by 0x424F022: g_strdup_printf (in /lib/i386-linux-gnu/libglib-2.0.so.0.4002.0)
==5039==    by 0x4056E03: gst_glsl_version_profile_to_string (gstglsl.c:204)
==5039==    by 0x8048E80: test_serialization (gstglsl.c:198)
==5039==    by 0x409CBAA: srunner_run (check_run.c:450)
==5039==    by 0x409CC62: srunner_run_all (check_run.c:674)
==5039==    by 0x408FA97: gst_check_run_suite (gstcheck.c:825)
==5039==    by 0x8048BEC: main (gstglsl.c:307)
Comment 3 Vineeth 2015-11-13 01:57:18 UTC
Created attachment 315381 [details] [review]
fix string leak

str is not getting freed while returning error.
Comment 4 Vineeth 2015-11-13 02:05:37 UTC
Created attachment 315382 [details] [review]
glupload: fix caps memory leak

==15122== 321 (40 direct, 281 indirect) bytes in 1 blocks are definitely lost in loss record 2,813 of 3,003
==15122==    at 0x402C17C: malloc (in /usr/lib/valgrind/vgpreload_memcheck-x86-linux.so)
==15122==    by 0x42A8BE2: g_malloc (in /lib/i386-linux-gnu/libglib-2.0.so.0.4002.0)
==15122==    by 0x42BF281: g_slice_alloc (in /lib/i386-linux-gnu/libglib-2.0.so.0.4002.0)
==15122==    by 0x41517CB: gst_caps_new_empty (gstcaps.c:243)
==15122==    by 0x4154EE2: gst_caps_from_string (gstcaps.c:2358)
==15122==    by 0x8049C43: test_upload_buffer (gstglupload.c:204)
==15122==    by 0x410EBAA: srunner_run (check_run.c:450)
==15122==    by 0x410EC62: srunner_run_all (check_run.c:674)
==15122==    by 0x4101A97: gst_check_run_suite (gstcheck.c:825)
==15122==    by 0x804944C: main (gstglupload.c:320)
==15122== 
==15122== 321 (40 direct, 281 indirect) bytes in 1 blocks are definitely lost in loss record 2,814 of 3,003
==15122==    at 0x402C17C: malloc (in /usr/lib/valgrind/vgpreload_memcheck-x86-linux.so)
==15122==    by 0x42A8BE2: g_malloc (in /lib/i386-linux-gnu/libglib-2.0.so.0.4002.0)
==15122==    by 0x42BF281: g_slice_alloc (in /lib/i386-linux-gnu/libglib-2.0.so.0.4002.0)
==15122==    by 0x41517CB: gst_caps_new_empty (gstcaps.c:243)
==15122==    by 0x4154EE2: gst_caps_from_string (gstcaps.c:2358)
==15122==    by 0x8049CFA: test_upload_buffer (gstglupload.c:222)
==15122==    by 0x410EBAA: srunner_run (check_run.c:450)
==15122==    by 0x410EC62: srunner_run_all (check_run.c:674)
==15122==    by 0x4101A97: gst_check_run_suite (gstcheck.c:825)
==15122==    by 0x804944C: main (gstglupload.c:320)
Comment 5 Vineeth 2015-11-13 02:25:02 UTC
Created attachment 315383 [details] [review]
glshader: Fix illegal access of memory

Due to illegal memory access in for loop, get the below error

==10034== Thread 3 gstglcontext:
==10034== Invalid read of size 4
==10034==    at 0x40557E6: gst_gl_shader_release_unlocked (gstglshader.c:671)
==10034==    by 0x405587B: _cleanup_shader (gstglshader.c:96)
==10034==    by 0x404713F: _gst_gl_context_thread_run_generic (gstglcontext.c:1456)
==10034==    by 0x406023A: _run_message_sync (gstglwindow.c:628)
==10034==    by 0x40601A1: _run_message_async (gstglwindow.c:697)
==10034==    by 0x42A0C4F: ??? (in /lib/i386-linux-gnu/libglib-2.0.so.0.4002.0)
==10034==    by 0x42A40A6: g_main_context_dispatch (in /lib/i386-linux-gnu/libglib-2.0.so.0.4002.0)
==10034==    by 0x42A4467: ??? (in /lib/i386-linux-gnu/libglib-2.0.so.0.4002.0)
==10034==    by 0x42A476A: g_main_loop_run (in /lib/i386-linux-gnu/libglib-2.0.so.0.4002.0)
==10034==    by 0x40602B3: gst_gl_window_default_run (gstglwindow.c:537)
==10034==    by 0x40609B8: gst_gl_window_run (gstglwindow.c:558)
==10034==    by 0x4049D97: gst_gl_context_create_thread (gstglcontext.c:1224)
==10034==    by 0x42CA9A9: ??? (in /lib/i386-linux-gnu/libglib-2.0.so.0.4002.0)
==10034==    by 0x4370F6F: start_thread (pthread_create.c:312)
==10034==    by 0x4471BED: clone (clone.S:129)
==10034==  Address 0x534bc7c is 4 bytes inside a block of size 12 free'd
==10034==    at 0x402D3D8: free (in /usr/lib/valgrind/vgpreload_memcheck-x86-linux.so)
==10034==    by 0x42A9D2F: g_free (in /lib/i386-linux-gnu/libglib-2.0.so.0.4002.0)
==10034==    by 0x42C087A: g_slice_free1 (in /lib/i386-linux-gnu/libglib-2.0.so.0.4002.0)
==10034==    by 0x42A0056: g_list_delete_link (in /lib/i386-linux-gnu/libglib-2.0.so.0.4002.0)
==10034==    by 0x4054B6B: gst_gl_shader_detach_unlocked (gstglshader.c:423)
==10034==    by 0x40557E5: gst_gl_shader_release_unlocked (gstglshader.c:674)
==10034==    by 0x405587B: _cleanup_shader (gstglshader.c:96)
==10034==    by 0x404713F: _gst_gl_context_thread_run_generic (gstglcontext.c:1456)
==10034==    by 0x406023A: _run_message_sync (gstglwindow.c:628)
==10034==    by 0x40601A1: _run_message_async (gstglwindow.c:697)
==10034==    by 0x42A0C4F: ??? (in /lib/i386-linux-gnu/libglib-2.0.so.0.4002.0)
==10034==    by 0x42A40A6: g_main_context_dispatch (in /lib/i386-linux-gnu/libglib-2.0.so.0.4002.0)
==10034==    by 0x42A4467: ??? (in /lib/i386-linux-gnu/libglib-2.0.so.0.4002.0)
==10034==    by 0x42A476A: g_main_loop_run (in /lib/i386-linux-gnu/libglib-2.0.so.0.4002.0)
==10034==    by 0x40602B3: gst_gl_window_default_run (gstglwindow.c:537)
==10034==    by 0x40609B8: gst_gl_window_run (gstglwindow.c:558)
==10034==    by 0x4049D97: gst_gl_context_create_thread (gstglcontext.c:1224)
==10034==    by 0x42CA9A9: ??? (in /lib/i386-linux-gnu/libglib-2.0.so.0.4002.0)
==10034==    by 0x4370F6F: start_thread (pthread_create.c:312)
==10034==    by 0x4471BED: clone (clone.S:129)
Comment 6 Matthew Waters (ystreet00) 2015-11-13 06:07:53 UTC
commit 6eae0c7e189f69c71cc3b0cf00e4fcdec172a13e
Author: Vineeth TM <vineeth.tm@samsung.com>
Date:   Fri Nov 13 10:41:58 2015 +0900

    glsl: fix possible string overrun in gst_glsl_version_profile_from_string
    
    given a NULL-terminated string, s.
    s[i] = '\0';
    i++;
    does not guarentee that s[i] is NULL terminated and thus string operations
    could read off the end of the array.
    
    https://bugzilla.gnome.org/show_bug.cgi?id=758039

commit babd066b894fc3694693a2abb3cd0d8c6a66992e
Author: Matthew Waters <matthew@centricular.com>
Date:   Fri Nov 13 16:50:22 2015 +1100

    glshader: don't read invalid list pointers (use after free)
    
    gst_gl_shader_detach_unlocked already removes the list entry so attempting to
    use the element to iterate to the next stage could read invalid data.
    
    Based on patch by Vineeth TM <vineeth.tm@samsung.com>
    
    https://bugzilla.gnome.org/show_bug.cgi?id=758039

commit e940d8875d01210ecc6ac280d5a91a6856bcb8f2
Author: Vineeth TM <vineeth.tm@samsung.com>
Date:   Fri Nov 13 10:44:26 2015 +0900

    tests:glsl: version_profile_s string leak
    
    https://bugzilla.gnome.org/show_bug.cgi?id=758039

commit 4c9ac5e474e22a48e476f4b9983e21260798883f
Author: Vineeth TM <vineeth.tm@samsung.com>
Date:   Fri Nov 13 10:56:10 2015 +0900

    glsl: free str while returning error
    
    https://bugzilla.gnome.org/show_bug.cgi?id=758039

commit a22f085eac42c2aeced4800e1c3332b508e642a8
Author: Vineeth TM <vineeth.tm@samsung.com>
Date:   Fri Nov 13 11:04:34 2015 +0900

    tests:glupload: fix caps memory leak
    
    https://bugzilla.gnome.org/show_bug.cgi?id=758039
Comment 7 Matthew Waters (ystreet00) 2015-11-13 06:10:12 UTC
Comment on attachment 315383 [details] [review]
glshader: Fix illegal access of memory

Slightly modified version pushed instead as gst_gl_shader_detach_unlocked () may fail to actually remove the stage from the list of stages which could cause an infinite loop.