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

Memory leak in sanity module 'proxy require' check #1990

Closed
tijszwinkels opened this issue Jun 21, 2019 · 1 comment
Closed

Memory leak in sanity module 'proxy require' check #1990

tijszwinkels opened this issue Jun 21, 2019 · 1 comment

Comments

@tijszwinkels
Copy link

@tijszwinkels tijszwinkels commented Jun 21, 2019

Description

We noticed one of our kamailio instances using increasing amounts of pkg memory.

This appears to be related to the sanity-check module.
sanity.c:655 - if (msg->proxy_require->parsed == NULL) then that header is parsed into msg->proxy_require->parsed. This buffer should be free'd in sanity.c:712, but this only happens if the sanity check is succesfull. If the sanity check fails (on line 701), then the buffer is never free'd.

Troubleshooting

Reproduction

We can reproduce the issue by repeatedly sending the following SIP packet to kamailio.
Please note that this 'Proxy-Require' header is invalid in our setup, and is most likely originally caused by an invalid configured phone.

SUBSCRIBE sip:phone_0@example.com:5060 SIP/2.0
Via: SIP/2.0/UDP 192.168.1.102:5062;branch=z9hG4bK2104306013
From: "xxx" <sip:phone_0@example.com>;tag=108111795
To: "xxx" <sip:phone_0@example.com>
Call-ID: 3276145660@192.168.1.102
CSeq: 1 SUBSCRIBE
Contact: <sip:phone_0@192.168.1.102:5062>
Accept: application/x-as-feature-event+xml
Max-Forwards: 70
User-Agent: Yealink SIP-T48G 35.72.188.7
Proxy-Require: sip.example.com
Expires: 3630
Event: as-feature-event
Content-Length: 0

Possible Solutions

The code to free the buffer could be duplicated before the return on sanity.c:701.

However, it's not clear to me why the msg->proxy_require->parsed structure doesn't get properly free'd when msg is free'd, as most other fields in this struct seem to be destructed correctly.

For now, we disabled the proxy require check with:

modparam("sanity", "default_checks", 2535)

this mitigates the memory leak.

Additional Information

  • Kamailio Version - output of kamailio -v
version: kamailio 5.2.2 (x86_64/linux) 
flags: STATS: Off, USE_TCP, USE_TLS, USE_SCTP, TLS_HOOKS, USE_RAW_SOCKS, DISABLE_NAGLE, USE_MCAST, DNS_IP_HACK, SHM_MEM, SHM_MMAP, PKG_MALLOC, Q_MALLOC, F_MALLOC, TLSF_MALLOC, DBG_SR_MEMORY, USE_FUTEX, FAST_LOCK-ADAPTIVE_WAIT, USE_DNS_CACHE, USE_DNS_FAILOVER, USE_NAPTR, USE_DST_BLACKLIST, HAVE_RESOLV_RES
ADAPTIVE_WAIT_LOOPS=1024, MAX_RECV_BUFFER_SIZE 262144 MAX_URI_SIZE 1024, BUF_SIZE 65535, DEFAULT PKG_SIZE 8MB
poll method support: poll, epoll_lt, epoll_et, sigio_rt, select.
id: unknown 
compiled with gcc 5.3.1

Problem still present in current master.

  • Operating System:

Ubuntu 16.04

Linux tijscmp01 4.4.0-145-generic #171-Ubuntu SMP Tue Mar 26 12:43:40 UTC 2019 x86_64 x86_64 x86_64 GNU/Linux
miconda added a commit that referenced this issue Jun 25, 2019
- reported by GH #1990
@miconda

This comment has been minimized.

Copy link
Member

@miconda miconda commented Jun 25, 2019

Thanks! I pushed a fix (commit referenced above), it will be backported to stable branches soon. If still an issue, reopen.

@miconda miconda closed this Jun 25, 2019
miconda added a commit that referenced this issue Aug 12, 2019
miconda added a commit that referenced this issue Sep 24, 2019
- reported by GH #1990

(cherry picked from commit ffa2aa4)
(cherry picked from commit bf5e169)
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Linked pull requests

Successfully merging a pull request may close this issue.

None yet
2 participants
You can’t perform that action at this time.