Skip to content

Commit a1982c1

Browse files
author
Tero Heinonen
authored
Remove transaction from list before callback call (#50)
In some callbacks, secure session might be removed and transactions been freed. That leads to double freeing, and reading from freed pointer. Transaction is now removed from list before calling callback to avoid that.
1 parent ae4d9db commit a1982c1

File tree

1 file changed

+26
-9
lines changed

1 file changed

+26
-9
lines changed

source/coap_message_handler.c

Lines changed: 26 additions & 9 deletions
Original file line numberDiff line numberDiff line change
@@ -81,14 +81,24 @@ static coap_transaction_t *transaction_create(void)
8181

8282
return this;
8383
}
84+
85+
static void transaction_free(coap_transaction_t *this)
86+
{
87+
if (!this) {
88+
return;
89+
}
90+
91+
if (this->data_ptr) {
92+
ns_dyn_mem_free(this->data_ptr);
93+
}
94+
ns_dyn_mem_free(this);
95+
}
96+
8497
void transaction_delete(coap_transaction_t *this)
8598
{
8699
if (this) {
87100
ns_list_remove(&request_list, this);
88-
if(this->data_ptr){
89-
ns_dyn_mem_free(this->data_ptr);
90-
}
91-
ns_dyn_mem_free(this);
101+
transaction_free(this);
92102
}
93103

94104
return;
@@ -99,11 +109,12 @@ void transactions_delete_all(uint8_t *address_ptr, uint16_t port)
99109
coap_transaction_t *transaction = transaction_find_by_address(address_ptr, port);
100110

101111
while (transaction) {
112+
ns_list_remove(&request_list, transaction);
102113
if (transaction->resp_cb) {
103114
transaction->resp_cb(transaction->service_id, address_ptr, port, NULL);
104115
}
105116
sn_coap_protocol_delete_retransmission(coap_service_handle->coap, transaction->msg_id);
106-
transaction_delete(transaction);
117+
transaction_free(transaction);
107118
transaction = transaction_find_by_address(address_ptr, port);
108119
}
109120
}
@@ -113,17 +124,22 @@ static int8_t coap_rx_function(sn_coap_hdr_s *resp_ptr, sn_nsdl_addr_s *address_
113124
coap_transaction_t *this = NULL;
114125
(void)address_ptr;
115126
(void)param;
116-
tr_warn("transaction was not handled");
127+
tr_warn("transaction was not handled %d", resp_ptr->msg_id);
117128
if (!resp_ptr) {
118129
return -1;
119130
}
120131
if( resp_ptr->token_ptr ){
121132
this = transaction_find_client_by_token(resp_ptr->token_ptr);
122133
}
123-
if (this && this->resp_cb) {
134+
if (!this) {
135+
return 0;
136+
}
137+
138+
ns_list_remove(&request_list, this);
139+
if (this->resp_cb) {
124140
this->resp_cb(this->service_id, address_ptr->addr_ptr, address_ptr->port, NULL);
125141
}
126-
transaction_delete(this);
142+
transaction_free(this);
127143
return 0;
128144
}
129145

@@ -250,11 +266,12 @@ int16_t coap_message_handler_coap_msg_process(coap_msg_handler_t *handle, int8_t
250266
return -1;
251267
}
252268
tr_debug("Service %d, response received", this->service_id);
269+
ns_list_remove(&request_list, this);
253270
if (this->resp_cb) {
254271
this->resp_cb(this->service_id, (uint8_t *)source_addr_ptr, port, coap_message);
255272
}
256273
sn_coap_parser_release_allocated_coap_msg_mem(handle->coap, coap_message);
257-
transaction_delete(this);
274+
transaction_free(this);
258275
}
259276

260277
return 0;

0 commit comments

Comments
 (0)