[OpenSIPS-Users] Possible crash in opensips 2.4

[Răzvan Crainea]

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 at lists.opensips.org
> http://lists.opensips.org/cgi-bin/mailman/listinfo/users

-- 
Răzvan Crainea
OpenSIPS Core Developer
   http://www.opensips-solutions.com