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 <cotequeiroz@gmail.com>
This commit is contained in:
Eneas U de Queiroz 2021-07-05 16:27:19 -03:00 committed by Polynomdivision
parent a22f5a79d6
commit 3db9607b14
4 changed files with 35 additions and 18 deletions

View file

@ -122,12 +122,15 @@ extern struct probe_metric_s dawn_metric;
/* Probe, Auth, Assoc */ /* Probe, Auth, Assoc */
#define SSID_MAX_LEN 32
// ---------------- Structs ---------------- // ---------------- Structs ----------------
typedef struct probe_entry_s { typedef struct probe_entry_s {
struct probe_entry_s* next_probe; struct probe_entry_s* next_probe;
struct probe_entry_s* next_probe_skip; struct probe_entry_s* next_probe_skip;
struct dawn_mac client_addr; struct dawn_mac client_addr;
struct dawn_mac bssid_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? struct dawn_mac target_addr; // TODO: Never evaluated?
uint32_t signal; // eval_probe_metric() uint32_t signal; // eval_probe_metric()
uint32_t freq; // eval_probe_metric() uint32_t freq; // eval_probe_metric()
@ -170,7 +173,6 @@ typedef struct auth_entry_s assoc_entry;
// ---------------- Defines ---------------- // ---------------- Defines ----------------
#define SSID_MAX_LEN 32
#define NEIGHBOR_REPORT_LEN 200 #define NEIGHBOR_REPORT_LEN 200
// ---------------- Global variables ---------------- // ---------------- Global variables ----------------
@ -305,7 +307,7 @@ void remove_old_ap_entries(time_t current_time, long long int threshold);
void print_ap_array(); 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); int probe_array_set_all_probe_count(struct dawn_mac client_addr, uint32_t probe_count);

View file

@ -158,7 +158,7 @@ static probe_entry** probe_array_find_first_entry(struct dawn_mac client_mac, st
return lo_ptr; 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; int lo = 0;
ap** lo_ptr = &ap_set; ap** lo_ptr = &ap_set;
@ -167,6 +167,7 @@ static ap** ap_array_find_first_entry(struct dawn_mac bssid_mac)
while (lo < hi) { while (lo < hi) {
ap** i = lo_ptr; ap** i = lo_ptr;
int scan_pos = lo; int scan_pos = lo;
int this_cmp;
// m is next test position of binary search // m is next test position of binary search
int m = (lo + hi) / 2; 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); 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) if (this_cmp < 0)
{ {
@ -489,7 +498,7 @@ int better_ap_available(ap *kicking_ap, struct dawn_mac client_mac, char* neighb
continue; 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) { if (candidate_ap == NULL) {
i = i->next_probe; 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? // 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 && 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; old_entry = NULL;
if (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); 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); pthread_mutex_unlock(&ap_array_mutex);

View file

@ -170,7 +170,7 @@ static int array_auto_helper(int action, int i0, int i1)
time_moves_on(); time_moves_on();
} }
else else
ap_array_delete(ap_array_get_ap(this_mac)); ap_array_delete(ap_array_get_ap(this_mac, NULL));
break; break;
case HELPER_CLIENT: case HELPER_CLIENT:
; // Empty statement to allow label before declaration ; // 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); hwaddr_aton(argv[1], kick_mac.u8);
load_u32(&kick_id, argv[2]); 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) 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]); 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 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); 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 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); int this_metric = eval_probe_metric(pr0, ap_entry);
printf("eval_probe_metric: Returned %d\n", this_metric); printf("eval_probe_metric: Returned %d\n", this_metric);

View file

@ -145,6 +145,7 @@ enum {
BEACON_REP_BSSID, BEACON_REP_BSSID,
BEACON_REP_ANTENNA_ID, BEACON_REP_ANTENNA_ID,
BEACON_REP_PARENT_TSF, BEACON_REP_PARENT_TSF,
BEACON_REP_SSID,
__BEACON_REP_MAX, __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_BSSID] = {.name = "bssid", .type = BLOBMSG_TYPE_STRING},
[BEACON_REP_ANTENNA_ID] = {.name = "antenna-id", .type = BLOBMSG_TYPE_INT16}, [BEACON_REP_ANTENNA_ID] = {.name = "antenna-id", .type = BLOBMSG_TYPE_INT16},
[BEACON_REP_PARENT_TSF] = {.name = "parent-tsf", .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 { 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! // TODO: Bug? This results in copious "Neigbor-Report is NULL" messages!
// find own probe entry and calculate score // 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)) { if (this_ap != NULL && better_ap_available(this_ap, prob_req->client_addr, NULL)) {
return 0; return 0;
} }
@ -330,7 +332,8 @@ int parse_to_beacon_rep(struct blob_attr *msg) {
return -1; 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!! // no client from network!!
if (ap_entry_rep == NULL) { 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->next_probe = NULL;
beacon_rep->bssid_addr = msg_bssid; beacon_rep->bssid_addr = msg_bssid;
beacon_rep->client_addr = msg_client; 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; 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? hwaddr_aton(blobmsg_data(tb[BEACON_REP_ADDR]), beacon_rep->target_addr.u8); // TODO: What is this for?
beacon_rep->signal = 0; 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); ssid_list = blobmsg_open_table(b, (char*)m->ssid);
probe_entry* i = probe_set; probe_entry* i = probe_set;
while (i != NULL) { 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) { if (ap_entry_i == NULL) {
i = i->next_probe; 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 != NULL && mac_is_equal_bb(k->client_addr, i->client_addr);
k = k->next_probe) { 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) { if (ap_k == NULL || strcmp((char*)ap_k->ssid, (char*)m->ssid) != 0) {
continue; continue;