nat: Introduce a config free for the test and fix valgrind issues
The talloc_free on the nat lead to the freeing of the bsc_config
which lead to freeing of the rate_ctr_group. The rate_ctr_group
remained in a global list and the next creation of a bsc_config
would access dead memory. Fix it.
The free routine is only meant to be used by the test, for the
real nat we would need to make sure that all connections and
other state that refers to the cfg is removed/closed first.
Fix various memleaks in the test while we are at it. There are
still some to fix.
==7195== Invalid write of size 4
==7195== at 0x4043171: rate_ctr_group_alloc (linuxlist.h:65)
==7195== by 0x804D893: bsc_config_alloc (bsc_nat_utils.c:174)
==7195== by 0x804B5D2: main (bsc_nat_test.c:954)
==7195== Address 0x4311cbc is 52 bytes inside a block of size 208 free'd
==7195== at 0x4029D28: free (in /usr/lib/valgrind/vgpreload_memcheck-x86-linux.so)
==7195== by 0x4048D98: _talloc_free (talloc.c:609)
==7195== by 0x4052806: talloc_free (talloc.c:578)
==7195== by 0x804B58A: main (bsc_nat_test.c:940)
diff --git a/openbsc/include/openbsc/bsc_nat.h b/openbsc/include/openbsc/bsc_nat.h
index 7a161ff..15e2099 100644
--- a/openbsc/include/openbsc/bsc_nat.h
+++ b/openbsc/include/openbsc/bsc_nat.h
@@ -498,4 +498,10 @@
int bsc_nat_extract_lac(struct bsc_connection *bsc, struct nat_sccp_connection *con,
struct bsc_nat_parsed *parsed, struct msgb *msg);
+
+/*
+ * Use for testing
+ */
+void bsc_nat_free(struct bsc_nat *nat);
+
#endif
diff --git a/openbsc/src/osmo-bsc_nat/bsc_nat_utils.c b/openbsc/src/osmo-bsc_nat/bsc_nat_utils.c
index 93831c4..70ad577 100644
--- a/openbsc/src/osmo-bsc_nat/bsc_nat_utils.c
+++ b/openbsc/src/osmo-bsc_nat/bsc_nat_utils.c
@@ -120,6 +120,32 @@
return nat;
}
+void bsc_nat_free(struct bsc_nat *nat)
+{
+ struct bsc_config *cfg, *tmp;
+ struct bsc_nat_acc_lst *lst, *tmp_lst;
+
+ llist_for_each_entry_safe(cfg, tmp, &nat->bsc_configs, entry)
+ bsc_config_free(cfg);
+ llist_for_each_entry_safe(lst, tmp_lst, &nat->access_lists, list)
+ bsc_nat_acc_lst_delete(lst);
+
+ bsc_nat_num_rewr_entry_adapt(nat, &nat->num_rewr, NULL);
+ bsc_nat_num_rewr_entry_adapt(nat, &nat->num_rewr_post, NULL);
+ bsc_nat_num_rewr_entry_adapt(nat, &nat->sms_clear_tp_srr, NULL);
+ bsc_nat_num_rewr_entry_adapt(nat, &nat->sms_num_rewr, NULL);
+ bsc_nat_num_rewr_entry_adapt(nat, &nat->tpdest_match, NULL);
+
+ osmo_counter_free(nat->stats.sccp.conn);
+ osmo_counter_free(nat->stats.sccp.calls);
+ osmo_counter_free(nat->stats.bsc.reconn);
+ osmo_counter_free(nat->stats.bsc.auth_fail);
+ osmo_counter_free(nat->stats.msc.reconn);
+ osmo_counter_free(nat->stats.ussd.reconn);
+ talloc_free(nat->mgcp_cfg);
+ talloc_free(nat);
+}
+
void bsc_nat_set_msc_ip(struct bsc_nat *nat, const char *ip)
{
bsc_replace_string(nat, &nat->main_dest->ip, ip);
@@ -167,7 +193,9 @@
void bsc_config_free(struct bsc_config *cfg)
{
+ llist_del(&cfg->entry);
rate_ctr_group_free(cfg->stats.ctrg);
+ talloc_free(cfg);
}
static void _add_lac(void *ctx, struct llist_head *list, int _lac)
diff --git a/openbsc/tests/bsc-nat/bsc_nat_test.c b/openbsc/tests/bsc-nat/bsc_nat_test.c
index 6f51707..245b081 100644
--- a/openbsc/tests/bsc-nat/bsc_nat_test.c
+++ b/openbsc/tests/bsc-nat/bsc_nat_test.c
@@ -419,7 +419,7 @@
bsc_config_free(con->cfg);
- talloc_free(nat);
+ bsc_nat_free(nat);
msgb_free(msg);
}
@@ -453,7 +453,7 @@
abort();
}
- talloc_free(nat);
+ bsc_nat_free(nat);
}
static void test_mgcp_allocations(void)
@@ -577,7 +577,7 @@
}
bsc_config_free(bsc->cfg);
- talloc_free(nat);
+ bsc_nat_free(nat);
}
/* test the code to find a given connection */
@@ -610,7 +610,7 @@
}
/* free everything */
- talloc_free(nat);
+ bsc_nat_free(nat);
}
static void test_mgcp_rewrite(void)
@@ -937,7 +937,7 @@
}
msgb_free(msg);
- talloc_free(nat);
+ bsc_nat_free(nat);
}
static void test_dt_filter()
@@ -994,6 +994,9 @@
memset(&cause, 0, sizeof(cause));
bsc_nat_filter_dt(bsc, msg, con, parsed, &cause);
}
+
+ msgb_free(msg);
+ bsc_nat_free(nat);
}
static void test_setup_rewrite()
@@ -1159,7 +1162,7 @@
ARRAY_SIZE(cc_setup_national_again));
msgb_free(out);
bsc_nat_num_rewr_entry_adapt(nat, &nat->num_rewr, NULL);
- talloc_free(nat);
+ bsc_nat_free(nat);
}
static void test_setup_rewrite_prefix(void)
@@ -1208,7 +1211,7 @@
msgb_free(out);
bsc_nat_num_rewr_entry_adapt(nat, &nat->num_rewr, NULL);
- talloc_free(nat);
+ bsc_nat_free(nat);
}
static void test_setup_rewrite_post(void)
@@ -1267,8 +1270,7 @@
verify_msg(out, cc_setup_national, ARRAY_SIZE(cc_setup_national));
msgb_free(out);
- bsc_nat_num_rewr_entry_adapt(nat, &nat->num_rewr, NULL);
- talloc_free(nat);
+ bsc_nat_free(nat);
}
static void test_sms_smsc_rewrite()
@@ -1364,6 +1366,7 @@
verify_msg(out, smsc_rewrite, ARRAY_SIZE(smsc_rewrite));
msgb_free(out);
+ bsc_nat_free(nat);
}
static void test_sms_number_rewrite(void)
@@ -1436,6 +1439,7 @@
verify_msg(out, smsc_rewrite_num_patched_tp_srr,
ARRAY_SIZE(smsc_rewrite_num_patched_tp_srr));
msgb_free(out);
+ bsc_nat_free(nat);
}
static void test_barr_list_parsing(void)
@@ -1502,6 +1506,7 @@
entry->cm_reject_cause, entry->lu_reject_cause);
}
+ rc = bsc_nat_barr_adapt(NULL, &root, NULL);
}
static void test_nat_extract_lac()
@@ -1535,6 +1540,8 @@
/* verify the LAC */
OSMO_ASSERT(con.lac == 8210);
OSMO_ASSERT(con.ci == 50000);
+
+ bsc_nat_free(nat);
}
int main(int argc, char **argv)