### Description
While testing to implement an IMS AS, I used the PUA_REGINFO module to receive details about the SIP registration. PUA_REGINFO module uses PUA and USRLOC modules.
However, a couple of issues was found including a crash:
1) Crash when processing SIP Notify with registration terminated info.
2) Randomly fails storing data to USRLOC, (entries already exists)
3) Saves corrupt data to database use_domain is set in usrloc but domain not provided in Notify request
### Troubleshooting
I am pretty new to Kamailio, but my findings are the following in notify.c
1) PUA_REGINFO modules, deletes the record with "ul.delete_urecord(...)" and later down in the code calls "if (ul_record) ul.release_urecord(ul_record);". Crashes on the release_urecord call.
My assumption without knowing to much about userloc is that release can not be called on a deleted record. Hence it should be enough to set ul_record = NULL after calling "ul.delete_urecord(...)" to not call release_urecord later on.
2) It looks like there is an issue handling parallell request.
Without knowing too much, it replacing sruid_next(..) with sruid_next_safe() resolves the issue.
Also, the static variable of type ucontact_info_t should probably become local as well.
3) Maybe incorrect to set use_domain to 1 when using the module, however I think it should be failsafe and not store garbage data in to database. Not looked into this issue.
#### Reproduction
Send Notify request with REGINFO body for registration and unregistration and forward it to the module according to the documentation of PUA_REGINFO.
#### Debugging Data
Attaching SIPp scenario.
Changing code according to 1) and 2) make the SIPp script runs and no critical issues seen.
(Have too less experience to ensure that no memleaks are introduced or still present in the module code)
<?xml version="1.0" encoding="ISO-8859-1" ?>
<scenario name="notify">
<send retrans="500">
<![CDATA[
NOTIFY sip:[remote_ip] SIP/2.0
Via: SIP/2.0/[transport] [local_ip]:[local_port];branch=[branch]
From: <sip:[field0]@[field1]>;tag=[call_number]
To: <sip:[field0]@[field1]>
Call-ID: [call_id]
CSeq: 1 NOTIFY
Contact: sip:[field0]@[local_ip]:[local_port]
Max-Forwards: 70
Expires: 1800
Event: reg
User-Agent: SIPp/Linux
Subscription-State: active;expires=6888
Content-Type: application/reginfo+xml
Content-Length: [len]
<?xml version="1.0"?>
<reginfo xmlns="urn:ietf:params:xml:ns:reginfo" version="2" state="full">
<registration aor="sip:[field0]@mnc001.mcc001.3gppnetwork.org" id="0x7feff71118f8" state="active">
<contact id="0x7feff7126e58" state="active" event="registered" expires="595" q="0.500">
<uri>sip:[field0]@192.168.55.103:21061;ob;alias=192.168.55.103~21061~1</uri>
<unknown-param name="+g.3gpp.smsip"></unknown-param>
<unknown-param name="q">"0.5"</unknown-param>
</contact>
</registration>
</reginfo>
]]>
</send>
<recv response="202" rtd="true">
</recv>
<pause milliseconds="1000"/>
<send retrans="500">
<![CDATA[
NOTIFY sip:[remote_ip] SIP/2.0
Via: SIP/2.0/[transport] [local_ip]:[local_port];branch=[branch]
From: <sip:[field0]@[field1]>;tag=[call_number]
To: <sip:[field0]@[field1]>
Call-ID: [call_id]
CSeq: 2 NOTIFY
Contact: sip:[field0]@[local_ip]:[local_port]
Max-Forwards: 70
Expires: 1800
User-Agent: SIPp/Linux
Event: reg
Subscription-State: active;expires=6888
Content-Type: application/reginfo+xml
Content-Length: [len]
<?xml version="1.0"?>
<reginfo xmlns="urn:ietf:params:xml:ns:reginfo" version="3" state="full">
<registration aor="sip:[field0]@mnc001.mcc001.3gppnetwork.org" id="0x7feff71118f8" state="terminated">
<contact id="0x1" state="terminated" event="expired" expires="0" q="0.000">
<uri>sip:[field0]@192.168.55.103:21061;ob;alias=192.168.55.103~21061~1</uri>
</contact>
</registration>
</reginfo>
]]>
</send>
<recv response="202" rtd="true">
</recv>
</scenario>
#### Log Messages
#### SIP Traffic
### Possible Solutions
See troubleshooting.
### Additional Information
* **Kamailio Version** - output of `kamailio -v`
```
5.1.4
```
* **Operating System**:
```
Debian 8.11
```
--
You are receiving this because you are subscribed to this thread.
Reply to this email directly or view it on GitHub:
https://github.com/kamailio/kamailio/issues/1579
Module: kamailio
Branch: 5.1
Commit: 2c91b11abbe835ca3319ba3517d9fd726f078937
URL: https://github.com/kamailio/kamailio/commit/2c91b11abbe835ca3319ba3517d9fd7…
Author: lasseo <lars.olsson76(a)gmail.com>
Committer: Daniel-Constantin Mierla <miconda(a)gmail.com>
Date: 2018-08-22T11:01:14+02:00
pua_reginfo: fix crash and issue during high load
- do not release a already deleted entry, as this causes a crash
- use sruid_next_safe instead of sruid_next as it has been seen during load that same sruid returned on multiple threads
(cherry picked from commit 62e0af0957a6c7e3c2ea9711cf64ad51f4b2e594)
---
Modified: src/modules/pua_reginfo/notify.c
---
Diff: https://github.com/kamailio/kamailio/commit/2c91b11abbe835ca3319ba3517d9fd7…
Patch: https://github.com/kamailio/kamailio/commit/2c91b11abbe835ca3319ba3517d9fd7…
---
diff --git a/src/modules/pua_reginfo/notify.c b/src/modules/pua_reginfo/notify.c
index 754a2ed099..ea50f2db65 100644
--- a/src/modules/pua_reginfo/notify.c
+++ b/src/modules/pua_reginfo/notify.c
@@ -111,7 +111,7 @@ int process_contact(udomain_t * domain, urecord_t ** ul_record, str aor, str cal
ci.expires = time(0) + expires;
/* set ruid */
- if(sruid_next(&_reginfo_sruid) < 0) {
+ if(sruid_next_safe(&_reginfo_sruid) < 0) {
LM_ERR("failed to generate ruid");
} else {
ci.ruid = _reginfo_sruid.uid;
@@ -306,9 +306,15 @@ int process_body(str notify_body, udomain_t * domain) {
}
ul_contact = ul_contact->next;
}
+
if (ul.delete_urecord(domain, &aor_key, ul_record) < 0) {
LM_ERR("failed to remove record from usrloc\n");
- }
+ }
+
+ /* Record deleted, and should not be used anymore */
+ ul_record = NULL;
+
+
/* If already a registration with contacts was found, then keep that result.
otherwise the result is now "No contacts found" */
if (final_result != RESULT_CONTACTS_FOUND) final_result = RESULT_NO_CONTACTS;
Module: kamailio
Branch: master
Commit: 62e0af0957a6c7e3c2ea9711cf64ad51f4b2e594
URL: https://github.com/kamailio/kamailio/commit/62e0af0957a6c7e3c2ea9711cf64ad5…
Author: lasseo <lars.olsson76(a)gmail.com>
Committer: lasseo <lars.olsson76(a)gmail.com>
Date: 2018-08-21T22:10:04Z
pua_reginfo: fix crash and issue during high load
- do not release a already deleted entry, as this causes a crash
- use sruid_next_safe instead of sruid_next as it has been seen during load that same sruid returned on multiple threads
---
Modified: src/modules/pua_reginfo/notify.c
---
Diff: https://github.com/kamailio/kamailio/commit/62e0af0957a6c7e3c2ea9711cf64ad5…
Patch: https://github.com/kamailio/kamailio/commit/62e0af0957a6c7e3c2ea9711cf64ad5…
---
diff --git a/src/modules/pua_reginfo/notify.c b/src/modules/pua_reginfo/notify.c
index 754a2ed099..ea50f2db65 100644
--- a/src/modules/pua_reginfo/notify.c
+++ b/src/modules/pua_reginfo/notify.c
@@ -111,7 +111,7 @@ int process_contact(udomain_t * domain, urecord_t ** ul_record, str aor, str cal
ci.expires = time(0) + expires;
/* set ruid */
- if(sruid_next(&_reginfo_sruid) < 0) {
+ if(sruid_next_safe(&_reginfo_sruid) < 0) {
LM_ERR("failed to generate ruid");
} else {
ci.ruid = _reginfo_sruid.uid;
@@ -306,9 +306,15 @@ int process_body(str notify_body, udomain_t * domain) {
}
ul_contact = ul_contact->next;
}
+
if (ul.delete_urecord(domain, &aor_key, ul_record) < 0) {
LM_ERR("failed to remove record from usrloc\n");
- }
+ }
+
+ /* Record deleted, and should not be used anymore */
+ ul_record = NULL;
+
+
/* If already a registration with contacts was found, then keep that result.
otherwise the result is now "No contacts found" */
if (final_result != RESULT_CONTACTS_FOUND) final_result = RESULT_NO_CONTACTS;
<!-- Kamailio Pull Request Template -->
<!--
IMPORTANT:
- for detailed contributing guidelines, read:
https://github.com/kamailio/kamailio/blob/master/.github/CONTRIBUTING.md
- pull requests must be done to master branch, unless they are backports
of fixes from master branch to a stable branch
- backports to stable branches must be done with 'git cherry-pick -x ...'
- code is contributed under BSD for core and main components (tm, sl, auth, tls)
- code is contributed GPLv2 or a compatible license for the other components
- GPL code is contributed with OpenSSL licensing exception
-->
#### Pre-Submission Checklist
<!-- Go over all points below, and after creating the PR, tick all the checkboxes that apply -->
<!-- All points should be verified, otherwise, read the CONTRIBUTING guidelines from above-->
<!-- If you're unsure about any of these, don't hesitate to ask on sr-dev mailing list -->
- [ x] Commit message has the format required by CONTRIBUTING guide
- [ x] Commits are split per component (core, individual modules, libs, utils, ...)
- [x ] Each component has a single commit (if not, squash them into one commit)
- [x] No commits to README files for modules (changes must be done to docbook files
in `doc/` subfolder, the README file is autogenerated)
#### Type Of Change
- [x ] Small bug fix (non-breaking change which fixes an issue)
- [ ] New feature (non-breaking change which adds new functionality)
- [ ] Breaking change (fix or feature that would change existing functionality)
#### Checklist:
<!-- Go over all points below, and after creating the PR, tick the checkboxes that apply -->
- [ x] PR should be backported to stable branches
- [x ] Tested changes locally
- [ x] Related to issue #1579 (replace XXXX with an open issue number)
#### Description
Fixes in pua_reginfo module that prevents module from crashing up on a register and unregister event.
Use secure sruid generator to avoid duplicated sruids.
Discussed in issue #1579
You can view, comment on, or merge this pull request online at:
https://github.com/kamailio/kamailio/pull/1623
-- Commit Summary --
* pua_reginfo: fix crash and issue during high load
-- File Changes --
M src/modules/pua_reginfo/notify.c (10)
-- Patch Links --
https://github.com/kamailio/kamailio/pull/1623.patchhttps://github.com/kamailio/kamailio/pull/1623.diff
--
You are receiving this because you are subscribed to this thread.
Reply to this email directly or view it on GitHub:
https://github.com/kamailio/kamailio/pull/1623
Module: kamailio
Branch: 5.1
Commit: 233079b5174ba0c4e325e80ebb5f11504349b504
URL: https://github.com/kamailio/kamailio/commit/233079b5174ba0c4e325e80ebb5f115…
Author: Victor Seva <linuxmaniac(a)torreviejawireless.org>
Committer: Victor Seva <linuxmaniac(a)torreviejawireless.org>
Date: 2018-08-22T10:06:48+02:00
pkg/kamailio/deb: version set to 5.1.5
---
Modified: pkg/kamailio/deb/buster/changelog
Modified: pkg/kamailio/deb/debian/changelog
Modified: pkg/kamailio/deb/jessie/changelog
Modified: pkg/kamailio/deb/precise/changelog
Modified: pkg/kamailio/deb/sid/changelog
Modified: pkg/kamailio/deb/stretch/changelog
Modified: pkg/kamailio/deb/trusty/changelog
Modified: pkg/kamailio/deb/wheezy/changelog
Modified: pkg/kamailio/deb/xenial/changelog
---
Diff: https://github.com/kamailio/kamailio/commit/233079b5174ba0c4e325e80ebb5f115…
Patch: https://github.com/kamailio/kamailio/commit/233079b5174ba0c4e325e80ebb5f115…
---
diff --git a/pkg/kamailio/deb/buster/changelog b/pkg/kamailio/deb/buster/changelog
index b078c9f9a7..19b4806e61 100644
--- a/pkg/kamailio/deb/buster/changelog
+++ b/pkg/kamailio/deb/buster/changelog
@@ -1,3 +1,9 @@
+kamailio (5.1.5) unstable; urgency=medium
+
+ * version set to 5.1.5
+
+ -- Victor Seva <vseva(a)debian.org> Wed, 22 Aug 2018 10:05:51 +0200
+
kamailio (5.1.4) unstable; urgency=medium
* version set to 5.1.4
diff --git a/pkg/kamailio/deb/debian/changelog b/pkg/kamailio/deb/debian/changelog
index b078c9f9a7..19b4806e61 100644
--- a/pkg/kamailio/deb/debian/changelog
+++ b/pkg/kamailio/deb/debian/changelog
@@ -1,3 +1,9 @@
+kamailio (5.1.5) unstable; urgency=medium
+
+ * version set to 5.1.5
+
+ -- Victor Seva <vseva(a)debian.org> Wed, 22 Aug 2018 10:05:51 +0200
+
kamailio (5.1.4) unstable; urgency=medium
* version set to 5.1.4
diff --git a/pkg/kamailio/deb/jessie/changelog b/pkg/kamailio/deb/jessie/changelog
index b078c9f9a7..19b4806e61 100644
--- a/pkg/kamailio/deb/jessie/changelog
+++ b/pkg/kamailio/deb/jessie/changelog
@@ -1,3 +1,9 @@
+kamailio (5.1.5) unstable; urgency=medium
+
+ * version set to 5.1.5
+
+ -- Victor Seva <vseva(a)debian.org> Wed, 22 Aug 2018 10:05:51 +0200
+
kamailio (5.1.4) unstable; urgency=medium
* version set to 5.1.4
diff --git a/pkg/kamailio/deb/precise/changelog b/pkg/kamailio/deb/precise/changelog
index b078c9f9a7..19b4806e61 100644
--- a/pkg/kamailio/deb/precise/changelog
+++ b/pkg/kamailio/deb/precise/changelog
@@ -1,3 +1,9 @@
+kamailio (5.1.5) unstable; urgency=medium
+
+ * version set to 5.1.5
+
+ -- Victor Seva <vseva(a)debian.org> Wed, 22 Aug 2018 10:05:51 +0200
+
kamailio (5.1.4) unstable; urgency=medium
* version set to 5.1.4
diff --git a/pkg/kamailio/deb/sid/changelog b/pkg/kamailio/deb/sid/changelog
index b078c9f9a7..19b4806e61 100644
--- a/pkg/kamailio/deb/sid/changelog
+++ b/pkg/kamailio/deb/sid/changelog
@@ -1,3 +1,9 @@
+kamailio (5.1.5) unstable; urgency=medium
+
+ * version set to 5.1.5
+
+ -- Victor Seva <vseva(a)debian.org> Wed, 22 Aug 2018 10:05:51 +0200
+
kamailio (5.1.4) unstable; urgency=medium
* version set to 5.1.4
diff --git a/pkg/kamailio/deb/stretch/changelog b/pkg/kamailio/deb/stretch/changelog
index b078c9f9a7..19b4806e61 100644
--- a/pkg/kamailio/deb/stretch/changelog
+++ b/pkg/kamailio/deb/stretch/changelog
@@ -1,3 +1,9 @@
+kamailio (5.1.5) unstable; urgency=medium
+
+ * version set to 5.1.5
+
+ -- Victor Seva <vseva(a)debian.org> Wed, 22 Aug 2018 10:05:51 +0200
+
kamailio (5.1.4) unstable; urgency=medium
* version set to 5.1.4
diff --git a/pkg/kamailio/deb/trusty/changelog b/pkg/kamailio/deb/trusty/changelog
index b078c9f9a7..19b4806e61 100644
--- a/pkg/kamailio/deb/trusty/changelog
+++ b/pkg/kamailio/deb/trusty/changelog
@@ -1,3 +1,9 @@
+kamailio (5.1.5) unstable; urgency=medium
+
+ * version set to 5.1.5
+
+ -- Victor Seva <vseva(a)debian.org> Wed, 22 Aug 2018 10:05:51 +0200
+
kamailio (5.1.4) unstable; urgency=medium
* version set to 5.1.4
diff --git a/pkg/kamailio/deb/wheezy/changelog b/pkg/kamailio/deb/wheezy/changelog
index b078c9f9a7..19b4806e61 100644
--- a/pkg/kamailio/deb/wheezy/changelog
+++ b/pkg/kamailio/deb/wheezy/changelog
@@ -1,3 +1,9 @@
+kamailio (5.1.5) unstable; urgency=medium
+
+ * version set to 5.1.5
+
+ -- Victor Seva <vseva(a)debian.org> Wed, 22 Aug 2018 10:05:51 +0200
+
kamailio (5.1.4) unstable; urgency=medium
* version set to 5.1.4
diff --git a/pkg/kamailio/deb/xenial/changelog b/pkg/kamailio/deb/xenial/changelog
index b078c9f9a7..19b4806e61 100644
--- a/pkg/kamailio/deb/xenial/changelog
+++ b/pkg/kamailio/deb/xenial/changelog
@@ -1,3 +1,9 @@
+kamailio (5.1.5) unstable; urgency=medium
+
+ * version set to 5.1.5
+
+ -- Victor Seva <vseva(a)debian.org> Wed, 22 Aug 2018 10:05:51 +0200
+
kamailio (5.1.4) unstable; urgency=medium
* version set to 5.1.4
Hello,
with the summer holidays in the northern hemisphere approaching the end,
I am considering to release v5.1.5 out of Kamailio git branch 5.1 --
there were several fixes done since 5.1.4 and it is time to package
another minor release.
The propose date is next week on Wednesday, August 22, or slightly after
in case there are unexpected events delays it.
Should there be any issues you are aware of and not yet reported on the
bug tracker, do it asap in order to have a chance to be fixed:
- https://github.com/kamailio/kamailio/issues
Cheers,
Daniel
--
Daniel-Constantin Mierla -- www.asipto.comwww.twitter.com/miconda -- www.linkedin.com/in/miconda
Kamailio World Conference -- www.kamailioworld.com