(v6.x backport) dns: fix crash while using dns.setServers after dns.resolve4 #13724
Conversation
The callback function in cares_query is synchronous and called before closed. So dns.setServers in the synchronous callback before closed will occur crashing. Fixes: nodejs#894 Refs: https://github.com/nodejs/node/blob/v6.9.4/deps/cares/src/ares_process.c#L1332-L1333 Refs: https://github.com/OpenSIPS/opensips/blob/2.3.0/proxy.c PR-URL: nodejs#13050 Reviewed-By: Anna Henningsen <anna@addaleax.net>
nodejs-github-bot
added
c++
XadillaX
changed the title
|
CI: https://ci.nodejs.org/job/node-test-commit/10667/ CI was green. |
|
@addaleax LGTY (you reviewed the original)? |
gibfahn
force-pushed
the
v6.x-staging
branch
2 times, most recently
from
6ef8c5b
to
4c2fa3d
Compare
src/cares_wrap.cc
Outdated
| @@ -318,7 +318,7 @@ void cares_wrap_hostent_cpy(struct hostent* dest, struct hostent* src) { | |||
|
|
|||
| /* copy `h_name` */ | |||
| size_t name_size = strlen(src->h_name) + 1; | |||
| dest->h_name = node::Malloc<char>(name_size); | |||
| dest->h_name = reinterpret_cast<char*>(node::Malloc(name_size)); | |||
There was a problem hiding this comment.
Can you avoid reinterpret_cast when a static_cast does the job just fine? Alternatively, you could also backport the overloads of node::Malloc & similar methods as they are on master, that would help with backporting other patches in the future.
There was a problem hiding this comment.
@XadillaX Yes, you’ll just need to try and see how to resolve the conflicts that you’re going to get. I suspect that will be a bit of work, so only do that if you have the time; for now, just changing to static_cast here is definitely easier. :)
|
Is landing this within our backporting contract? If so someone from @nodejs/backporting can feel free to land on staging and we will put it out in a future maintenance release |
|
Do remember to backport 06f62eb before release. |
|
landed in d3caea7 |
The callback function in cares_query is synchronous and called before closed. So dns.setServers in the synchronous callback before closed will occur crashing. Fixes: #894 Refs: https://github.com/nodejs/node/blob/v6.9.4/deps/cares/src/ares_process.c#L1332-L1333 Refs: https://github.com/OpenSIPS/opensips/blob/2.3.0/proxy.c Backport-PR-URL: #13724 PR-URL: #13050 Reviewed-By: Anna Henningsen <anna@addaleax.net>
The callback function in cares_query is synchronous and called before closed. So dns.setServers in the synchronous callback before closed will occur crashing. Fixes: #894 Refs: https://github.com/nodejs/node/blob/v6.9.4/deps/cares/src/ares_process.c#L1332-L1333 Refs: https://github.com/OpenSIPS/opensips/blob/2.3.0/proxy.c Backport-PR-URL: #13724 PR-URL: #13050 Reviewed-By: Anna Henningsen <anna@addaleax.net>
The callback function in
cares_queryis synchronous and called before closed. Sodns.setServersin the synchronous callback before closed will occur crashing.This PR makes the real callback of
dns.resolve4or some other functions indnsasynchronous to resolve this problem.Solution
I use
uv_async_tto send thecallbacktask to next loop. And the task data is created fromCaresAsyncData.Because
Callbackhas two functions with parametershostent*orunsigned char*, int,CaresAsyncDatahas a flagis_hostto distinguish them.In the
Callbackfunction, whetherhostent*orunsigned char*will be destroyed after this function is done. So I make a copy for those buffers for asynchronous use. In the asynchronous function and asynchronous done callback, I delete some related pointers.Related Issue: #894 #1071
Checklist
make -j4 test(UNIX), orvcbuild test(Windows) passesAffected core subsystem(s)
dns