mdns: fix possible crash when packet scheduled to transmit contained service which might have been already removed

packets scheduled to transmit are pushed to action queue and removed from tx_queue_head structure, which is searched for all remaining services and while service is removed, then service questions/asnwers are also removed from this structure. This update fixes possible crash when packet is pushed to action queue, and when service is removed, its answers are removed from tx_queue_head, but not from action queue. this could lead to a crash when the packet is poped from action queue containing questions/answers to already removed (freed) service

Closes IDF-504
This commit is contained in:
David Cermak 2019-02-25 14:29:39 +01:00
parent edb0374b9d
commit 35a30072f4
2 changed files with 22 additions and 3 deletions

View File

@ -3819,7 +3819,17 @@ static void _mdns_execute_action(mdns_action_t * action)
_mdns_search_finish(action->data.search_add.search); _mdns_search_finish(action->data.search_add.search);
break; break;
case ACTION_TX_HANDLE: case ACTION_TX_HANDLE:
_mdns_tx_handle_packet(action->data.tx_handle.packet); {
mdns_tx_packet_t * p = _mdns_server->tx_queue_head;
// packet to be handled should be at tx head, but must be consistent with the one pushed to action queue
if (p && p==action->data.tx_handle.packet && p->queued) {
p->queued = false; // clearing, as the packet might be reused (pushed and transmitted again)
_mdns_server->tx_queue_head = p->next;
_mdns_tx_handle_packet(p);
} else {
ESP_LOGD(TAG, "Skipping transmit of an unexpected packet!");
}
}
break; break;
case ACTION_RX_HANDLE: case ACTION_RX_HANDLE:
mdns_parse_packet(action->data.rx_handle.packet); mdns_parse_packet(action->data.rx_handle.packet);
@ -3856,6 +3866,10 @@ static esp_err_t _mdns_send_search_action(mdns_action_type_t type, mdns_search_o
/** /**
* @brief Called from timer task to run mDNS responder * @brief Called from timer task to run mDNS responder
*
* periodically checks first unqueued packet (from tx head).
* if it is scheduled to be transmitted, then pushes the packet to action queue to be handled.
*
*/ */
static void _mdns_scheduler_run() static void _mdns_scheduler_run()
{ {
@ -3863,6 +3877,10 @@ static void _mdns_scheduler_run()
mdns_tx_packet_t * p = _mdns_server->tx_queue_head; mdns_tx_packet_t * p = _mdns_server->tx_queue_head;
mdns_action_t * action = NULL; mdns_action_t * action = NULL;
// find first unqueued packet
while (p && p->queued) {
p = p->next;
}
if (!p) { if (!p) {
MDNS_SERVICE_UNLOCK(); MDNS_SERVICE_UNLOCK();
return; return;
@ -3870,12 +3888,12 @@ static void _mdns_scheduler_run()
if ((int32_t)(p->send_at - (xTaskGetTickCount() * portTICK_PERIOD_MS)) < 0) { if ((int32_t)(p->send_at - (xTaskGetTickCount() * portTICK_PERIOD_MS)) < 0) {
action = (mdns_action_t *)malloc(sizeof(mdns_action_t)); action = (mdns_action_t *)malloc(sizeof(mdns_action_t));
if (action) { if (action) {
_mdns_server->tx_queue_head = p->next;
action->type = ACTION_TX_HANDLE; action->type = ACTION_TX_HANDLE;
action->data.tx_handle.packet = p; action->data.tx_handle.packet = p;
p->queued = true;
if (xQueueSend(_mdns_server->action_queue, &action, (portTickType)0) != pdPASS) { if (xQueueSend(_mdns_server->action_queue, &action, (portTickType)0) != pdPASS) {
free(action); free(action);
_mdns_server->tx_queue_head = p; p->queued = false;
} }
} else { } else {
HOOK_MALLOC_FAILED; HOOK_MALLOC_FAILED;

View File

@ -289,6 +289,7 @@ typedef struct mdns_tx_packet_s {
mdns_out_answer_t * answers; mdns_out_answer_t * answers;
mdns_out_answer_t * servers; mdns_out_answer_t * servers;
mdns_out_answer_t * additional; mdns_out_answer_t * additional;
bool queued;
} mdns_tx_packet_t; } mdns_tx_packet_t;
typedef struct { typedef struct {