From 3db9607b146c34b5cd7cccce04bf4442e257f5c8 Mon Sep 17 00:00:00 2001 From: Eneas U de Queiroz Date: Mon, 5 Jul 2021 16:27:19 -0300 Subject: [PATCH] data storage: match SSID when searching ap entry Currenty, ap_array_get_ap returns an entry by matching just the bssid, without checking if the SSID matches. ap_array_find_first_entry does a binary search through the ap set without cheking the SSID, which is bad, as the list is sorted by SSID then by bssid_mac. As a side effect, if you have more than one ssid, the network ap list grows unbounded over time. Signed-off-by: Eneas U de Queiroz --- src/include/datastorage.h | 6 ++++-- src/storage/datastorage.c | 24 +++++++++++++++++------- src/test/test_storage.c | 10 +++++----- src/utils/ubus.c | 13 +++++++++---- 4 files changed, 35 insertions(+), 18 deletions(-) diff --git a/src/include/datastorage.h b/src/include/datastorage.h index b4953f8..052e2a2 100644 --- a/src/include/datastorage.h +++ b/src/include/datastorage.h @@ -122,12 +122,15 @@ extern struct probe_metric_s dawn_metric; /* Probe, Auth, Assoc */ +#define SSID_MAX_LEN 32 + // ---------------- Structs ---------------- typedef struct probe_entry_s { struct probe_entry_s* next_probe; struct probe_entry_s* next_probe_skip; struct dawn_mac client_addr; struct dawn_mac bssid_addr; + uint8_t ssid[SSID_MAX_LEN + 1]; // parse_to_beacon_rep() struct dawn_mac target_addr; // TODO: Never evaluated? uint32_t signal; // eval_probe_metric() uint32_t freq; // eval_probe_metric() @@ -170,7 +173,6 @@ typedef struct auth_entry_s assoc_entry; // ---------------- Defines ---------------- -#define SSID_MAX_LEN 32 #define NEIGHBOR_REPORT_LEN 200 // ---------------- Global variables ---------------- @@ -305,7 +307,7 @@ void remove_old_ap_entries(time_t current_time, long long int threshold); void print_ap_array(); -ap *ap_array_get_ap(struct dawn_mac bssid_mac); +ap *ap_array_get_ap(struct dawn_mac bssid_mac, const uint8_t* ssid); int probe_array_set_all_probe_count(struct dawn_mac client_addr, uint32_t probe_count); diff --git a/src/storage/datastorage.c b/src/storage/datastorage.c index ae84588..2813b63 100644 --- a/src/storage/datastorage.c +++ b/src/storage/datastorage.c @@ -158,7 +158,7 @@ static probe_entry** probe_array_find_first_entry(struct dawn_mac client_mac, st return lo_ptr; } -static ap** ap_array_find_first_entry(struct dawn_mac bssid_mac) +static ap** ap_array_find_first_entry(struct dawn_mac bssid_mac, const uint8_t* ssid) { int lo = 0; ap** lo_ptr = &ap_set; @@ -167,6 +167,7 @@ static ap** ap_array_find_first_entry(struct dawn_mac bssid_mac) while (lo < hi) { ap** i = lo_ptr; int scan_pos = lo; + int this_cmp; // m is next test position of binary search int m = (lo + hi) / 2; @@ -177,7 +178,15 @@ static ap** ap_array_find_first_entry(struct dawn_mac bssid_mac) i = &((*i)->next_ap); } - int this_cmp = mac_compare_bb((*i)->bssid_addr, bssid_mac); + if (ssid) + { + this_cmp = strcmp((char*)(*i)->ssid, (char*)ssid); + } + else + { + this_cmp = 0; + } + this_cmp = this_cmp ? this_cmp : mac_compare_bb((*i)->bssid_addr, bssid_mac); if (this_cmp < 0) { @@ -489,7 +498,7 @@ int better_ap_available(ap *kicking_ap, struct dawn_mac client_mac, char* neighb continue; } - ap* candidate_ap = ap_array_get_ap(i->bssid_addr); + ap* candidate_ap = ap_array_get_ap(i->bssid_addr, kicking_ap->ssid); if (candidate_ap == NULL) { i = i->next_probe; @@ -1060,10 +1069,11 @@ ap *insert_to_ap_array(ap* entry, time_t expiry) { // TODO: Why do we delete and add here? - ap* old_entry = *ap_array_find_first_entry(entry->bssid_addr); + ap* old_entry = *ap_array_find_first_entry(entry->bssid_addr, entry->ssid); if (old_entry != NULL && - !mac_is_equal_bb((old_entry)->bssid_addr, entry->bssid_addr)) + !mac_is_equal_bb((old_entry)->bssid_addr, entry->bssid_addr) && + !strcmp((char*)old_entry->ssid, (char*)entry->ssid)) old_entry = NULL; if (old_entry != NULL) @@ -1116,11 +1126,11 @@ void ap_array_insert(ap* entry) { } } -ap* ap_array_get_ap(struct dawn_mac bssid_mac) { +ap* ap_array_get_ap(struct dawn_mac bssid_mac, const uint8_t* ssid) { pthread_mutex_lock(&ap_array_mutex); - ap* ret = *ap_array_find_first_entry(bssid_mac); + ap* ret = *ap_array_find_first_entry(bssid_mac, ssid); pthread_mutex_unlock(&ap_array_mutex); diff --git a/src/test/test_storage.c b/src/test/test_storage.c index 543a9a1..69d24c2 100644 --- a/src/test/test_storage.c +++ b/src/test/test_storage.c @@ -170,7 +170,7 @@ static int array_auto_helper(int action, int i0, int i1) time_moves_on(); } else - ap_array_delete(ap_array_get_ap(this_mac)); + ap_array_delete(ap_array_get_ap(this_mac, NULL)); break; case HELPER_CLIENT: ; // Empty statement to allow label before declaration @@ -938,7 +938,7 @@ static int consume_actions(int argc, char* argv[], int harness_verbosity) hwaddr_aton(argv[1], kick_mac.u8); load_u32(&kick_id, argv[2]); - while ((kick_clients(ap_array_get_ap(kick_mac), kick_id) != 0) && safety_count--); + while ((kick_clients(ap_array_get_ap(kick_mac, NULL), kick_id) != 0) && safety_count--); } } else if (strcmp(*argv, "better_ap_available") == 0) @@ -971,11 +971,11 @@ static int consume_actions(int argc, char* argv[], int harness_verbosity) strcpy(nb, argv[4]); } - tr = better_ap_available(ap_array_get_ap(bssid_mac), client_mac, nb); + tr = better_ap_available(ap_array_get_ap(bssid_mac, NULL), client_mac, nb); } else { - tr = better_ap_available(ap_array_get_ap(bssid_mac), client_mac, NULL); + tr = better_ap_available(ap_array_get_ap(bssid_mac, NULL), client_mac, NULL); } printf("better_ap_available returned %d (with neighbour report %s)\n", tr, nb); @@ -1000,7 +1000,7 @@ static int consume_actions(int argc, char* argv[], int harness_verbosity) } else { - ap* ap_entry = ap_array_get_ap(pr0->bssid_addr); + ap* ap_entry = ap_array_get_ap(pr0->bssid_addr, NULL); int this_metric = eval_probe_metric(pr0, ap_entry); printf("eval_probe_metric: Returned %d\n", this_metric); diff --git a/src/utils/ubus.c b/src/utils/ubus.c index c8153b0..5a7eaa8 100644 --- a/src/utils/ubus.c +++ b/src/utils/ubus.c @@ -145,6 +145,7 @@ enum { BEACON_REP_BSSID, BEACON_REP_ANTENNA_ID, BEACON_REP_PARENT_TSF, + BEACON_REP_SSID, __BEACON_REP_MAX, }; @@ -160,6 +161,7 @@ static const struct blobmsg_policy beacon_rep_policy[__BEACON_REP_MAX] = { [BEACON_REP_BSSID] = {.name = "bssid", .type = BLOBMSG_TYPE_STRING}, [BEACON_REP_ANTENNA_ID] = {.name = "antenna-id", .type = BLOBMSG_TYPE_INT16}, [BEACON_REP_PARENT_TSF] = {.name = "parent-tsf", .type = BLOBMSG_TYPE_INT16}, + [BEACON_REP_SSID] = {.name = "ssid", .type = BLOBMSG_TYPE_STRING}, }; enum { @@ -272,7 +274,7 @@ static int decide_function(probe_entry *prob_req, int req_type) { // TODO: Bug? This results in copious "Neigbor-Report is NULL" messages! // find own probe entry and calculate score - ap* this_ap = ap_array_get_ap(prob_req->bssid_addr); + ap* this_ap = ap_array_get_ap(prob_req->bssid_addr, prob_req->ssid); if (this_ap != NULL && better_ap_available(this_ap, prob_req->client_addr, NULL)) { return 0; } @@ -330,7 +332,8 @@ int parse_to_beacon_rep(struct blob_attr *msg) { return -1; } - ap *ap_entry_rep = ap_array_get_ap(msg_bssid); + const uint8_t *ssid = (const uint8_t*)blobmsg_get_string(tb[BEACON_REP_SSID]); + ap *ap_entry_rep = ap_array_get_ap(msg_bssid, ssid); // no client from network!! if (ap_entry_rep == NULL) { @@ -361,6 +364,8 @@ int parse_to_beacon_rep(struct blob_attr *msg) { beacon_rep->next_probe = NULL; beacon_rep->bssid_addr = msg_bssid; beacon_rep->client_addr = msg_client; + strncpy((char*)beacon_rep->ssid, (char*)ssid, SSID_MAX_LEN); + beacon_rep->ssid[SSID_MAX_LEN] = '\0'; beacon_rep->counter = dawn_metric.min_probe_count; hwaddr_aton(blobmsg_data(tb[BEACON_REP_ADDR]), beacon_rep->target_addr.u8); // TODO: What is this for? beacon_rep->signal = 0; @@ -1397,7 +1402,7 @@ int build_hearing_map_sort_client(struct blob_buf *b) { ssid_list = blobmsg_open_table(b, (char*)m->ssid); probe_entry* i = probe_set; while (i != NULL) { - ap *ap_entry_i = ap_array_get_ap(i->bssid_addr); + ap *ap_entry_i = ap_array_get_ap(i->bssid_addr, m->ssid); if (ap_entry_i == NULL) { i = i->next_probe; @@ -1416,7 +1421,7 @@ int build_hearing_map_sort_client(struct blob_buf *b) { k != NULL && mac_is_equal_bb(k->client_addr, i->client_addr); k = k->next_probe) { - ap *ap_k = ap_array_get_ap(k->bssid_addr); + ap *ap_k = ap_array_get_ap(k->bssid_addr, m->ssid); if (ap_k == NULL || strcmp((char*)ap_k->ssid, (char*)m->ssid) != 0) { continue;