Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Jump to bottom

Fix OMEMO leaks #1131

Closed
jubalh opened this issue Jun 20, 2019 · 5 comments
Closed

Fix OMEMO leaks #1131

jubalh opened this issue Jun 20, 2019 · 5 comments
Labels
bug OMEMO
Milestone
0.7.0

Comments

@jubalh
Copy link
Member

jubalh commented Jun 20, 2019 •
edited
Loading

Creating this issue so we don't forget to fix the OMEMO leaks before releasing 0.7.0.

Conversation started in #1130

@paulfariello :-)
@jubalh jubalh added the bug label Jun 20, 2019
@jubalh jubalh added this to the 0.7.0 milestone Jun 20, 2019
@jubalh
Copy link
Member Author

jubalh commented Jun 21, 2019 

==26554== 8,147 (352 direct, 7,795 indirect) bytes in 4 blocks are definitely lost in loss record 3,642 of 3,682                                              
==26554==    at 0x483677F: malloc (in /usr/lib64/valgrind/vgpreload_memcheck-amd64-linux.so)                                                                  
==26554==    by 0x548C6A8: g_malloc (in /usr/lib64/libglib-2.0.so.0.6000.3)                                                                                   
==26554==    by 0x54A44A1: g_slice_alloc (in /usr/lib64/libglib-2.0.so.0.6000.3)                                                                              
==26554==    by 0x547466D: g_hash_table_new_full (in /usr/lib64/libglib-2.0.so.0.6000.3)                                                                      
==26554==    by 0x4AF22A: _load_sessions (omemo.c:1519)                                                                                                       
==26554==    by 0x4ABE3D: omemo_on_connect (omemo.c:279)                                                                                                      
==26554==    by 0x44744D: sv_ev_login_account_success (server_events.c:88)                                                                                    
==26554==    by 0x43244B: session_login_success (session.c:289)                                                                                               
==26554==    by 0x433978: _connection_handler (connection.c:504)                                                                                              
==26554==    by 0x5AEBED5: ??? (in /usr/lib64/libmesode.so.0.0.0)                                                                                             
==26554==    by 0x5AF103E: ??? (in /usr/lib64/libmesode.so.0.0.0)                                                                                             
==26554==    by 0x5AEDBDA: ??? (in /usr/lib64/libmesode.so.0.0.0)                                                                                             
==26554==    by 0x5AFA43E: ??? (in /usr/lib64/libmesode.so.0.0.0)                                                                                             
==26554==    by 0x681850D: ??? (in /usr/lib64/libexpat.so.1.6.8)                                                                                              
==26554==    by 0x681A3AB: ??? (in /usr/lib64/libexpat.so.1.6.8)                                                                                              
==26554==    by 0x681D7EB: XML_ParseBuffer (in /usr/lib64/libexpat.so.1.6.8)                                                                                  
==26554==    by 0x5AF0A63: xmpp_run_once (in /usr/lib64/libmesode.so.0.0.0)                                                                                   
==26554==    by 0x432E48: connection_check_events (connection.c:104)                                                                                          
==26554==    by 0x43239E: session_process_events (session.c:255)                                                                                              
==26554==    by 0x42C087: prof_run (profanity.c:128)                                                                                                          
==26554==    by 0x4B24A7: main (main.c:172) 

==26554== 32 bytes in 1 blocks are definitely lost in loss record 1,466 of 3,682                                                                              
==26554==    at 0x483677F: malloc (in /usr/lib64/valgrind/vgpreload_memcheck-amd64-linux.so)                                                                  
==26554==    by 0x4B156F: omemo_start_device_session_handle_bundle (omemo.c:163)                                                                              
==26554==    by 0x434049: _iq_handler (iq.c:214)                                                                                                              
==26554==    by 0x5AF118E: ??? (in /usr/lib64/libmesode.so.0.0.0)                                                                                             
==26554==    by 0x5AEDBDA: ??? (in /usr/lib64/libmesode.so.0.0.0)                                                                                             
==26554==    by 0x5AFA43E: ??? (in /usr/lib64/libmesode.so.0.0.0)                                                                                             
==26554==    by 0x6818AA4: ??? (in /usr/lib64/libexpat.so.1.6.8)                                                                                              
==26554==    by 0x681A3AB: ??? (in /usr/lib64/libexpat.so.1.6.8)                                                                                              
==26554==    by 0x681D7EB: XML_ParseBuffer (in /usr/lib64/libexpat.so.1.6.8)                                                                                  
==26554==    by 0x5AF0A63: xmpp_run_once (in /usr/lib64/libmesode.so.0.0.0)                                                                                   
==26554==    by 0x432E48: connection_check_events (connection.c:104)                                                                                          
==26554==    by 0x43239E: session_process_events (session.c:255)                                                                                              
==26554==    by 0x42C087: prof_run (profanity.c:128)                                                                                                          
==26554==    by 0x4B24A7: main (main.c:172)

