laforge submitted this change.
Fix: Prevent race conditions when accessing slotmap in bankd
If a client connects to a worker, the worker will check if there is an
existing mapping between a reader and this client. If it exists,
slotmap_by_client() will return a pointer. If the mapping is deleted at
this time by the server, the worker uses a pointer to a mapping entry
that has just been freed.
To prevent this, the worker locks the slot map and calls the new
function slotmap_by_client_nolock(). After it has finished working with
the returned pointer, it unlocks the slot map again. A possible delete
by the main thread would be delayed.
Change-Id: I3464726f37beb7c47b4e1f00c018ffa4f3948906
---
M src/bankd/bankd_main.c
M src/slotmap.c
M src/slotmap.h
3 files changed, 21 insertions(+), 9 deletions(-)
diff --git a/src/bankd/bankd_main.c b/src/bankd/bankd_main.c
index 479adf0..fc8f480 100644
--- a/src/bankd/bankd_main.c
+++ b/src/bankd/bankd_main.c
@@ -700,8 +700,10 @@
{
struct slot_mapping *slmap;
- slmap = slotmap_by_client(worker->bankd->slotmaps, &worker->client.clslot);
+ slotmaps_rdlock(worker->bankd->slotmaps);
+ slmap = slotmap_by_client_nolock(worker->bankd->slotmaps, &worker->client.clslot);
if (!slmap) {
+ slotmaps_unlock(worker->bankd->slotmaps);
LOGW(worker, "No slotmap (yet) for client C(%u:%u)\n",
worker->client.clslot.client_id, worker->client.clslot.slot_nr);
/* check in 10s if the map has been installed meanwhile by main thread */
@@ -712,6 +714,7 @@
slmap->client.client_id, slmap->client.slot_nr,
slmap->bank.bank_id, slmap->bank.slot_nr);
worker->slot = slmap->bank;
+ slotmaps_unlock(worker->bankd->slotmaps);
worker_set_state_timeout(worker, BW_ST_CONN_CLIENT_MAPPED, 10);
return worker_open_card(worker);
}
diff --git a/src/slotmap.c b/src/slotmap.c
index b1be0ab..1883cf5 100644
--- a/src/slotmap.c
+++ b/src/slotmap.c
@@ -53,20 +53,27 @@
return (map->bank.bank_id << 16) | map->bank.slot_nr;
}
+/* lookup of map by client:slot */
+struct slot_mapping *slotmap_by_client_nolock(struct slotmaps *maps, const struct client_slot *client)
+{
+ struct slot_mapping *map;
+
+ llist_for_each_entry(map, &maps->mappings, list) {
+ if (client_slot_equals(&map->client, client))
+ return map;
+ }
+ return NULL;
+}
+
/* thread-safe lookup of map by client:slot */
struct slot_mapping *slotmap_by_client(struct slotmaps *maps, const struct client_slot *client)
{
struct slot_mapping *map;
slotmaps_rdlock(maps);
- llist_for_each_entry(map, &maps->mappings, list) {
- if (client_slot_equals(&map->client, client)) {
- slotmaps_unlock(maps);
- return map;
- }
- }
+ map = slotmap_by_client_nolock(maps, client);
slotmaps_unlock(maps);
- return NULL;
+ return map;
}
/* thread-safe lookup of map by bank:slot */
@@ -83,7 +90,6 @@
}
slotmaps_unlock(maps);
return NULL;
-
}
/* thread-safe creating of a new bank<->client map */
diff --git a/src/slotmap.h b/src/slotmap.h
index 3d07c8d..a14e1cf 100644
--- a/src/slotmap.h
+++ b/src/slotmap.h
@@ -70,6 +70,9 @@
uint32_t slotmap_get_id(const struct slot_mapping *map);
+/* lookup of map by client:slot */
+struct slot_mapping *slotmap_by_client_nolock(struct slotmaps *maps, const struct client_slot *client);
+
/* thread-safe lookup of map by client:slot */
struct slot_mapping *slotmap_by_client(struct slotmaps *maps, const struct client_slot *client);
To view, visit change 42664. To unsubscribe, or for help writing mail filters, visit settings.