Review at  https://gerrit.osmocom.org/2667

host/mobile: use talloc for ms->name allocation

The approach of talloc memory management reduces memory usage,
and prevents some buffer overflows, which were possible before.

Change-Id: Icd6706117fdd7f1b3481b0e3817bbb3b31f12f60
---
M src/host/layer23/include/osmocom/bb/common/osmocom_data.h
M src/host/layer23/src/common/main.c
M src/host/layer23/src/mobile/app_mobile.c
M src/host/layer23/src/mobile/gsm322.c
M src/host/layer23/src/mobile/vty_interface.c
5 files changed, 57 insertions(+), 33 deletions(-)


  git pull ssh://gerrit.osmocom.org:29418/osmocom-bb refs/changes/67/2667/1

diff --git a/src/host/layer23/include/osmocom/bb/common/osmocom_data.h 
b/src/host/layer23/include/osmocom/bb/common/osmocom_data.h
index 17dad10..9b544ab 100644
--- a/src/host/layer23/include/osmocom/bb/common/osmocom_data.h
+++ b/src/host/layer23/include/osmocom/bb/common/osmocom_data.h
@@ -57,7 +57,7 @@
 /* One Mobilestation for osmocom */
 struct osmocom_ms {
        struct llist_head entity;
-       char name[32];
+       char *name;
        struct osmo_wqueue l2_wq, sap_wq;
        uint16_t test_arfcn;
        struct osmol1_entity l1_entity;
diff --git a/src/host/layer23/src/common/main.c 
b/src/host/layer23/src/common/main.c
index 59cee03..693b10e 100644
--- a/src/host/layer23/src/common/main.c
+++ b/src/host/layer23/src/common/main.c
@@ -246,8 +246,7 @@
 
        llist_add_tail(&ms->entity, &ms_list);
 
-       sprintf(ms->name, "1");
-
+       ms->name = talloc_strdup(ms, "1");
        ms->test_arfcn = 871;
 
        handle_options(argc, argv);
diff --git a/src/host/layer23/src/mobile/app_mobile.c 
b/src/host/layer23/src/mobile/app_mobile.c
index c74a93f..7dc7208 100644
--- a/src/host/layer23/src/mobile/app_mobile.c
+++ b/src/host/layer23/src/mobile/app_mobile.c
@@ -248,18 +248,21 @@
 struct osmocom_ms *mobile_new(char *name)
 {
        static struct osmocom_ms *ms;
+       char *mncc_name;
 
        ms = talloc_zero(l23_ctx, struct osmocom_ms);
        if (!ms) {
                fprintf(stderr, "Failed to allocate MS\n");
                return NULL;
        }
-       llist_add_tail(&ms->entity, &ms_list);
 
-       strcpy(ms->name, name);
-
+       talloc_set_name(ms, "ms_%s", name);
+       ms->name = talloc_strdup(ms, name);
        ms->l2_wq.bfd.fd = -1;
        ms->sap_wq.bfd.fd = -1;
+
+       /* Register a new MS */
+       llist_add_tail(&ms->entity, &ms_list);
 
        gsm_support_init(ms);
        gsm_settings_init(ms);
@@ -267,13 +270,12 @@
        ms->shutdown = 3; /* being down */
 
        if (mncc_recv_app) {
-               char name[32];
-
-               sprintf(name, "/tmp/ms_mncc_%s", ms->name);
+               mncc_name = talloc_asprintf(ms, "/tmp/ms_mncc_%s", ms->name);
 
                ms->mncc_entity.mncc_recv = mncc_recv_app;
-               ms->mncc_entity.sock_state = mncc_sock_init(ms, name, l23_ctx);
+               ms->mncc_entity.sock_state = mncc_sock_init(ms, mncc_name, 
l23_ctx);
 
+               talloc_free(mncc_name);
        } else if (ms->settings.ch_cap == GSM_CAP_SDCCH)
                ms->mncc_entity.mncc_recv = mncc_recv_dummy;
        else
diff --git a/src/host/layer23/src/mobile/gsm322.c 
b/src/host/layer23/src/mobile/gsm322.c
index 325852d..f0b31ff 100644
--- a/src/host/layer23/src/mobile/gsm322.c
+++ b/src/host/layer23/src/mobile/gsm322.c
@@ -5030,7 +5030,7 @@
        struct gsm322_plmn *plmn = &ms->plmn;
        struct gsm322_cellsel *cs = &ms->cellsel;
        FILE *fp;
-       char filename[PATH_MAX];
+       char *ba_filename;
        int i;
        struct gsm322_ba_list *ba;
        uint8_t buf[4];
@@ -5062,8 +5062,14 @@
                        cs->list[i].flags |= GSM322_CS_FLAG_SUPPORT;
 
        /* read BA list */
-       sprintf(filename, "%s/%s.ba", config_dir, ms->name);
-       fp = fopen(filename, "r");
+       ba_filename = talloc_asprintf(ms, "%s/%s.ba", config_dir, ms->name);
+       if (!ba_filename) {
+               LOGP(DCS, LOGL_ERROR, "Failed to read BA list: "
+                       "couldn't allocate memory\n");
+               return -ENOMEM;
+       }
+
+       fp = fopen(ba_filename, "r");
        if (fp) {
                int rc;
                char *s_rc;
@@ -5100,6 +5106,7 @@
        } else
                LOGP(DCS, LOGL_INFO, "No stored BA list\n");
 
+       talloc_free(ba_filename);
        return 0;
 }
 
@@ -5110,7 +5117,7 @@
        struct llist_head *lh, *lh2;
        struct msgb *msg;
        FILE *fp;
-       char filename[PATH_MAX];
+       char *ba_filename;
        struct gsm322_ba_list *ba;
        uint8_t buf[4];
        int i;
@@ -5138,25 +5145,33 @@
        }
 
        /* store BA list */
-       sprintf(filename, "%s/%s.ba", config_dir, ms->name);
-       fp = fopen(filename, "w");
-       if (fp) {
-               fputs(ba_version, fp);
-               llist_for_each_entry(ba, &cs->ba_list, entry) {
-                       buf[0] = ba->mcc >> 8;
-                       buf[1] = ba->mcc & 0xff;
-                       buf[2] = ba->mnc >> 8;
-                       buf[3] = ba->mnc & 0xff;
-                       fwrite(buf, 4, 1, fp);
-                       fwrite(ba->freq, sizeof(ba->freq), 1, fp);
-                       LOGP(DCS, LOGL_INFO, "Write stored BA list (mcc=%s "
-                               "mnc=%s  %s, %s)\n", gsm_print_mcc(ba->mcc),
-                               gsm_print_mnc(ba->mnc), gsm_get_mcc(ba->mcc),
-                               gsm_get_mnc(ba->mcc, ba->mnc));
+       ba_filename = talloc_asprintf(ms, "%s/%s.ba", config_dir, ms->name);
+       if (ba_filename) {
+               fp = fopen(ba_filename, "w");
+               if (fp) {
+                       fputs(ba_version, fp);
+                       llist_for_each_entry(ba, &cs->ba_list, entry) {
+                               buf[0] = ba->mcc >> 8;
+                               buf[1] = ba->mcc & 0xff;
+                               buf[2] = ba->mnc >> 8;
+                               buf[3] = ba->mnc & 0xff;
+                               fwrite(buf, 4, 1, fp);
+                               fwrite(ba->freq, sizeof(ba->freq), 1, fp);
+                               LOGP(DCS, LOGL_INFO, "Write stored BA list 
(mcc=%s "
+                                       "mnc=%s  %s, %s)\n", 
gsm_print_mcc(ba->mcc),
+                                       gsm_print_mnc(ba->mnc), 
gsm_get_mcc(ba->mcc),
+                                       gsm_get_mnc(ba->mcc, ba->mnc));
+                       }
+                       fclose(fp);
+               } else {
+                       LOGP(DCS, LOGL_ERROR, "Failed to write BA list: "
+                               "couldn't open file '%s' for write\n" % 
ba_filename);
                }
-               fclose(fp);
-       } else
-               LOGP(DCS, LOGL_ERROR, "Failed to write BA list\n");
+               talloc_free(ba_filename);
+       } else {
+               LOGP(DCS, LOGL_ERROR, "Failed to write BA list: "
+                       "couldn't allocate memory\n");
+       }
 
        /* free lists */
        while ((msg = msgb_dequeue(&plmn->event_queue)))
diff --git a/src/host/layer23/src/mobile/vty_interface.c 
b/src/host/layer23/src/mobile/vty_interface.c
index 78d136d..d6591d3 100644
--- a/src/host/layer23/src/mobile/vty_interface.c
+++ b/src/host/layer23/src/mobile/vty_interface.c
@@ -1251,6 +1251,7 @@
 {
        struct osmocom_ms *ms;
        int found = 0;
+       char *name;
 
        llist_for_each_entry(ms, &ms_list, entity) {
                if (!strcmp(ms->name, argv[0])) {
@@ -1265,7 +1266,14 @@
                return CMD_WARNING;
        }
 
-       strncpy(ms->name, argv[1], sizeof(ms->name) - 1);
+       name = talloc_strdup(ms, argv[1]);
+       if (name) {
+               talloc_free(ms->name);
+               ms->name = name;
+       } else {
+               vty_out(vty, "Couldn't rename MS: no memory%s", VTY_NEWLINE);
+               return CMD_WARNING;
+       }
 
        return CMD_SUCCESS;
 }

-- 
To view, visit https://gerrit.osmocom.org/2667
To unsubscribe, visit https://gerrit.osmocom.org/settings

Gerrit-MessageType: newchange
Gerrit-Change-Id: Icd6706117fdd7f1b3481b0e3817bbb3b31f12f60
Gerrit-PatchSet: 1
Gerrit-Project: osmocom-bb
Gerrit-Branch: master
Gerrit-Owner: Vadim Yanitskiy <[email protected]>

Reply via email to