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

dnsdist: Release memory on DNS over TLS handshake failure #7060

Merged
merged 3 commits into from Oct 15, 2018

Conversation

Projects
None yet
3 participants
@rgacogne
Member

rgacogne commented Oct 12, 2018

Short description

Otherwise we leak the memory allocated to handle the TLS connection if the handshake doesn't succeed correctly.

Checklist

I have:

  • read the CONTRIBUTING.md document
  • compiled this code
  • tested this code
  • included documentation (including possible behaviour changes)
  • documented the code
  • added or modified regression test(s)
  • added or modified unit test(s)

@rgacogne rgacogne added this to the dnsdist-1.3.x milestone Oct 12, 2018

@rgacogne rgacogne requested a review from chbruyand Oct 12, 2018

@rgacogne

This comment has been minimized.

Member

rgacogne commented Oct 12, 2018

We should really use smart pointers there, although OpenSSL structures being opaque makes it a bit harder.

try {
handleIORequest(res, timeout);
}
catch(const std::exception&) {

This comment has been minimized.

@zeha

zeha Oct 12, 2018

Collaborator

maybe this should catch (...) instead, to also get PDNSExceptions?

This comment has been minimized.

@rgacogne

rgacogne Oct 12, 2018

Member

Done, thanks!

@zeha

zeha approved these changes Oct 12, 2018

lgtm

@chbruyand

LGTM

@rgacogne rgacogne merged commit 5c6ce70 into PowerDNS:master Oct 15, 2018

1 check passed

continuous-integration/travis-ci/pr The Travis CI build passed
Details

@rgacogne rgacogne deleted the rgacogne:dnsdist-tls-accept-leak branch Oct 15, 2018

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment