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

Fix memleaks in BSD syscollector #3795

Merged
merged 15 commits into from Aug 5, 2019
Merged

Fix memleaks in BSD syscollector #3795

merged 15 commits into from Aug 5, 2019

Conversation

JuantAldea
Copy link
Contributor

@JuantAldea JuantAldea commented Aug 2, 2019

Related issues
#3773 and #3785

Description

This PR fixes several memory leaks, some of them cherry-picked from #3773 and #3785.
Most of this leaks aren't detected by Memcheck.

Tests

  • Scan-build Reports one issue already reported on one the aforementioned PRs
  • Compilation without warnings in every supported platform
    • Linux
    • Windows
    • MAC OS X
  • Memory tests
    • Valgrind report for affected components
    • RAM usage impact

@BraulioV
Copy link
Contributor

BraulioV commented Aug 2, 2019

Hi all,

I've been testing the changes and they look really good. Good job!. In this image, you can see the memory used by the Wazuh Agent in macOS:
image

And this is the result after building a new agent with these changes:
image

As you can see, this is a huge improvement and the leaks seem solved. We are still analyzing the memory usage of wazuh-modulesd to discard any possible leak.

Regards.

@chemamartinez
Copy link
Contributor

Scan-build output

Previous bugs

Bug Group Bug Type File Function/Method Line Fixed in
Memory Error Memory leak wazuh_modules/syscollector/syscollector_bsd.c getGatewayList 1307 #3773
Memory Error Memory leak wazuh_modules/syscollector/syscollector_bsd.c sys_hw_bsd 572 70b3def
Memory Error Memory leak wazuh_modules/syscollector/syscollector_bsd.c sys_hw_bsd 586 5744ae0

These errors disappear in scan-build reports of this branch.

@chemamartinez
Copy link
Contributor

AddressSanitizer

Previous output

==89222==ERROR: AddressSanitizer: heap-buffer-overflow on address 0x602000006038 at pc 0x0001000eee05 bp 0x7000065455b0 sp 0x7000065455a8
WRITE of size 4 at 0x602000006038 thread T5
    #0 0x1000eee04 in getGatewayList syscollector_bsd.c:1295
    #1 0x1000b2779 in getPrimaryIP wm_control.c:76
    #2 0x1000b3a1f in send_ip wm_control.c:231
    #3 0x1000b3ad4 in wm_control_main wm_control.c:150
    #4 0x7fffa9c6093a in _pthread_body (libsystem_pthread.dylib:x86_64+0x393a)
    #5 0x7fffa9c60886 in _pthread_start (libsystem_pthread.dylib:x86_64+0x3886)
    #6 0x7fffa9c6008c in thread_start (libsystem_pthread.dylib:x86_64+0x308c)

0x602000006038 is located 0 bytes to the right of 8-byte region [0x602000006030,0x602000006038)
allocated by thread T5 here:
    #0 0x10078c4c0 in wrap_calloc (libclang_rt.asan_osx_dynamic.dylib:x86_64+0x564c0)
    #1 0x1000ee6e0 in getGatewayList syscollector_bsd.c:1256
    #2 0x1000b2779 in getPrimaryIP wm_control.c:76
    #3 0x1000b3a1f in send_ip wm_control.c:231
    #4 0x1000b3ad4 in wm_control_main wm_control.c:150
    #5 0x7fffa9c6093a in _pthread_body (libsystem_pthread.dylib:x86_64+0x393a)
    #6 0x7fffa9c60886 in _pthread_start (libsystem_pthread.dylib:x86_64+0x3886)
    #7 0x7fffa9c6008c in thread_start (libsystem_pthread.dylib:x86_64+0x308c)

Thread T5 created by T0 here:
    #0 0x1007837a6 in wrap_pthread_create (libclang_rt.asan_osx_dynamic.dylib:x86_64+0x4d7a6)
    #1 0x1001221d4 in CreateThreadJoinable pthreads_op.c:47
    #2 0x1000013ee in main main.c:95
    #3 0x7fffa9a47234 in start (libdyld.dylib:x86_64+0x5234)

SUMMARY: AddressSanitizer: heap-buffer-overflow syscollector_bsd.c:1295 in getGatewayList

Tested on this branch, AddressSanitizer doesn't report any memory issue.

@chemamartinez
Copy link
Contributor

Leaks tool

For these tests, the Syscollector interval is one minute.

Results for the previous version (3.9 branch)

The summary reports several leaks in wazuh-modulesd:

