Skip to content

Remove useless select() calls in Analysisd decoders#3964

Merged
chemamartinez merged 3 commits into3.10from
3.10-fix-select-call
Sep 13, 2019
Merged

Remove useless select() calls in Analysisd decoders#3964
chemamartinez merged 3 commits into3.10from
3.10-fix-select-call

Conversation

@chemamartinez
Copy link
Copy Markdown
Contributor

@chemamartinez chemamartinez commented Sep 12, 2019

Description

As @vikman90 states here. There exists a known bug with the select() call when handling file descriptors higher than 1024.

We noticed the select() calls in the SCA, Syscollector, and Syscheck decoders are not needed since OS_RecvSecureTCP() waits for the response from Wazuh DB.

Tests

  • Compilation without warnings in every supported platform
    • Linux
  • Source installation
  • Review logs syntax and correct language
  • Tested queries from the three issued decoders.

Analysisd stats after restarting:

# Syscheck events decoded
syscheck_events_decoded='431'
syscheck_edps='86'

# Syscollector events decoded
syscollector_events_decoded='472'
syscollector_edps='94'

# Security configuration assessment events decoded
sca_events_decoded='101'
sca_edps='20'
  • Memory tests for Linux
    • Scan-build report
    • Valgrind (memcheck and descriptor leaks check)

Valgrind report of ossec-analysisd stressed with Syscheck, Syscollector and SCA events:

==46889== LEAK SUMMARY:
==46889==    definitely lost: 100 bytes in 9 blocks
==46889==    indirectly lost: 0 bytes in 0 blocks
==46889==      possibly lost: 7,072 bytes in 26 blocks
==46889==    still reachable: 14,881,769 bytes in 68,750 blocks
==46889==         suppressed: 0 bytes in 0 blocks
==46889== Reachable blocks (those to which a pointer was found) are not shown.
==46889== To see them, rerun with: --leak-check=full --show-leak-kinds=all

Scan-build report

Done building server

scan-build: Removing directory '/tmp/scan-build-2019-09-12-103908-47512-1' because it contains no reports.
scan-build: No bugs found.

vikman90 and others added 2 commits September 12, 2019 10:16
- This prevents an overflow when using fds greater than 1024.
- select() call is useless here as the flow continues whether there is timeout or not.
@chemamartinez chemamartinez added type/bug Something isn't working module/analysis Issues related to the Analysis daemon labels Sep 12, 2019
@chemamartinez chemamartinez added this to the Sprint - 100 milestone Sep 12, 2019
@chemamartinez chemamartinez requested a review from snaow September 12, 2019 17:50
@chemamartinez chemamartinez self-assigned this Sep 12, 2019
@JuantAldea
Copy link
Copy Markdown
Contributor

JuantAldea commented Sep 13, 2019

Changes are correct.

Given that:

  • recv, on its own, waits, and
  • the only purpose of those selects is to wait on a single descriptor,

connectwill return:

  • -1: failure and return. But if there's no select there's no error,
  • >=0: number of descriptors with activity
    • 0: No descriptors with activity -> timeout -> recv will wait
    • >0: recv will do whatever it has to.

As shown, recv will wait if needed, rendering the wait performed by connect redundant.

Q.E.D

@chemamartinez chemamartinez merged commit 3b02f9d into 3.10 Sep 13, 2019
@chemamartinez chemamartinez deleted the 3.10-fix-select-call branch September 13, 2019 13:22
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

module/analysis Issues related to the Analysis daemon type/bug Something isn't working

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants