Skip to content

GitLab

Commit 52afb00f authored by Robert Schmidt's avatar Robert Schmidt
Browse files

Fix: repair RC.mac memory allocation 

* do not test NULL on un-initialized memory
* there can be a race between the FlexRAN agent reading the RC.mac and its
  allocation in main.c
* in order to circumvent this, change the allocation by allocating
  everything "into" a local variable
* if RC.mac is already allocated, reinitialize it without re-allocating memory
* move UE_list init code into dedicated function
parent b34e6d45
Hide whitespace changes
Inline Side-by-side
Showing with 70 additions and 79 deletions
openair2/LAYER2/MAC/mac_proto.h
View file @ 52afb00f
......@@ -181,6 +181,8 @@ void add_msg3(module_id_t module_idP, int CC_id, RA_t * ra, frame_t frameP,
//main.c
void init_UE_list(UE_list_t *UE_list);
int mac_top_init(int eMBMS_active, char *uecap_xer,
uint8_t cba_group_active, uint8_t HO_active);
......
openair2/LAYER2/MAC/main.c
View file @ 52afb00f
......@@ -45,95 +45,84 @@
extern RAN_CONTEXT_t RC;
void init_UE_list(UE_list_t *UE_list)
{
int list_el;
UE_list->num_UEs = 0;
UE_list->head = -1;
UE_list->head_ul = -1;
UE_list->avail = 0;
for (list_el = 0; list_el < MAX_MOBILES_PER_ENB - 1; list_el++) {
UE_list->next[list_el] = list_el + 1;
UE_list->next_ul[list_el] = list_el + 1;
}
UE_list->next[list_el] = -1;
UE_list->next_ul[list_el] = -1;
memset(UE_list->DLSCH_pdu, 0, sizeof(UE_list->DLSCH_pdu));
memset(UE_list->UE_template, 0, sizeof(UE_list->UE_template));
memset(UE_list->eNB_UE_stats, 0, sizeof(UE_list->eNB_UE_stats));
memset(UE_list->UE_sched_ctrl, 0, sizeof(UE_list->UE_sched_ctrl));
memset(UE_list->active, 0, sizeof(UE_list->active));
}
void mac_top_init_eNB(void)
{
module_id_t i, j;
int list_el;
UE_list_t *UE_list;
eNB_MAC_INST *mac;
LOG_I(MAC, "[MAIN] Init function start:nb_macrlc_inst=%d\n",
RC.nb_macrlc_inst);
if (RC.nb_macrlc_inst > 0) {
if (RC.mac == NULL){
RC.mac =
(eNB_MAC_INST **) malloc16(RC.nb_macrlc_inst *
sizeof(eNB_MAC_INST *));
}
AssertFatal(RC.mac != NULL,
"can't ALLOCATE %zu Bytes for %d eNB_MAC_INST with size %zu \n",
RC.nb_macrlc_inst * sizeof(eNB_MAC_INST *),
RC.nb_macrlc_inst, sizeof(eNB_MAC_INST));
for (i = 0; i < RC.nb_macrlc_inst; i++) {
if (RC.mac[i] == NULL) {
RC.mac[i] = (eNB_MAC_INST *) malloc16(sizeof(eNB_MAC_INST));
AssertFatal(RC.mac[i] != NULL,
"can't ALLOCATE %zu Bytes for %d eNB_MAC_INST with size %zu \n",
RC.nb_macrlc_inst * sizeof(eNB_MAC_INST *),
RC.nb_macrlc_inst, sizeof(eNB_MAC_INST));
LOG_D(MAC,
"[MAIN] ALLOCATE %zu Bytes for %d eNB_MAC_INST @ %p\n",
sizeof(eNB_MAC_INST), RC.nb_macrlc_inst, RC.mac);
bzero(RC.mac[i], sizeof(eNB_MAC_INST));
}
RC.mac[i]->Mod_id = i;
for (j = 0; j < MAX_NUM_CCs; j++) {
RC.mac[i]->DL_req[j].dl_config_request_body.
dl_config_pdu_list = RC.mac[i]->dl_config_pdu_list[j];
RC.mac[i]->UL_req[j].ul_config_request_body.
ul_config_pdu_list = RC.mac[i]->ul_config_pdu_list[j];
for (int k = 0; k < 10; k++)
RC.mac[i]->UL_req_tmp[j][k].
ul_config_request_body.ul_config_pdu_list =
RC.mac[i]->ul_config_pdu_list_tmp[j][k];
for(int sf=0;sf<10;sf++){
RC.mac[i]->HI_DCI0_req[j][sf].hi_dci0_request_body.hi_dci0_pdu_list =RC.mac[i]->hi_dci0_pdu_list[j][sf];
}
RC.mac[i]->TX_req[j].tx_request_body.tx_pdu_list =
RC.mac[i]->tx_request_pdu[j];
RC.mac[i]->ul_handle = 0;
}
}
AssertFatal(rlc_module_init() == 0,
"Could not initialize RLC layer\n");
// These should be out of here later
pdcp_layer_init();
rrc_init_global_param();
} else {
RC.mac = NULL;
module_id_t i, j;
eNB_MAC_INST **mac;
LOG_I(MAC, "[MAIN] Init function start:nb_macrlc_inst=%d\n",
RC.nb_macrlc_inst);
if (RC.nb_macrlc_inst <= 0) {
RC.mac = NULL;
return;
}
mac = RC.mac ? RC.mac : malloc16(RC.nb_macrlc_inst * sizeof(eNB_MAC_INST *));
  • Cedric Roux @cedric.roux ·
    Owner

    we need to call bzero in case we call malloc16. So it should be more something like:

      mac = RC.mac;
  if (mac == NULL) {
    mac = malloc16(RC.nb_macrlc_inst * sizeof(eNB_MAC_INST *));
    AssertFatal(mac != NULL,
                "can't ALLOCATE %zu Bytes for %d eNB_MAC_INST with size %zu \n",
                RC.nb_macrlc_inst * sizeof(eNB_MAC_INST *),
                RC.nb_macrlc_inst, sizeof(eNB_MAC_INST));
    bzero(mac, RC.nb_macrlc_inst * sizeof(eNB_MAC_INST *));
  }
  • Robert Schmidt @schmidtr ·
    Developer

    Why do we need to do this? It will be overwritten afterwards anyway, either with the existing RC.mac[i] or with a new malloc().

    Edited by Robert Schmidt
Please register or sign in to reply
AssertFatal(mac != NULL,
"can't ALLOCATE %zu Bytes for %d eNB_MAC_INST with size %zu \n",
RC.nb_macrlc_inst * sizeof(eNB_MAC_INST *),
RC.nb_macrlc_inst, sizeof(eNB_MAC_INST));
for (i = 0; i < RC.nb_macrlc_inst; i++) {
mac[i] = RC.mac && RC.mac[i] ? RC.mac[i] : malloc16(sizeof(eNB_MAC_INST));
AssertFatal(mac[i] != NULL,
"can't ALLOCATE %zu Bytes for %d eNB_MAC_INST with size %zu \n",
RC.nb_macrlc_inst * sizeof(eNB_MAC_INST *),
RC.nb_macrlc_inst, sizeof(eNB_MAC_INST));
LOG_D(MAC,
"[MAIN] ALLOCATE %zu Bytes for %d eNB_MAC_INST @ %p\n",
sizeof(eNB_MAC_INST), RC.nb_macrlc_inst, mac);
bzero(mac[i], sizeof(eNB_MAC_INST));
  • Cedric Roux @cedric.roux ·
    Owner

    Here we should call bzero only if mac[i] was allocated with malloc16 above. As it is we always call it.

    Actually the whole logic of this function is very strange. An init function should be called only once and all those tests with != NULL should not be done. Maybe for some unknown reason it is "legal" to call it several times...

  • Robert Schmidt @schmidtr ·
    Developer

    I didn't have those tests in there initially but included them after commit dc9b0fa5 which did it for some reason.

    Personally I also think it should be called only once. I included the bzero() to overwrite it always as this brings the RC.mac structure back to a "clean" state whenever this is called. If we manage to call it only once (achieved through commit a1ef7159) then we could leave out those tests.

Please register or sign in to reply
mac[i]->Mod_id = i;
for (j = 0; j < MAX_NUM_CCs; j++) {
mac[i]->DL_req[j].dl_config_request_body.dl_config_pdu_list =
mac[i]->dl_config_pdu_list[j];
mac[i]->UL_req[j].ul_config_request_body.ul_config_pdu_list =
mac[i]->ul_config_pdu_list[j];
for (int k = 0; k < 10; k++)
mac[i]->UL_req_tmp[j][k].ul_config_request_body.ul_config_pdu_list =
mac[i]->ul_config_pdu_list_tmp[j][k];
for (int sf = 0; sf < 10; sf++)
mac[i]->HI_DCI0_req[j][sf].hi_dci0_request_body.hi_dci0_pdu_list =
mac[i]->hi_dci0_pdu_list[j][sf];
mac[i]->TX_req[j].tx_request_body.tx_pdu_list = mac[i]->tx_request_pdu[j];
mac[i]->ul_handle = 0;
}
// Initialize Linked-List for Active UEs
for (i = 0; i < RC.nb_macrlc_inst; i++) {
mac = RC.mac[i];
mac[i]->if_inst = IF_Module_init(i);
mac->if_inst = IF_Module_init(i);
init_UE_list(&mac[i]->UE_list);
}
UE_list = &mac->UE_list;
RC.mac = mac;
UE_list->num_UEs = 0;
UE_list->head = -1;
UE_list->head_ul = -1;
UE_list->avail = 0;
AssertFatal(rlc_module_init() == 0,
"Could not initialize RLC layer\n");
for (list_el = 0; list_el < MAX_MOBILES_PER_ENB - 1; list_el++) {
UE_list->next[list_el] = list_el + 1;
UE_list->next_ul[list_el] = list_el + 1;
}
UE_list->next[list_el] = -1;
UE_list->next_ul[list_el] = -1;
}
// These should be out of here later
pdcp_layer_init();
rrc_init_global_param();
}
void mac_init_cell_params(int Mod_idP, int CC_idP)
......
Markdown is supported
0% or .
You are about to add 0 people to the discussion. Proceed with caution.
Finish editing this message first!
Please register or to comment