### Description
We've been experiencing random Kamailio crashes related to invalid memory access attempts at `0xffff` in `w_save` call, `ims_registrar_scscf_mod.c`:
``` Core was generated by `kamailio -w /home -DD -E -e -f /etc/kamailio/kamailio_scscf.cfg'. Program terminated with signal SIGSEGV, Segmentation fault. #0 0x00007f5be796c452 in w_save (_m=0x7f5beafa5188, _route=0x7f5beaeab988 " \b\361\352[\177", _d=0x7f5be3bf8200 "", mode=0x7f5beafa5188 "4", _cflags=0xffff <error: Cannot access memory at address 0xffff>) at ims_registrar_scscf_mod.c:628 ```
We are running the latest Kamailio 5.6.4 using the official Docker image.
### Analysis
We were able to analyze corresponding core dump using `gdb` and determine the cause of those random crashes. It turns out that it is related to calling the `save` function of IMS Registrar SCSCF module with 2 arguments.
Example configuration: ``` save("REG_SAR_REPLY", "location"); ```
According to documentation, this call is valid, as `mode` and `flags` parameters are optional: https://kamailio.org/docs/modules/5.6.x/modules/ims_registrar_scscf.html#idm...
The `save` function is exported in `ims_registrar_scscf_mod.c` as follows: ``` /*! \brief * Exported functions */ static cmd_export_t cmds[] = { {"save", (cmd_function) w_save, 2, assign_save_fixup3_async, 0, REQUEST_ROUTE | ONREPLY_ROUTE}, {"save", (cmd_function) w_save, 3, assign_save_fixup3_async, 0, REQUEST_ROUTE | ONREPLY_ROUTE}, {"save", (cmd_function) w_save, 4, save_fixup3, free_uint_fixup, REQUEST_ROUTE | ONREPLY_ROUTE}, ... } ```
And implemented as: ``` /*! \brief * Wrapper to save(location) */ static int w_save(struct sip_msg* _m, char* _route, char* _d, char* mode, char* _cflags) { if(_cflags){ return save(_m, _d, _route, ((int)(*_cflags))); }
return save(_m, _d, _route, 0); } ```
Using the 2-argument `save` variant in configuration effectively causes the `w_save` to be called from `src/core/action.c :: do_action` via a 3-argument function-pointer cast: ``` case MODULE2_T: MODF_CALL(cmd_function, h, msg, a->val, (char*)a->val[2].u.data, (char*)a->val[3].u.data ); break; ```
Unfortunately, this cast is inherently unsafe - it leaves the `mode` and `_cflags` undetermined. They are probably effectively bound to some memory area beyond the parameter values of the stack frame corresponding to the function call.
Our guess is that incidentally `_cflags == 0x0000` for most of those calls, due to how the stack is structured. But sometimes `_cflags == 0xFFFF` which satisfies the condition in `w_save`, causes `(int)(*_cflags)` dereference attempt and leads to the segmentation fault we've encountered.
### Workaround
Based on source code analysis, we have determined that always using `save` with a full set of arguments (including optional ones) will result in `w_save` being called via a 5-argument function pointer, which matches its signature and avoids the issue.
Example configuration with workaround applied: ``` save("REG_SAR_REPLY", "location", "0", "0"); ```
After introducing this workaround on target environment we are no longer experiencing the problem.
This issue is stale because it has been open 6 weeks with no activity. Remove stale label or comment or this will be closed in 2 weeks.
Closed #3412 as not planned.