Another one seems to be about iq_id_handler_add():

==26554== 148 (96 direct, 52 indirect) bytes in 4 blocks are definitely lost in loss record 3,184 of 3,682                                                    
==26554==    at 0x483677F: malloc (in /usr/lib64/valgrind/vgpreload_memcheck-amd64-linux.so)                                                                  
==26554==    by 0x434233: iq_id_handler_add (iq.c:265)                                                                                                        
==26554==    by 0x4B1360: omemo_bundle_request (omemo.c:103)                                                                                                  
==26554==    by 0x4AE77F: _handle_own_device_list (omemo.c:1265)                                                                                              
==26554==    by 0x4AC8AB: omemo_set_device_list (omemo.c:533)                                                                                                 
==26554==    by 0x4B1E41: _omemo_receive_devicelist (omemo.c:369)                                                                                             
==26554==    by 0x434049: _iq_handler (iq.c:214)                                                                                                              
==26554==    by 0x5AF118E: ??? (in /usr/lib64/libmesode.so.0.0.0)                                                                                             
==26554==    by 0x5AEDBDA: ??? (in /usr/lib64/libmesode.so.0.0.0)                                                                                             
==26554==    by 0x5AFA43E: ??? (in /usr/lib64/libmesode.so.0.0.0)                                                                                             
==26554==    by 0x6818AA4: ??? (in /usr/lib64/libexpat.so.1.6.8)                                                                                              
==26554==    by 0x681A3AB: ??? (in /usr/lib64/libexpat.so.1.6.8)                                                                                              
==26554==    by 0x681D7EB: XML_ParseBuffer (in /usr/lib64/libexpat.so.1.6.8)                                                                                  
==26554==    by 0x5AF0A63: xmpp_run_once (in /usr/lib64/libmesode.so.0.0.0)                                                                                   
==26554==    by 0x432E48: connection_check_events (connection.c:104)                                                                                          
==26554==    by 0x43239E: session_process_events (session.c:255)                                                                                              
==26554==    by 0x42C087: prof_run (profanity.c:128)                                                                                                          
==26554==    by 0x4B24A7: main (main.c:172)

@bitkeks
Copy link

bitkeks commented Jun 26, 2019

Adding from 0.6.0dev.master.165602e5 valgrind logs:

