Project

General

Profile

Bug #2632

osmo-mgw's VTY config modifies the length used for internal endpoints array without reallocating the array

Added by neels 10 days ago. Updated 9 days ago.

Status:
In Progress
Priority:
Urgent
Assignee:
Target version:
-
Start date:
11/10/2017
Due date:
% Done:

50%


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

Related to OsmoMGW - Feature #2631: allow arbitrary endpoint names, be more dynamic in allocation and iteration New 11/10/2017

History

#1 Updated by neels 10 days ago

  • Related to Feature #2631: allow arbitrary endpoint names, be more dynamic in allocation and iteration added

#2 Updated by dexter 9 days 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.

#3 Updated by dexter 9 days ago

  • % Done changed from 0 to 50

Also available in: Atom PDF