Skip to content

Commit 9057f2d

Browse files
committed
fix(mdns): Don't free a browse result already freed by an earlier sync batch
browse_sync() frees sync_result->result whenever result->ttl is 0 at processing time, but sync entries only borrow the node: one packet produces one mdns_browse_sync_t, each queued as its own ACTION_BROWSE_SYNC, and add_browse_result() dedupes within a single batch only. mdns_priv_query_update_result_ttl() takes the minimum, so a goodbye lowers a cached node's TTL to 0 and re-adds it to a fresh batch while an older batch still references it. Processing the older batch frees the node; processing the newer one reads result->ttl from freed memory and frees it again: assert failed: tlsf_free ... block already marked as free mdns_mem_free mdns_mem_caps.c mdns_priv_query_results_free mdns_querier.c browse_sync mdns_browser.c mdns_priv_browse_action mdns_browser.c execute_action mdns_service.c Skip sync entries whose node is no longer linked in browse->result. That covers both the use-after-free read and the second free, and needs no change to the result lifetime or to the sync batch layout.
1 parent cf3f8ef commit 9057f2d

1 file changed

Lines changed: 23 additions & 0 deletions

File tree

components/mdns/mdns_browser.c

Lines changed: 23 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -56,6 +56,24 @@ static void browse_item_free(mdns_browse_t *browse)
5656
mdns_mem_free(browse);
5757
}
5858

59+
/**
60+
* @brief Check that a result node is still linked in the browse cache
61+
*
62+
* Sync entries hold a borrowed pointer to a node in @c browse->result. One node
63+
* can be referenced by several queued sync batches, and the first batch to see
64+
* it with @c ttl==0 detaches and frees it, so a later batch must not use its
65+
* pointer without checking.
66+
*/
67+
static bool result_is_cached(const mdns_browse_t *browse, const mdns_result_t *result)
68+
{
69+
for (const mdns_result_t *r = browse->result; r != NULL; r = r->next) {
70+
if (r == result) {
71+
return true;
72+
}
73+
}
74+
return false;
75+
}
76+
5977
/**
6078
* @brief Deliver browse updates to the user notifier
6179
*
@@ -70,6 +88,11 @@ static void browse_sync(mdns_browse_sync_t *browse_sync)
7088
mdns_browse_result_sync_t *sync_result = browse_sync->sync_result;
7189
while (sync_result) {
7290
mdns_result_t *result = sync_result->result;
91+
if (!result_is_cached(browse, result)) {
92+
// Already removed and freed by an earlier sync batch
93+
sync_result = sync_result->next;
94+
continue;
95+
}
7396
DBG_BROWSE_RESULTS(result, browse_sync->browse);
7497
browse->notifier(result);
7598
if (result->ttl == 0) {

0 commit comments

Comments
 (0)