==02:17:04:33.711 9726== 386 bytes in 1 blocks are definitely lost in loss record 468 of 569
==02:17:04:33.711 9726==    at 0x483877F: malloc (vg_replace_malloc.c:299)
==02:17:04:33.711 9726==    by 0x1A17E4: signal_buffer_alloc 
==02:17:04:33.711 9726==    by 0x1A1811: signal_buffer_create
==02:17:04:33.711 9726==    by 0x19E20E: _load_sessions (omemo.c:1530)
==02:17:04:33.711 9726==    by 0x19A86E: omemo_on_connect (omemo.c:279)
==02:17:04:33.711 9726==    by 0x156640: sv_ev_login_account_success (server_events.c:88)
==02:17:04:33.711 9726==    by 0x1462A7: session_login_success (session.c:289)
==02:17:04:33.712 9726==    by 0x4F47496: ??? (in /usr/lib/libmesode.so.0.0.0)
==02:17:04:33.712 9726==    by 0x4F4C6AE: ??? (in /usr/lib/libmesode.so.0.0.0)
==02:17:04:33.712 9726==    by 0x4F4924B: ??? (in /usr/lib/libmesode.so.0.0.0)
==02:17:04:33.712 9726==    by 0x4F55BEE: ??? (in /usr/lib/libmesode.so.0.0.0)
==02:17:04:33.712 9726==    by 0x601B30E: ??? (in /usr/lib/libexpat.so.1.6.8)
==02:17:04:33.712 9726==
==02:17:04:33.712 9726== 388 bytes in 1 blocks are definitely lost in loss record 469 of 569
==02:17:04:33.712 9726==    at 0x483877F: malloc (vg_replace_malloc.c:299)
==02:17:04:33.712 9726==    by 0x1A17E4: signal_buffer_alloc 
==02:17:04:33.712 9726==    by 0x1A1811: signal_buffer_create
==02:17:04:33.712 9726==    by 0x19F707: store_session (store.c:140)
==02:17:04:33.712 9726==    by 0x1A2A80: signal_protocol_session_store_session 
==02:17:04:33.712 9726==    by 0x1ACA39: session_cipher_encrypt 
==02:17:04:33.712 9726==    by 0x19BFF1: omemo_on_message_send (omemo.c:792)
==02:17:04:33.712 9726==    by 0x1584C7: cl_ev_send_msg (client_events.c:212)
==02:17:04:33.712 9726==    by 0x170F6F: _cmd_execute_default (cmd_funcs.c:7961)
==02:17:04:33.712 9726==    by 0x171A1F: cmd_process_input (cmd_funcs.c:145)
==02:17:04:33.712 9726==    by 0x1411EF: prof_run (profanity.c:116)
==02:17:04:33.712 9726==    by 0x1A1761: main (main.c:172)

==02:17:04:33.712 9726== 438 (208 direct, 230 indirect) bytes in 1 blocks are definitely lost in loss record 474 of 569
==02:17:04:33.712 9726==    at 0x483877F: malloc (vg_replace_malloc.c:299)
==02:17:04:33.712 9726==    by 0x18E11B: account_new (account.c:59)
==02:17:04:33.712 9726==    by 0x18B8C8: accounts_get_account (accounts.c:328)
==02:17:04:33.712 9726==    by 0x19D62C: omemo_automatic_start (omemo.c:1307)
==02:17:04:33.712 9726==    by 0x1485B8: _room_info_response_id_handler (iq.c:2121)
==02:17:04:33.712 9726==    by 0x14A8C0: _iq_handler (iq.c:214)
==02:17:04:33.712 9726==    by 0x14A8C0: _iq_handler (iq.c:150)
==02:17:04:33.712 9726==    by 0x4F4C818: ??? (in /usr/lib/libmesode.so.0.0.0)
==02:17:04:33.712 9726==    by 0x4F4924B: ??? (in /usr/lib/libmesode.so.0.0.0)
==02:17:04:33.712 9726==    by 0x4F55BEE: ??? (in /usr/lib/libmesode.so.0.0.0)
==02:17:04:33.712 9726==    by 0x601B926: ??? (in /usr/lib/libexpat.so.1.6.8)
==02:17:04:33.712 9726==    by 0x601C3CB: ??? (in /usr/lib/libexpat.so.1.6.8)
==02:17:04:33.712 9726==    by 0x601E945: XML_ParseBuffer (in /usr/lib/libexpat.so.1.6.8)

==02:17:04:33.712 9726== 720 bytes in 30 blocks are definitely lost in loss record 489 of 569
==02:17:04:33.712 9726==    at 0x483877F: malloc (vg_replace_malloc.c:299)
==02:17:04:33.712 9726==    by 0x14A2CD: iq_id_handler_add (iq.c:265)
==02:17:04:33.712 9726==    by 0x1A0238: omemo_devicelist_request (omemo.c:46)
==02:17:04:33.712 9726==    by 0x19AD99: omemo_start_session (omemo.c:401)
==02:17:04:33.712 9726==    by 0x19AD00: omemo_start_sessions (omemo.c:388)
==02:17:04:33.712 9726==    by 0x1567AB: sv_ev_roster_received (server_events.c:191)
==02:17:04:33.712 9726==    by 0x14A8FF: _iq_handler (iq.c:202)
==02:17:04:33.712 9726==    by 0x14A8FF: _iq_handler (iq.c:150)
==02:17:04:33.712 9726==    by 0x4F4C818: ??? (in /usr/lib/libmesode.so.0.0.0)
==02:17:04:33.712 9726==    by 0x4F4924B: ??? (in /usr/lib/libmesode.so.0.0.0)
==02:17:04:33.712 9726==    by 0x4F55BEE: ??? (in /usr/lib/libmesode.so.0.0.0)
==02:17:04:33.712 9726==    by 0x601B926: ??? (in /usr/lib/libexpat.so.1.6.8)
==02:17:04:33.712 9726==    by 0x601C3CB: ??? (in /usr/lib/libexpat.so.1.6.8)

