<!-- 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 --> - [ ] PR should be backported to stable branches - [x] Tested changes locally - [ ] Related to issue #XXXX (replace XXXX with an open issue number)
#### Description <!-- Describe your changes in detail --> Ensure that the pointer 'name' is not null and its size not negative.
- Returns 0 in case of invalid name values: null or empty or name length have a negative value. You can view, comment on, or merge this pull request online at:
https://github.com/kamailio/kamailio/pull/4302
-- Commit Summary --
* core: Add null and empty check for AVP name
-- File Changes --
M src/core/usr_avp.c (4)
-- Patch Links --
https://github.com/kamailio/kamailio/pull/4302.patch https://github.com/kamailio/kamailio/pull/4302.diff
@Tomohare pushed 1 commit.
c4fb93cd2092f20277f7a687071868039f68b870 core: Add null and empty check for AVP name
Tomohare left a comment (kamailio/kamailio#4302)
@linuxmaniac Please, could you have a look a this.
miconda left a comment (kamailio/kamailio#4302)
Have you faced any issue that requires such check? As I can see in the code, `match_by_name()` is used only once inside `search_next_avp()`, which expects to have the name set, because one avp was found already.
Tomohare left a comment (kamailio/kamailio#4302)
Yes, we had a core dump in that region, for out of an abundance of caution this check was added.
linuxmaniac left a comment (kamailio/kamailio#4302)
Can you show an example of such coredump?
Tomohare left a comment (kamailio/kamailio#4302)
Here is the coredump: [coredump_kamailio.txt](https://github.com/user-attachments/files/20941539/coredump_kamailio.txt)
Tomohare left a comment (kamailio/kamailio#4302)
PS: this occur on Debian Trixie
linuxmaniac left a comment (kamailio/kamailio#4302)
``s`` is clearly not empty. That value is comming from: ``` #4 0x00007ff1b164d7bc in pv_get_avp (msg=0x7ff1b5cade00, param=0x7ff1b5fdca08, res=0x7fff555caca8) at ./src/modules/pv/pv_core.c:1849 [...] state = {flags = 497, id = 64, name = {n = 140676117154143, s = {s = 0x7ff1b5fdc95f "eventapi_lock)", len = 13}, re = 0x7ff1b5fdc95f}, avp = 0x6874656d5f687365} ```
I would assume the issue here is our friend ``transaction is gone`` instead. The check will not help.
miconda left a comment (kamailio/kamailio#4302)
I guess you still have the coredump, can you get the output with gdb for the next commands?
``` frame 4 print *param ```
The avp name seems to be set down in the trace (`eventapi_lock`), is it used like `$avp(eventapi_lock)` or via a variable as name (e.g., `$avp($var(name))`)?
Tomohare left a comment (kamailio/kamailio#4302)
The coredump was deleted by mistake. We will be re-running tests to try to generate this error again. Also, we are using AVP as this `$avp(s:eventapi_lock)`.
Tomohare left a comment (kamailio/kamailio#4302)
Here is the result of those commands with a new coredump:
`(gdb) frame 4 #4 0x00007f87918217bc in pv_get_avp (msg=0x7f8795ec8100, param=0x7f87961f6d08, res=0x7ffdbb3a7b38) at ./src/modules/pv/pv_core.c:1849 1849 if((avp = search_first_avp(name_type, avp_name, &avp_value, &state)) == 0) (gdb) print *param $1 = {pvn = {type = 0, nfree = 0x0, u = {isname = {type = 1, name = {n = 140220315954271, s = {s = 0x7f87961f6c5f "eventapi_lock)", len = 13}, re = 0x7f87961f6c5f}}, dname = 0x1}}, pvi = {type = 4, u = {ival = 0, dval = 0x0}}}`
miconda left a comment (kamailio/kamailio#4302)
It looks good, so likely something happened with the avp.
The backtrace shows that params/vars are optimized, but let's try to see them anyhow, in gdb do:
``` frame 0 p avp_name p *avp_name p avp p *avp p name p *name p id ```
Tomohare left a comment (kamailio/kamailio#4302)
``` (gdb) p *avp_name value has been optimized out (gdb) p avp $1 = (avp_t *) 0x205049532e797469 (gdb) p *avp Cannot access memory at address 0x205049532e797469 (gdb) p name $2 = (str *) 0x7ffdbb3a77a8 (gdb) p *name $3 = {s = 0x7f87961f6c5f "eventapi_lock)", len = 13} (gdb) p id $4 = 64 ```
miconda left a comment (kamailio/kamailio#4302)
The `name` seems to be ok, so the patch of the PR won't save it. The `avp` seems to be the issue.
Tomohare left a comment (kamailio/kamailio#4302)
Yes, will be closing it since it doesn't do what I expected to do (and the problem seems to be in the avp as miconda said).
Closed #4302.
-
Daniel-Constantin Mierla -
Tomohare
-
Victor Seva