Skip to content

Commit 3f97a82

Browse files
committed
fix(mdns): Fix API races removing txt item for services
Fixes **API race issue** (described in 8a69050) for API mdns_service_txt_item_remove_for_host()
1 parent c62b920 commit 3f97a82

File tree

2 files changed

+48
-74
lines changed

2 files changed

+48
-74
lines changed

components/mdns/mdns.c

Lines changed: 48 additions & 69 deletions
Original file line numberDiff line numberDiff line change
@@ -5111,9 +5111,6 @@ static void _mdns_free_action(mdns_action_t *action)
51115111
case ACTION_SERVICE_INSTANCE_SET:
51125112
free(action->data.srv_instance.instance);
51135113
break;
5114-
case ACTION_SERVICE_TXT_DEL:
5115-
free(action->data.srv_txt_del.key);
5116-
break;
51175114
case ACTION_SERVICE_SUBTYPE_ADD:
51185115
free(action->data.srv_subtype_add.subtype);
51195116
break;
@@ -5159,10 +5156,8 @@ static void _mdns_execute_action(mdns_action_t *action)
51595156
{
51605157
mdns_srv_item_t *a = NULL;
51615158
mdns_service_t *service;
5162-
char *key;
51635159
char *subtype;
51645160
mdns_subtype_t *subtype_item;
5165-
mdns_txt_linked_item_t *txt, * t;
51665161

51675162
switch (action->type) {
51685163
case ACTION_SYSTEM_EVENT:
@@ -5192,37 +5187,6 @@ static void _mdns_execute_action(mdns_action_t *action)
51925187
action->data.srv_instance.service->service->instance = action->data.srv_instance.instance;
51935188
_mdns_probe_all_pcbs(&action->data.srv_instance.service, 1, false, false);
51945189

5195-
break;
5196-
case ACTION_SERVICE_TXT_DEL:
5197-
service = action->data.srv_txt_del.service->service;
5198-
key = action->data.srv_txt_del.key;
5199-
txt = service->txt;
5200-
if (!txt) {
5201-
break;
5202-
}
5203-
if (strcmp(txt->key, key) == 0) {
5204-
service->txt = txt->next;
5205-
free((char *)txt->key);
5206-
free((char *)txt->value);
5207-
free(txt);
5208-
} else {
5209-
while (txt->next) {
5210-
if (strcmp(txt->next->key, key) == 0) {
5211-
t = txt->next;
5212-
txt->next = t->next;
5213-
free((char *)t->key);
5214-
free((char *)t->value);
5215-
free(t);
5216-
break;
5217-
} else {
5218-
txt = txt->next;
5219-
}
5220-
}
5221-
}
5222-
free(key);
5223-
5224-
_mdns_announce_all_pcbs(&action->data.srv_txt_del.service, 1, false);
5225-
52265190
break;
52275191
case ACTION_SERVICE_SUBTYPE_ADD:
52285192
service = action->data.srv_subtype_add.service->service;
@@ -6248,6 +6212,7 @@ esp_err_t mdns_service_txt_item_set_for_host_with_explicit_value_len(const char
62486212
{
62496213
MDNS_SERVICE_LOCK();
62506214
esp_err_t ret = ESP_OK;
6215+
char *value = NULL;
62516216
const char *hostname = host ? host : _mdns_server->hostname;
62526217
ESP_GOTO_ON_FALSE(_mdns_server && _mdns_server->services && !_str_null_or_empty(service) && !_str_null_or_empty(proto) && !_str_null_or_empty(key) &&
62536218
!((!value_arg && value_len)), ESP_ERR_INVALID_ARG, err, TAG, "Invalid state or arguments");
@@ -6256,10 +6221,9 @@ esp_err_t mdns_service_txt_item_set_for_host_with_explicit_value_len(const char
62566221
ESP_GOTO_ON_FALSE(s, ESP_ERR_NOT_FOUND, err, TAG, "Service doesn't exist");
62576222

62586223
mdns_service_t *srv = s->service;
6259-
char *value = NULL;
62606224
if (value_len > 0) {
62616225
value = (char *) malloc(value_len);
6262-
ESP_GOTO_ON_FALSE(s, ESP_ERR_NO_MEM, err, TAG, "Out of memory");
6226+
ESP_GOTO_ON_FALSE(s, ESP_ERR_NO_MEM, out_of_mem, TAG, "Out of memory");
62636227
memcpy(value, value_arg, value_len);
62646228
} else {
62656229
value_len = 0;
@@ -6276,9 +6240,9 @@ esp_err_t mdns_service_txt_item_set_for_host_with_explicit_value_len(const char
62766240
}
62776241
if (!txt) {
62786242
txt = (mdns_txt_linked_item_t *)malloc(sizeof(mdns_txt_linked_item_t));
6279-
ESP_GOTO_ON_FALSE(s, ESP_ERR_NO_MEM, err, TAG, "Out of memory");
6243+
ESP_GOTO_ON_FALSE(s, ESP_ERR_NO_MEM, out_of_mem, TAG, "Out of memory");
62806244
txt->key = strdup(key);
6281-
ESP_GOTO_ON_FALSE(txt->key, ESP_ERR_NO_MEM, err, TAG, "Out of memory");
6245+
ESP_GOTO_ON_FALSE(txt->key, ESP_ERR_NO_MEM, out_of_mem, TAG, "Out of memory");
62826246
txt->value = value;
62836247
txt->value_len = value_len;
62846248
txt->next = srv->txt;
@@ -6289,10 +6253,12 @@ esp_err_t mdns_service_txt_item_set_for_host_with_explicit_value_len(const char
62896253

62906254
err:
62916255
MDNS_SERVICE_UNLOCK();
6292-
if (ret == ESP_ERR_NO_MEM) {
6293-
HOOK_MALLOC_FAILED;
6294-
}
6295-
return ESP_OK;
6256+
return ret;
6257+
out_of_mem:
6258+
MDNS_SERVICE_UNLOCK();
6259+
HOOK_MALLOC_FAILED;
6260+
free(value);
6261+
return ret;
62966262
}
62976263

62986264
esp_err_t mdns_service_txt_item_set_for_host(const char *instance, const char *service, const char *proto, const char *hostname,
@@ -6321,46 +6287,59 @@ esp_err_t mdns_service_txt_item_set_with_explicit_value_len(const char *service,
63216287
return mdns_service_txt_item_set_for_host_with_explicit_value_len(NULL, service, proto, NULL, key, value, value_len);
63226288
}
63236289

6324-
esp_err_t mdns_service_txt_item_remove_for_host(const char *instance, const char *service, const char *proto, const char *hostname,
6290+
esp_err_t mdns_service_txt_item_remove_for_host(const char *instance, const char *service, const char *proto, const char *host,
63256291
const char *key)
63266292
{
63276293
MDNS_SERVICE_LOCK();
6328-
if (!_mdns_server || !_mdns_server->services || _str_null_or_empty(service) || _str_null_or_empty(proto) || _str_null_or_empty(key)) {
6329-
MDNS_SERVICE_UNLOCK();
6330-
return ESP_ERR_INVALID_ARG;
6331-
}
6294+
esp_err_t ret = ESP_OK;
6295+
const char *hostname = host ? host : _mdns_server->hostname;
6296+
ESP_GOTO_ON_FALSE(_mdns_server && _mdns_server->services && !_str_null_or_empty(service) && !_str_null_or_empty(proto) && !_str_null_or_empty(key),
6297+
ESP_ERR_INVALID_ARG, err, TAG, "Invalid state or arguments");
6298+
63326299
mdns_srv_item_t *s = _mdns_get_service_item_instance(instance, service, proto, hostname);
6333-
MDNS_SERVICE_UNLOCK();
6334-
if (!s) {
6335-
return ESP_ERR_NOT_FOUND;
6300+
ESP_GOTO_ON_FALSE(s, ESP_ERR_NOT_FOUND, err, TAG, "Service doesn't exist");
6301+
6302+
mdns_service_t *srv = s->service;
6303+
mdns_txt_linked_item_t *txt = srv->txt;
6304+
if (!txt) {
6305+
goto err;
63366306
}
6337-
mdns_action_t *action = (mdns_action_t *)malloc(sizeof(mdns_action_t));
6338-
if (!action) {
6339-
HOOK_MALLOC_FAILED;
6340-
return ESP_ERR_NO_MEM;
6307+
if (strcmp(txt->key, key) == 0) {
6308+
srv->txt = txt->next;
6309+
free((char *)txt->key);
6310+
free((char *)txt->value);
6311+
free(txt);
6312+
} else {
6313+
while (txt->next) {
6314+
if (strcmp(txt->next->key, key) == 0) {
6315+
mdns_txt_linked_item_t *t = txt->next;
6316+
txt->next = t->next;
6317+
free((char *)t->key);
6318+
free((char *)t->value);
6319+
free(t);
6320+
break;
6321+
} else {
6322+
txt = txt->next;
6323+
}
6324+
}
63416325
}
63426326

6343-
action->type = ACTION_SERVICE_TXT_DEL;
6344-
action->data.srv_txt_del.service = s;
6345-
action->data.srv_txt_del.key = strdup(key);
6346-
if (!action->data.srv_txt_del.key) {
6347-
free(action);
6348-
return ESP_ERR_NO_MEM;
6349-
}
6350-
if (xQueueSend(_mdns_server->action_queue, &action, (TickType_t)0) != pdPASS) {
6351-
free(action->data.srv_txt_del.key);
6352-
free(action);
6353-
return ESP_ERR_NO_MEM;
6327+
_mdns_announce_all_pcbs(&s, 1, false);
6328+
6329+
err:
6330+
MDNS_SERVICE_UNLOCK();
6331+
if (ret == ESP_ERR_NO_MEM) {
6332+
HOOK_MALLOC_FAILED;
63546333
}
6355-
return ESP_OK;
6334+
return ret;
63566335
}
63576336

63586337
esp_err_t mdns_service_txt_item_remove(const char *service, const char *proto, const char *key)
63596338
{
63606339
if (!_mdns_server) {
63616340
return ESP_ERR_INVALID_STATE;
63626341
}
6363-
return mdns_service_txt_item_remove_for_host(NULL, service, proto, _mdns_server->hostname, key);
6342+
return mdns_service_txt_item_remove_for_host(NULL, service, proto, NULL, key);
63646343
}
63656344

63666345
esp_err_t mdns_service_subtype_add_for_host(const char *instance_name, const char *service, const char *proto,

components/mdns/private_include/mdns_private.h

Lines changed: 0 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -188,7 +188,6 @@ typedef enum {
188188
ACTION_HOSTNAME_SET,
189189
ACTION_INSTANCE_SET,
190190
ACTION_SERVICE_INSTANCE_SET,
191-
ACTION_SERVICE_TXT_DEL,
192191
ACTION_SERVICE_SUBTYPE_ADD,
193192
ACTION_SERVICES_CLEAR,
194193
ACTION_SEARCH_ADD,
@@ -445,10 +444,6 @@ typedef struct {
445444
mdns_srv_item_t *service;
446445
char *instance;
447446
} srv_instance;
448-
struct {
449-
mdns_srv_item_t *service;
450-
char *key;
451-
} srv_txt_del;
452447
struct {
453448
mdns_srv_item_t *service;
454449
char *subtype;

0 commit comments

Comments
 (0)