Bug #2632
closedosmo-mgw's VTY config modifies the length used for internal endpoints array without reallocating the array
100%
Description
the option 'mgcp' / 'number endpoints 123' currently tells osmo-mgw how many endpoints to keep ready. So far it does so by allocating an array of endpoint structs, upon reading the config file, in mgcp_parse_config().
However, the 'number endpoints' command, when issued on a telnet VTY, will modify g_cfg->trunk.number_endpoints, which is used everywhere as the length indicator for the endpoints array. This VTY command does not reallocate the endpoints array, so it could easily make osmo-mgw believe it had 10000 endpoints even though only 32 have been allocated, and it would happily iterate over that memory it doesn't own.
The first fix is to separately store the configuration gotten from VTY and the actual length of the endpoints array. My preference would also be to keep the allocate_trunk() function outside of mgcp_vty.c, so that the VTY is plain for parsing config items, and the actions are separate ... but that might just be my taste.
A more profound fix would be #2631 -- but until we implement that, this issue here needs to be fixed urgently.
Related issues
Updated by neels over 6 years ago
- Related to Feature #2631: allow arbitrary endpoint names, be more dynamic in allocation and iteration added
Updated by dexter over 6 years ago
- Status changed from New to In Progress
The function allocate_trunk() is a bit missleading. It does nothing else than checking a return code and printing an error message. I have removed it now. The actual allocation happens outside the VTY. See patch: https://gerrit.osmocom.org/4775
Unfortunately this is not simple to fix. We could use malloc_realloc to increase the array size, but it is not easy to shrink the array again if someone decides to have a lower number of endpoints at runtime. The problem is that we can not easily know which endpoint is still used and which is not. And even if we know that, we might run into trouble because one endpoint at the very end of the array might still be in use when we want to shrink the array size.
I have now implemented a mechanism which stores the changed endpoint number. Next time the config file is written, the new value is written to the config. After the next restart the change will take effect. Patch is here https://gerrit.osmocom.org/4779
However, I think it is a very odd scenario to change the endpoint numbers on an MGW while calls are busy. I really think we should go for a list here, then things will become a lot easier then.
Updated by dexter over 6 years ago
- % Done changed from 50 to 100
I corrected the patch as neels suggested it. I think this is a solution we can live with for now.