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

gh-184 Fix segfault and socket fd leak #227

Merged
merged 1 commit into from Aug 6, 2021

Conversation

m-sola
Copy link
Contributor

@m-sola m-sola commented Jul 27, 2021

This fixes a fatal issue that would occur when unable to queue events due to
clamonacc improperly using all available fds.

It also fixes the core fd socket leak issue at the heart of the segfault by
properly cleaning up after a failed curl connection.

@m-sola m-sola mentioned this pull request Jul 27, 2021
@micahsnyder micahsnyder self-requested a review July 27, 2021 23:25
@micahsnyder
Copy link
Contributor

Can you please edit the commit message so it starts with ClamOnAcc: ?
And for the ticket, gh-184 works but I'd recommend just a link at the end of the commit message so it's easier to refer to when not browsing on github.com. Something like:
Fixes https://github.com/Cisco-Talos/clamav/issues/184

@@ -241,6 +241,7 @@ int onas_dsresult(CURL *curl, int scantype, uint64_t maxstream, const char *file
int sockd = -1;
int (*recv_func)(struct onas_rcvln *, char **, char **, int64_t) = NULL;


Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This file needs to be clang-format'ed.

@goshansp
Copy link
Contributor

wow, this amazing fix performs perfectly stable in my scenarios - looking forward to the merge! thanks

This fixes a fatal issue that would occur when unable to queue events due to
clamonacc improperly using all available fds.

It also fixes the core fd socket leak issue at the heart of the segfault by
properly cleaning up after a failed curl connection.

Lastly, worst case recovery code now allows more time for consumer queue
to catchup. It accomplishes this by increasing wait time and adding
retry logic.

More info: Cisco-Talos#184
@micahsnyder
Copy link
Contributor

Looks good to me too!

@micahsnyder micahsnyder changed the base branch from rel/0.104 to main August 6, 2021 00:31
@micahsnyder
Copy link
Contributor

micahsnyder commented Aug 6, 2021

Will merge into main, then cherry-pick to rel/0.104. We'll need to create a separate PR for dev/0.103.4 that includes a comment in the News about the fix.

@micahsnyder micahsnyder merged commit d0d0a83 into Cisco-Talos:main Aug 6, 2021
22 of 24 checks passed
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

3 participants