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

Fixes #275

Open
wants to merge 12 commits into
base: master
Choose a base branch
from
Open

Fixes #275

wants to merge 12 commits into from

Conversation

dougnazar
Copy link
Contributor

Here are several fixes for nrpe & check_nrpe. Let me know if you'd prefer me to break this up into multiple requests. Kind of kept finding new stuff as I went along.

  • Fix various warnings when building with -Wall.

    Various misc fixes, mainly left over code.

  • Split my_system() into separate functions for child & parent.

  • Rewrite my_system_*() IO loops to handle async & large buffers.

    Due to scheduling and various implementations, we can't depend on read/write having data/space available. It's also possible to fill the send buffer before we exit so need to read data while waiting for child to exit.
    I've tested with 10000, 60000, 70000 sized outputs. The 10000 & 60000 hashes match, and as expected the 70000 gets truncated.
    Will probably help with nrpe[xxxx]: ERROR: my_system() write(fd, buffer)-2 failed... #271

  • Fix debug display of acl IP addresses

    Would always show same address due to shared buffer.

  • Show correct address family when setting up listen addresses.

    Would always say IPv4 no matter which family the address was.

  • Only make requested binary

    Top level Makefile didn't pass command to child so would always make all

  • Don't compile utils.c twice, once for each program.

    utils.c is shared between both programs.

  • Fix a few pedantic warnings, mostly prototype related.

  • Fix use of uninitialized variable.

  • Clear any existing ACLs before parsing new config.

    Would get duplicate ACL entries if allowed_hosts was used multiple times in config or on every SIGHUP.
    Also would try to dump IPv6 ACLs as IPv4.

  • Fix various leaks detected by valgrind.

    All config options that use strdup() would leak memory if used more than once or on SIGHUP.
    Only allocate fdset when size changes and ensure we free it.

In my_system_parent(), don't wait for child to exit before reading buffer.
The child may fill the pipe buffer and start erroring, so do both operations
concurrently.

In my_system_child(), similarly check that data/space is available before
we read/write to handle asymetric data rates.
inet_ntoa uses a static buffer which causes duplicate IP addresses to be printed.
Spotted via valgrind.
Multiple allowed_hosts config lines or every SIGHUP would cause duplicates.
Free previous allowed_hosts to prevent leaking.
Fix dumping of ACLs to properly display AF_INET6 addresses.
Mostly config options that strdup() their value, causing leaks if
there are multiple config lines or after restarting with SIGHUP.

The last was the fd_set for incoming connections wouldn't be freed
on restart. Also optimize the allocation of fdset and re-use if possible.
my_strpos() modifies the string pointer causing us to free
an incorrect pointer.
src/nrpe.c Show resolved Hide resolved
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.

2 participants