==02:17:04:33.712 9726== 913 (480 direct, 433 indirect) bytes in 20 blocks are definitely lost in loss record 494 of 569
==02:17:04:33.712 9726==    at 0x483877F: malloc (vg_replace_malloc.c:299)
==02:17:04:33.712 9726==    by 0x14A2CD: iq_id_handler_add (iq.c:265)
==02:17:04:33.712 9726==    by 0x1A0518: omemo_bundle_request (omemo.c:103)
==02:17:04:33.712 9726==    by 0x19AE05: omemo_start_session (omemo.c:408)
==02:17:04:33.712 9726==    by 0x19D586: _handle_device_list_start_session (omemo.c:1274)
==02:17:04:33.712 9726==    by 0x19B3D4: omemo_set_device_list (omemo.c:533)
==02:17:04:33.712 9726==    by 0x1A10CC: _omemo_receive_devicelist (omemo.c:369)
==02:17:04:33.712 9726==    by 0x14A8C0: _iq_handler (iq.c:214)
==02:17:04:33.712 9726==    by 0x14A8C0: _iq_handler (iq.c:150)
==02:17:04:33.712 9726==    by 0x4F4C818: ??? (in /usr/lib/libmesode.so.0.0.0)
==02:17:04:33.712 9726==    by 0x4F4924B: ??? (in /usr/lib/libmesode.so.0.0.0)
==02:17:04:33.712 9726==    by 0x4F55BEE: ??? (in /usr/lib/libmesode.so.0.0.0)
==02:17:04:33.712 9726==    by 0x601B926: ??? (in /usr/lib/libexpat.so.1.6.8)

==02:17:04:33.712 9726== 1,690 bytes in 26 blocks are definitely lost in loss record 502 of 569
==02:17:04:33.712 9726==    at 0x483877F: malloc (vg_replace_malloc.c:299)
==02:17:04:33.712 9726==    by 0x50076FE: strdup (in /usr/lib/libc-2.29.so)
==02:17:04:33.712 9726==    by 0x19E44A: _load_known_devices (omemo.c:1560)
==02:17:04:33.712 9726==    by 0x19A8E4: omemo_on_connect (omemo.c:286)
==02:17:04:33.712 9726==    by 0x156640: sv_ev_login_account_success (server_events.c:88)
==02:17:04:33.712 9726==    by 0x1462A7: session_login_success (session.c:289)
==02:17:04:33.712 9726==    by 0x4F47496: ??? (in /usr/lib/libmesode.so.0.0.0)
==02:17:04:33.712 9726==    by 0x4F4C6AE: ??? (in /usr/lib/libmesode.so.0.0.0)
==02:17:04:33.712 9726==    by 0x4F4924B: ??? (in /usr/lib/libmesode.so.0.0.0)
==02:17:04:33.712 9726==    by 0x4F55BEE: ??? (in /usr/lib/libmesode.so.0.0.0)
==02:17:04:33.712 9726==    by 0x601B30E: ??? (in /usr/lib/libexpat.so.1.6.8)
==02:17:04:33.712 9726==    by 0x601C3CB: ??? (in /usr/lib/libexpat.so.1.6.8)

==02:17:04:33.712 9726== 3,128 (1,496 direct, 1,632 indirect) bytes in 17 blocks are definitely lost in loss record 513 of 569
==02:17:04:33.712 9726==    at 0x483877F: malloc (vg_replace_malloc.c:299)
==02:17:04:33.712 9726==    by 0x4AD1AB1: g_malloc (in /usr/lib/libglib-2.0.so.0.6000.2)
==02:17:04:33.712 9726==    by 0x4AB18E4: g_slice_alloc (in /usr/lib/libglib-2.0.so.0.6000.2)
==02:17:04:33.712 9726==    by 0x4AEBB7E: g_hash_table_new_full (in /usr/lib/libglib-2.0.so.0.6000.2)
==02:17:04:33.712 9726==    by 0x19E34A: _load_known_devices (omemo.c:1552)
==02:17:04:33.712 9726==    by 0x19A8E4: omemo_on_connect (omemo.c:286)
==02:17:04:33.712 9726==    by 0x156640: sv_ev_login_account_success (server_events.c:88)
==02:17:04:33.712 9726==    by 0x1462A7: session_login_success (session.c:289)
==02:17:04:33.712 9726==    by 0x4F47496: ??? (in /usr/lib/libmesode.so.0.0.0)
==02:17:04:33.712 9726==    by 0x4F4C6AE: ??? (in /usr/lib/libmesode.so.0.0.0)
==02:17:04:33.712 9726==    by 0x4F4924B: ??? (in /usr/lib/libmesode.so.0.0.0)
==02:17:04:33.712 9726==    by 0x4F55BEE: ??? (in /usr/lib/libmesode.so.0.0.0)

@bitkeks bitkeks mentioned this issue Jun 26, 2019
Closed
@jubalh
Copy link
Member Author

jubalh commented Jul 4, 2019

In 46ecdc3 I updated the supressions file so that more false positives are catched.
Let's keep in mind that there are still some upstream glib issues in glib depending on which version you run.

@jubalh
Copy link
Member Author

jubalh commented Jul 4, 2019

omemo_init() and omemo_start_device_session() create a new autocompleter via omemo_ctx.fingerprint_ac = autocomplete_new();.
This needs to be freed via autocomplete_clear().

@paulfariello what is the right place to do this? omemo_on_disconnect() might be one but I'm not entirely sure. Maybe there needs to be a cleanup function.

==13226== 24 bytes in 1 blocks are definitely lost in loss record 2,855 of 6,958
==13226==    at 0x483677F: malloc (in /usr/lib64/valgrind/vgpreload_memcheck-amd64-linux.so)
==13226==    by 0x48AD39: autocomplete_new (autocomplete.c:57)
==13226==    by 0x4AB89F: omemo_init (omemo.c:127)
==13226==    by 0x42C283: _init (profanity.c:206)
==13226==    by 0x42BFF3: prof_run (profanity.c:98)
==13226==    by 0x4B25E6: main (main.c:172)

jubalh added a commit that referenced this issue Jul 4, 2019
@jubalh
Add omemo_close function
199162b 
We call omemo_init() when starting profanity and should have an
omemo_close() at exit.

For now we free the fingerprint autocompleter in there.

Fixes valgrind:
```
==13226== 24 bytes in 1 blocks are definitely lost in loss record 2,855
of 6,958
==13226==    at 0x483677F: malloc (in
/usr/lib64/valgrind/vgpreload_memcheck-amd64-linux.so)
==13226==    by 0x48AD39: autocomplete_new (autocomplete.c:57)
==13226==    by 0x4AB89F: omemo_init (omemo.c:127)
==13226==    by 0x42C283: _init (profanity.c:206)
==13226==    by 0x42BFF3: prof_run (profanity.c:98)
==13226==    by 0x4B25E6: main (main.c:172)
```

Regards #1131
@jubalh jubalh mentioned this issue Jul 4, 2019
Merged
@jubalh jubalh added the OMEMO label Jul 4, 2019
@jubalh
Copy link
Member Author

jubalh commented Jul 4, 2019

Several of the leaks I get are actually no from us!

Now that I use glib 2.60.4 I have less leaks than with 2.60.2.
Also libexpat has one: libexpat/libexpat#186 which causes many leaks for us.

libexpat 2.2.7, which came out recently, fixes that.

jubalh added a commit that referenced this issue Jul 11, 2019
@jubalh
Merge pull request #1150 from paulfariello/hotfix/omemo-memleaks
ad41959 
Hotfix/omemo memleaks

Regards #1131
@jubalh jubalh closed this as completed Jul 11, 2019
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Assignees
No one assigned
Labels
bug OMEMO
Projects
None yet
Milestone
0.7.0
Development

No branches or pull requests
2 participants
@jubalh @bitkeks