~ leaks --list --fullStacks --fullContent wazuh-modulesd
Process:         wazuh-modulesd [20747]
Path:            /Library/Ossec/*/wazuh-modulesd
Load Address:    0x100ba1000
Identifier:      wazuh-modulesd
Version:         ???
Code Type:       X86-64
Parent Process:  launchd [1]

Date/Time:       2019-08-05 12:34:22.918 +0200
Launch Time:     2019-08-05 12:34:17.631 +0200
OS Version:      Mac OS X 10.14.6 (18G87)
Report Version:  7
Analysis Tool:   /Applications/Xcode.app/Contents/Developer/usr/bin/leaks
Analysis Tool Version:  Xcode 10.3 (10G8)

Physical footprint:         996K
Physical footprint (peak):  996K
----

leaks Report Version: 3.0
Process 20747: 3290 nodes malloced for 202 KB
Process 20747: 2513 leaks for 95408 total leaked bytes.

As we can see, the content of the leaked memory contains parts of the data reported in the package inventory, the serial board, and the agent IP. The three modules where the memory leaks were found.

Leak: 0x7ffb6ac03ae0  size=16  zone: DefaultMallocZone_0x100c0b000    length: 7  "10.2.13"
Leak: 0x7ffb6ac03af0  size=16  zone: DefaultMallocZone_0x100c0b000    length: 7  "version"
Leak: 0x7ffb6ac03b00  size=16  zone: DefaultMallocZone_0x100c0b000    length: 2  "ID"
Leak: 0x7ffb6ac03b10  size=32  zone: DefaultMallocZone_0x100c0b000    length: 19  "2019/08/05 12:34:19"
Leak: 0x7ffb6ac03b70  size=32  zone: DefaultMallocZone_0x100c0b000    length: 23  "com.microsoft.rdc.macos"
Leak: 0x7ffb6ac03b90  size=16  zone: DefaultMallocZone_0x100c0b000    length: 11  "description"
Leak: 0x7ffb6ac03ba0  size=16  zone: DefaultMallocZone_0x100c0b000    length: 6  "source"
Leak: 0x7ffb6ac03bb0  size=16  zone: DefaultMallocZone_0x100c0b000    length: 8  "location"
------------------
Leak: 0x7ffb6ae052f0  size=16  zone: DefaultMallocZone_0x100c0b000    length: 13  " <serial number>"
------------------
Leak: 0x7ffb6ae01b90  size=16  zone: DefaultMallocZone_0x100c0b000    length: 11  "192.168.0.1"
Leak: 0x7ffb6ae01ba0  size=16  zone: DefaultMallocZone_0x100c0b000    length: 11  "192.168.0.1"

In addition, after successive Syscollector scans, the ammount of memory leaks grow.

Process:         wazuh-modulesd [20747]
Path:            /Library/Ossec/*/wazuh-modulesd
Load Address:    0x100ba1000
Identifier:      wazuh-modulesd
Version:         ???
Code Type:       X86-64
Parent Process:  launchd [1]

Date/Time:       2019-08-05 12:37:40.540 +0200
Launch Time:     2019-08-05 12:34:17.631 +0200
OS Version:      Mac OS X 10.14.6 (18G87)
Report Version:  7
Analysis Tool:   /Applications/Xcode.app/Contents/Developer/usr/bin/leaks
Analysis Tool Version:  Xcode 10.3 (10G8)

Physical footprint:         1348K
Physical footprint (peak):  1348K
----

leaks Report Version: 3.0
Process 20747: 12576 nodes malloced for 488 KB
Process 20747: 11799 leaks for 387728 total leaked bytes.

Results when applying the fixes (tmp-scanbuild-mac branch)

Here the summary of the leaks tool in successive scans of Syscollector:

Process:         wazuh-modulesd [23351]
Path:            /Library/Ossec/*/wazuh-modulesd
Load Address:    0x10c02c000
Identifier:      wazuh-modulesd
Version:         ???
Code Type:       X86-64
Parent Process:  launchd [1]

Date/Time:       2019-08-05 13:16:25.467 +0200
Launch Time:     2019-08-05 13:15:21.235 +0200
OS Version:      Mac OS X 10.14.6 (18G87)
Report Version:  7
Analysis Tool:   /Applications/Xcode.app/Contents/Developer/usr/bin/leaks
Analysis Tool Version:  Xcode 10.3 (10G8)

Physical footprint:         964K
Physical footprint (peak):  964K
----

leaks Report Version: 3.0
Process 23351: 712 nodes malloced for 115 KB
Process 23351: 27 leaks for 9376 total leaked bytes.
Process:         wazuh-modulesd [23351]
Path:            /Library/Ossec/*/wazuh-modulesd
Load Address:    0x10c02c000
Identifier:      wazuh-modulesd
Version:         ???
Code Type:       X86-64
Parent Process:  launchd [1]

Date/Time:       2019-08-05 13:18:34.058 +0200
Launch Time:     2019-08-05 13:15:21.235 +0200
OS Version:      Mac OS X 10.14.6 (18G87)
Report Version:  7
Analysis Tool:   /Applications/Xcode.app/Contents/Developer/usr/bin/leaks
Analysis Tool Version:  Xcode 10.3 (10G8)

Physical footprint:         984K
Physical footprint (peak):  984K
----

leaks Report Version: 3.0
Process 23351: 712 nodes malloced for 115 KB
Process 23351: 27 leaks for 9376 total leaked bytes.

We can see in the fields Date/Time and Launch time that the leaks don't grow in the same execution after several scans. That leads us to think that the reported memory leaks are related to the start-up process of wazuh-modulesd.

@chemamartinez chemamartinez marked this pull request as ready for review August 5, 2019 11:23
@chemamartinez chemamartinez merged commit 64a660a into 3.9 Aug 5, 2019
@chemamartinez chemamartinez deleted the tmp-scanbuild-mac branch August 5, 2019 12:54
chemamartinez added a commit that referenced this pull request Aug 5, 2019
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

4 participants