Hi, Vitalii!
I had this mail unread for a while to check your report, but apparently
Dan Pascu detected it and fixed it faster than I could check it[1].
So I can confirm the fix you suggested is OK, and it is now pushed
upstream. I will backport it down to 2.4.
Just a small suggestion - next time you have a devel related request,
better post it on the github issue - over there we can categorize the
issues faster and easier.
[1] https://github.com/OpenSIPS/opensips/issues/1974
Best regards,
Răzvan
On 1/24/20 6:25 PM, Vitalii Aleksandrov wrote:
Yesterday I've mentioned that has reproduces one more crash.
Reverted my fix and wanted to reproduce the problem and create properly
filled crash report, but unfortunately failed. Can you just check the
part of code I've seen in core dumps?
fake_req() in modules/tm/t_msgbuilder.h clones sip_msg allocated in shm
and then substitutes some fields with pkg allocated copies.
Here is one of those copy operations:
if (uac->duri.s) {
faked_req->dst_uri.s = pkg_malloc(uac->duri.len);
if (!faked_req->dst_uri.s) {
LM_ERR("out of pkg mem\n");
goto out;
}
memcpy(faked_req->dst_uri.s, uac->duri.s, uac->duri.len);
}
Then free_faked_req() deletes those copies calling pkg_free():
if (faked_req->dst_uri.s) {
pkg_free(faked_req->dst_uri.s);
faked_req->dst_uri.s = NULL;
}
I've had crashes here and there and gdb showed corrupted or overwritten
memory chunks. After switching to QM_MALLOC and enabling DBG_MALLOC I've
got opensips aborted trying to call pkg_free()
for shm allocated memory. It somehow happened that fake_req() hasn't
allocated pkg copy for faked_req->dst_uri.s and it stayed pointing to
shm allocated chunk and then crashed in free_faked_req().
Have no idea why I can't reproduce it anymore. Remember that backtrace
had t_should_relay_responce()->do_dns_failover()->free_faked_req() and
it was a processing of 408 reply for BYE request.
The only thing I'm not sure about is whether I had it before or after
rebasing my code under the latest 2.4 with
cc62f7df728467b8144095767183fedfdf74be8d commit.
Maybe adding safety checks to fake_req() still makes sense to make look
like this:
if (uac->duri.s) {
faked_req->dst_uri.s = pkg_malloc(uac->duri.len);
if (!faked_req->dst_uri.s) {
LM_ERR("out of pkg mem\n");
goto out;
}
memcpy(faked_req->dst_uri.s, uac->duri.s, uac->duri.len);
} else {
faked_req->dst_uri.s = NULL; // <----
faked_req->dst_uri.len = 0; // <----
}
if (uac->path_vec.s) {
faked_req->path_vec.s = pkg_malloc(uac->path_vec.len);
if (!faked_req->path_vec.s) {
LM_ERR("out of pkg mem\n");
goto out2;
}
memcpy(faked_req->path_vec.s, uac->path_vec.s,
uac->path_vec.len);
} else {
faked_req->path_vec.s = NULL; // <---
faked_req->path_vec.len = 0; // <---
}
_______________________________________________
Users mailing list
Users@lists.opensips.org
http://lists.opensips.org/cgi-bin/mailman/listinfo/users
--
Răzvan Crainea
OpenSIPS Core Developer
http://www.opensips-solutions.com
_______________________________________________
Users mailing list
Users@lists.opensips.org
http://lists.opensips.org/cgi-bin/mailman/listinfo/users
