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.

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

configuration: Fix addr/notify flag string leaks. #246

Merged

Conversation

ossama-othman
Copy link
Member

Fix memory leaks that occured when parsing the "addr-flags" and
"notify-flags" entries in the mptcpd configuration file. The string
returned from the ELL l_settings_get_string() function is dynamically
allocated and must be deallocated with l_free().

Reported-by: Mat Martineau mathew.j.martineau@linux.intel.com

Fix memory leaks that occured when parsing the "addr-flags" and
"notify-flags" entries in the mptcpd configuration file.  The string
returned from the ELL l_settings_get_string() function is dynamically
allocated and must be deallocated with l_free().

Reported-by: Mat Martineau <mathew.j.martineau@linux.intel.com>
@ossama-othman ossama-othman added the bug Something isn't working label Jul 7, 2022
@ossama-othman ossama-othman self-assigned this Jul 7, 2022
@ossama-othman
Copy link
Member Author

Steps to reproduce:

  1. ./bootstrap
  2. ./configure --prefix=/usr/local
  3. make
  4. sudo make install
  5. sudo vi /usr/local/etc/mptcpd/mptcpd.conf # uncomment addr-flags and notify-flags entries
  6. LD_LIBRARY_PATH=/usr/local/lib valgrind --leak-check=full /usr/local/libexec/mptcpd
  7. CTRL-c # Stop mptcpd
  8. sudo make uninstall

Leaks detected by Valgrind before applying patch:

==753501== HEAP SUMMARY:
==753501==     in use at exit: 47 bytes in 2 blocks
==753501==   total heap usage: 229 allocs, 227 frees, 58,188 bytes allocated
==753501== 
==753501== 8 bytes in 1 blocks are definitely lost in loss record 1 of 2
==753501==    at 0x4848899: malloc (in /usr/libexec/valgrind/vgpreload_memcheck-amd64-linux.so)
==753501==    by 0x487AA91: l_malloc (util.c:62)
==753501==    by 0x487E508: unescape_value (settings.c:196)
==753501==    by 0x10C3A8: parse_config_addr_flags (configuration.c:573)
==753501==    by 0x10C512: parse_config_file (configuration.c:675)
==753501==    by 0x10C5C9: parse_config_files (configuration.c:723)
==753501==    by 0x10C9C4: mptcpd_config_create (configuration.c:823)
==753501==    by 0x10BC19: main (mptcpd.c:51)
==753501== 
==753501== 39 bytes in 1 blocks are definitely lost in loss record 2 of 2
==753501==    at 0x4848899: malloc (in /usr/libexec/valgrind/vgpreload_memcheck-amd64-linux.so)
==753501==    by 0x487AA91: l_malloc (util.c:62)
==753501==    by 0x487E508: unescape_value (settings.c:196)
==753501==    by 0x10C338: parse_config_notify_flags (configuration.c:592)
==753501==    by 0x10C504: parse_config_file (configuration.c:672)
==753501==    by 0x10C5C9: parse_config_files (configuration.c:723)
==753501==    by 0x10C9C4: mptcpd_config_create (configuration.c:823)
==753501==    by 0x10BC19: main (mptcpd.c:51)
==753501== 
==753501== LEAK SUMMARY:
==753501==    definitely lost: 47 bytes in 2 blocks
==753501==    indirectly lost: 0 bytes in 0 blocks
==753501==      possibly lost: 0 bytes in 0 blocks
==753501==    still reachable: 0 bytes in 0 blocks
==753501==         suppressed: 0 bytes in 0 blocks
==753501== 
==753501== For lists of detected and suppressed errors, rerun with: -s
==753501== ERROR SUMMARY: 2 errors from 2 contexts (suppressed: 0 from 0)

Valgrind results after applying the patch:

==754343== 
==754343== HEAP SUMMARY:
==754343==     in use at exit: 0 bytes in 0 blocks
==754343==   total heap usage: 231 allocs, 231 frees, 58,220 bytes allocated
==754343== 
==754343== All heap blocks were freed -- no leaks are possible
==754343== 
==754343== For lists of detected and suppressed errors, rerun with: -s
==754343== ERROR SUMMARY: 0 errors from 0 contexts (suppressed: 0 from 0)

@coveralls
Copy link

coveralls commented Jul 7, 2022

Pull Request Test Coverage Report for Build 2627152348

Warning: This coverage report may be inaccurate.

This pull request's base commit is no longer the HEAD commit of its target branch. This means it includes changes from outside the original pull request, including, potentially, unrelated coverage changes.

Details

  • 0 of 4 (0.0%) changed or added relevant lines in 1 file are covered.
  • No unchanged relevant lines lost coverage.
  • Overall coverage decreased (-0.07%) to 62.277%

Changes Missing Coverage Covered Lines Changed/Added Lines %
src/configuration.c 0 4 0.0%
Totals Coverage Status
Change from base Build 2626644661: -0.07%
Covered Lines: 1187
Relevant Lines: 1906

💛 - Coveralls

@ossama-othman ossama-othman merged commit 88bea68 into multipath-tcp:master Jul 8, 2022
@ossama-othman ossama-othman deleted the config-flags-leak branch July 8, 2022 02:59
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
bug Something isn't working
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants