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 7 months ago. Updated 4 months ago.

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

100%

Estimated time:

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 iterationResolved2017-11-10

History

#1 Updated by neels 7 months ago

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

#2 Updated by dexter 7 months 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 7 months ago

  • % Done changed from 0 to 50

#4 Updated by dexter 7 months 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.

#5 Updated by dexter 7 months ago

  • Status changed from In Progress to Resolved

#6 Updated by laforge 4 months ago

  • Status changed from Resolved to Closed

Also available in: Atom PDF

Add picture from clipboard (Maximum size: 48.8 MB)