oic; use os_mempool for coap observers. Fix use-after-free error.
Project: http://git-wip-us.apache.org/repos/asf/incubator-mynewt-core/repo Commit: http://git-wip-us.apache.org/repos/asf/incubator-mynewt-core/commit/408077eb Tree: http://git-wip-us.apache.org/repos/asf/incubator-mynewt-core/tree/408077eb Diff: http://git-wip-us.apache.org/repos/asf/incubator-mynewt-core/diff/408077eb Branch: refs/heads/develop Commit: 408077eb74a8cd0a9cf5cc376e432cb4de6a2b66 Parents: f861cf8 Author: Marko Kiiskila <ma...@runtime.io> Authored: Mon Nov 21 15:12:46 2016 -0800 Committer: Marko Kiiskila <ma...@runtime.io> Committed: Mon Nov 21 17:15:49 2016 -0800 ---------------------------------------------------------------------- net/oic/src/messaging/coap/engine.c | 1 + net/oic/src/messaging/coap/observe.c | 401 ++++++++++++++++-------------- net/oic/src/messaging/coap/observe.h | 2 + 3 files changed, 212 insertions(+), 192 deletions(-) ---------------------------------------------------------------------- http://git-wip-us.apache.org/repos/asf/incubator-mynewt-core/blob/408077eb/net/oic/src/messaging/coap/engine.c ---------------------------------------------------------------------- diff --git a/net/oic/src/messaging/coap/engine.c b/net/oic/src/messaging/coap/engine.c index a17ebd6..1145447 100644 --- a/net/oic/src/messaging/coap/engine.c +++ b/net/oic/src/messaging/coap/engine.c @@ -306,5 +306,6 @@ coap_engine_init(void) coap_transaction_init(); #ifdef OC_SERVER coap_separate_init(); + coap_observe_init(); #endif } http://git-wip-us.apache.org/repos/asf/incubator-mynewt-core/blob/408077eb/net/oic/src/messaging/coap/observe.c ---------------------------------------------------------------------- diff --git a/net/oic/src/messaging/coap/observe.c b/net/oic/src/messaging/coap/observe.c index 4bd1d96..c16823a 100644 --- a/net/oic/src/messaging/coap/observe.c +++ b/net/oic/src/messaging/coap/observe.c @@ -35,11 +35,14 @@ #ifdef OC_SERVER -#include "observe.h" -#include "util/oc_memb.h" #include <stdio.h> #include <string.h> +#include <os/os_mempool.h> + +#include "observe.h" +#include "util/oc_memb.h" + #include "oc_coap.h" #include "oc_rep.h" #include "oc_ri.h" @@ -47,7 +50,10 @@ uint64_t observe_counter = 3; /*---------------------------------------------------------------------------*/ OC_LIST(observers_list); -OC_MEMB(observers_memb, coap_observer_t, COAP_MAX_OBSERVERS); + +static struct os_mempool coap_observers; +static uint8_t coap_observer_area[OS_MEMPOOL_BYTES(COAP_MAX_OBSERVERS, + sizeof(coap_observer_t))]; /*---------------------------------------------------------------------------*/ /*- Internal API ------------------------------------------------------------*/ @@ -57,32 +63,32 @@ add_observer(oc_resource_t *resource, oc_endpoint_t *endpoint, const uint8_t *token, size_t token_len, const char *uri, int uri_len) { - /* Remove existing observe relationship, if any. */ - int dup = coap_remove_observer_by_uri(endpoint, uri); + /* Remove existing observe relationship, if any. */ + int dup = coap_remove_observer_by_uri(endpoint, uri); - coap_observer_t *o = oc_memb_alloc(&observers_memb); + coap_observer_t *o = os_memblock_get(&coap_observers); - if (o) { - int max = sizeof(o->url) - 1; - if (max > uri_len) { - max = uri_len; + if (o) { + int max = sizeof(o->url) - 1; + if (max > uri_len) { + max = uri_len; + } + memcpy(o->url, uri, max); + o->url[max] = 0; + memcpy(&o->endpoint, endpoint, sizeof(oc_endpoint_t)); + o->token_len = token_len; + memcpy(o->token, token, token_len); + o->last_mid = 0; + o->obs_counter = observe_counter; + o->resource = resource; + resource->num_observers++; + LOG("Adding observer (%u/%u) for /%s [0x%02X%02X]\n", + oc_list_length(observers_list) + 1, COAP_MAX_OBSERVERS, o->url, + o->token[0], o->token[1]); + oc_list_add(observers_list, o); + return dup; } - memcpy(o->url, uri, max); - o->url[max] = 0; - memcpy(&o->endpoint, endpoint, sizeof(oc_endpoint_t)); - o->token_len = token_len; - memcpy(o->token, token, token_len); - o->last_mid = 0; - o->obs_counter = observe_counter; - o->resource = resource; - resource->num_observers++; - LOG("Adding observer (%u/%u) for /%s [0x%02X%02X]\n", - oc_list_length(observers_list) + 1, COAP_MAX_OBSERVERS, o->url, - o->token[0], o->token[1]); - oc_list_add(observers_list, o); - return dup; - } - return -1; + return -1; } /*---------------------------------------------------------------------------*/ /*- Removal -----------------------------------------------------------------*/ @@ -90,97 +96,99 @@ add_observer(oc_resource_t *resource, oc_endpoint_t *endpoint, void coap_remove_observer(coap_observer_t *o) { - LOG("Removing observer for /%s [0x%02X%02X]\n", o->url, o->token[0], + LOG("Removing observer for /%s [0x%02X%02X]\n", o->url, o->token[0], o->token[1]); - oc_memb_free(&observers_memb, o); - oc_list_remove(observers_list, o); + oc_list_remove(observers_list, o); + os_memblock_put(&coap_observers, o); } /*---------------------------------------------------------------------------*/ int coap_remove_observer_by_client(oc_endpoint_t *endpoint) { - int removed = 0; - coap_observer_t *obs = (coap_observer_t *)oc_list_head(observers_list), *next; + int removed = 0; + coap_observer_t *obs = (coap_observer_t *)oc_list_head(observers_list), + *next; - LOG("Unregistering observers for client at: "); - LOGipaddr(*endpoint); + LOG("Unregistering observers for client at: "); + LOGipaddr(*endpoint); - while (obs) { - next = obs->next; - if (memcmp(&obs->endpoint, endpoint, sizeof(oc_endpoint_t)) == 0) { - obs->resource->num_observers--; - coap_remove_observer(obs); - removed++; + while (obs) { + next = obs->next; + if (memcmp(&obs->endpoint, endpoint, sizeof(oc_endpoint_t)) == 0) { + obs->resource->num_observers--; + coap_remove_observer(obs); + removed++; + } + obs = next; } - obs = next; - } - LOG("Removed %d observers\n", removed); - return removed; + LOG("Removed %d observers\n", removed); + return removed; } /*---------------------------------------------------------------------------*/ int coap_remove_observer_by_token(oc_endpoint_t *endpoint, uint8_t *token, size_t token_len) { - int removed = 0; - coap_observer_t *obs = (coap_observer_t *)oc_list_head(observers_list); - LOG("Unregistering observers for request token 0x%02X%02X\n", token[0], + int removed = 0; + coap_observer_t *obs = (coap_observer_t *)oc_list_head(observers_list); + LOG("Unregistering observers for request token 0x%02X%02X\n", token[0], token[1]); - while (obs) { - if (memcmp(&obs->endpoint, endpoint, sizeof(oc_endpoint_t)) == 0 && - obs->token_len == token_len && - memcmp(obs->token, token, token_len) == 0) { - obs->resource->num_observers--; - coap_remove_observer(obs); - removed++; - break; + while (obs) { + if (memcmp(&obs->endpoint, endpoint, sizeof(oc_endpoint_t)) == 0 && + obs->token_len == token_len && + memcmp(obs->token, token, token_len) == 0) { + obs->resource->num_observers--; + coap_remove_observer(obs); + removed++; + break; + } + obs = obs->next; } - obs = obs->next; - } - LOG("Removed %d observers\n", removed); - return removed; + LOG("Removed %d observers\n", removed); + return removed; } /*---------------------------------------------------------------------------*/ int coap_remove_observer_by_uri(oc_endpoint_t *endpoint, const char *uri) { - LOG("Unregistering observers for resource uri /%s", uri); - int removed = 0; - coap_observer_t *obs = (coap_observer_t *)oc_list_head(observers_list), *next; + LOG("Unregistering observers for resource uri /%s", uri); + int removed = 0; + coap_observer_t *obs = (coap_observer_t *)oc_list_head(observers_list), + *next; - while (obs) { - next = obs->next; - if (((memcmp(&obs->endpoint, endpoint, sizeof(oc_endpoint_t)) == 0)) && - (obs->url == uri || memcmp(obs->url, uri, strlen(obs->url)) == 0)) { - obs->resource->num_observers--; - coap_remove_observer(obs); - removed++; + while (obs) { + next = obs->next; + if (((memcmp(&obs->endpoint, endpoint, sizeof(oc_endpoint_t)) == 0)) && + (obs->url == uri || memcmp(obs->url, uri, strlen(obs->url)) == 0)) { + obs->resource->num_observers--; + coap_remove_observer(obs); + removed++; + } + obs = next; } - obs = next; - } - LOG("Removed %d observers\n", removed); - return removed; + LOG("Removed %d observers\n", removed); + return removed; } /*---------------------------------------------------------------------------*/ int coap_remove_observer_by_mid(oc_endpoint_t *endpoint, uint16_t mid) { - int removed = 0; - coap_observer_t *obs = NULL; - LOG("Unregistering observers for request MID %u\n", mid); + int removed = 0; + coap_observer_t *obs = NULL; + LOG("Unregistering observers for request MID %u\n", mid); - for (obs = (coap_observer_t *)oc_list_head(observers_list); obs != NULL; - obs = obs->next) { - if (memcmp(&obs->endpoint, endpoint, sizeof(*endpoint)) == 0 && - obs->last_mid == mid) { - obs->resource->num_observers--; - coap_remove_observer(obs); - removed++; - break; + for (obs = (coap_observer_t *)oc_list_head(observers_list); obs != NULL; + obs = obs->next) { + if (memcmp(&obs->endpoint, endpoint, sizeof(*endpoint)) == 0 && + obs->last_mid == mid) { + obs->resource->num_observers--; + coap_remove_observer(obs); + removed++; + break; + } } - } - LOG("Removed %d observers\n", removed); - return removed; + LOG("Removed %d observers\n", removed); + return removed; } /*---------------------------------------------------------------------------*/ /*- Notification ------------------------------------------------------------*/ @@ -190,130 +198,139 @@ coap_notify_observers(oc_resource_t *resource, oc_response_buffer_t *response_buf, oc_endpoint_t *endpoint) { - int num_observers = 0; - if (resource) { - if (!resource->num_observers) { - LOG("coap_notify_observers: no observers; returning\n"); - return 0; + int num_observers = 0; + if (resource) { + if (!resource->num_observers) { + LOG("coap_notify_observers: no observers; returning\n"); + return 0; + } + num_observers = resource->num_observers; } - num_observers = resource->num_observers; - } - uint8_t buffer[COAP_MAX_BLOCK_SIZE]; - oc_request_t request = {}; - oc_response_t response = {}; - response.separate_response = 0; - oc_response_buffer_t response_buffer; - if (!response_buf && resource && (resource->properties & OC_PERIODIC)) { - LOG("coap_notify_observers: Issue GET request to resource\n"); - /* performing GET on the resource */ - response_buffer.buffer = buffer; - response_buffer.buffer_size = COAP_MAX_BLOCK_SIZE; - response_buffer.block_offset = NULL; - response.response_buffer = &response_buffer; - request.resource = resource; - request.response = &response; - request.request_payload = NULL; - oc_rep_new(buffer, COAP_MAX_BLOCK_SIZE); - resource->get_handler(&request, resource->default_interface); - response_buf = &response_buffer; - if (response_buf->code == OC_IGNORE) { - LOG("coap_notify_observers: Resource ignored request\n"); - return num_observers; + uint8_t buffer[COAP_MAX_BLOCK_SIZE]; + oc_request_t request = {}; + oc_response_t response = {}; + response.separate_response = 0; + oc_response_buffer_t response_buffer; + if (!response_buf && resource && (resource->properties & OC_PERIODIC)) { + LOG("coap_notify_observers: Issue GET request to resource\n"); + /* performing GET on the resource */ + response_buffer.buffer = buffer; + response_buffer.buffer_size = COAP_MAX_BLOCK_SIZE; + response_buffer.block_offset = NULL; + response.response_buffer = &response_buffer; + request.resource = resource; + request.response = &response; + request.request_payload = NULL; + oc_rep_new(buffer, COAP_MAX_BLOCK_SIZE); + resource->get_handler(&request, resource->default_interface); + response_buf = &response_buffer; + if (response_buf->code == OC_IGNORE) { + LOG("coap_notify_observers: Resource ignored request\n"); + return num_observers; + } } - } - coap_observer_t *obs = NULL; - /* iterate over observers */ - for (obs = (coap_observer_t *)oc_list_head(observers_list); - obs && ((resource && obs->resource == resource) || - (endpoint && - memcmp(&obs->endpoint, endpoint, sizeof(oc_endpoint_t)) == 0)); - obs = obs->next) { - num_observers = obs->resource->num_observers; - if (response.separate_response != NULL && - response_buf->code == oc_status_code(OC_STATUS_OK)) { - coap_packet_t req[1]; - /* - req->block1_num = 0; - req->block1_size = 0; - req->block2_num = 0; - req->block2_size = 0; - */ - coap_init_message(req, COAP_TYPE_NON, CONTENT_2_05, 0); - memcpy(req->token, obs->token, obs->token_len); - req->token_len = obs->token_len; - LOG("Resource is SLOW; creating separate response\n"); - if (coap_separate_accept(req, response.separate_response, &obs->endpoint, - 0) == 1) - response.separate_response->active = 1; - } else { - LOG("coap_notify_observers: notifying observer\n"); - coap_transaction_t *transaction = NULL; - if (response_buf && (transaction = coap_new_transaction( - coap_get_mid(), &obs->endpoint))) { - memcpy(transaction->message->data + COAP_MAX_HEADER_SIZE, - response_buf->buffer, response_buf->response_length); + coap_observer_t *obs = NULL; + /* iterate over observers */ + for (obs = (coap_observer_t *)oc_list_head(observers_list); + obs && ((resource && obs->resource == resource) || + (endpoint && + memcmp(&obs->endpoint, endpoint, sizeof(oc_endpoint_t)) == 0)); + obs = obs->next) { + num_observers = obs->resource->num_observers; + if (response.separate_response != NULL && + response_buf->code == oc_status_code(OC_STATUS_OK)) { + coap_packet_t req[1]; + /* + req->block1_num = 0; + req->block1_size = 0; + req->block2_num = 0; + req->block2_size = 0; + */ + coap_init_message(req, COAP_TYPE_NON, CONTENT_2_05, 0); + memcpy(req->token, obs->token, obs->token_len); + req->token_len = obs->token_len; + LOG("Resource is SLOW; creating separate response\n"); + if (coap_separate_accept(req, response.separate_response, + &obs->endpoint, 0) == 1) { + response.separate_response->active = 1; + } + } else { + LOG("coap_notify_observers: notifying observer\n"); + coap_transaction_t *transaction = NULL; + if (response_buf && (transaction = coap_new_transaction( + coap_get_mid(), &obs->endpoint))) { + memcpy(transaction->message->data + COAP_MAX_HEADER_SIZE, + response_buf->buffer, response_buf->response_length); - /* update last MID for RST matching */ - obs->last_mid = transaction->mid; + /* update last MID for RST matching */ + obs->last_mid = transaction->mid; - /* prepare response */ - /* build notification */ - coap_packet_t notification - [1]; /* this way the packet can be treated as pointer as usual */ - coap_init_message(notification, COAP_TYPE_NON, CONTENT_2_05, 0); + /* prepare response */ + /* build notification */ + coap_packet_t notification[1]; + /* this way the packet can be treated as pointer as usual */ + coap_init_message(notification, COAP_TYPE_NON, CONTENT_2_05, 0); - notification->mid = transaction->mid; - if (obs->obs_counter % COAP_OBSERVE_REFRESH_INTERVAL == 0) { - LOG("coap_observe_notify: forcing CON notification to check for " - "client liveness\n"); - notification->type = COAP_TYPE_CON; - } - coap_set_payload(notification, response_buf->buffer, - response_buf->response_length); - coap_set_status_code(notification, response_buf->code); - if (notification->code < BAD_REQUEST_4_00 && - obs->resource->num_observers) { - coap_set_header_observe(notification, (obs->obs_counter)++); - observe_counter++; - } else { - coap_set_header_observe(notification, 1); - } - coap_set_token(notification, obs->token, obs->token_len); + notification->mid = transaction->mid; + if (obs->obs_counter % COAP_OBSERVE_REFRESH_INTERVAL == 0) { + LOG("coap_observe_notify: forcing CON notification to " + "check for client liveness\n"); + notification->type = COAP_TYPE_CON; + } + coap_set_payload(notification, response_buf->buffer, + response_buf->response_length); + coap_set_status_code(notification, response_buf->code); + if (notification->code < BAD_REQUEST_4_00 && + obs->resource->num_observers) { + coap_set_header_observe(notification, (obs->obs_counter)++); + observe_counter++; + } else { + coap_set_header_observe(notification, 1); + } + coap_set_token(notification, obs->token, obs->token_len); - transaction->message->length = - coap_serialize_message(notification, transaction->message->data); + transaction->message->length = + coap_serialize_message(notification, + transaction->message->data); - coap_send_transaction(transaction); - } + coap_send_transaction(transaction); + } + } } - } - return num_observers; + return num_observers; } /*---------------------------------------------------------------------------*/ int coap_observe_handler(void *request, void *response, oc_resource_t *resource, oc_endpoint_t *endpoint) { - coap_packet_t *const coap_req = (coap_packet_t *)request; - coap_packet_t *const coap_res = (coap_packet_t *)response; - int dup = -1; - if (coap_req->code == COAP_GET && + coap_packet_t *const coap_req = (coap_packet_t *)request; + coap_packet_t *const coap_res = (coap_packet_t *)response; + int dup = -1; + if (coap_req->code == COAP_GET && coap_res->code < 128) { /* GET request and response without error code */ - if (IS_OPTION(coap_req, COAP_OPTION_OBSERVE)) { - if (coap_req->observe == 0) { - dup = - add_observer(resource, endpoint, coap_req->token, coap_req->token_len, - coap_req->uri_path, coap_req->uri_path_len); - } else if (coap_req->observe == 1) { - /* remove client if it is currently observe */ - dup = coap_remove_observer_by_token(endpoint, coap_req->token, - coap_req->token_len); - } + if (IS_OPTION(coap_req, COAP_OPTION_OBSERVE)) { + if (coap_req->observe == 0) { + dup = + add_observer(resource, endpoint, coap_req->token, + coap_req->token_len, coap_req->uri_path, + coap_req->uri_path_len); + } else if (coap_req->observe == 1) { + /* remove client if it is currently observe */ + dup = coap_remove_observer_by_token(endpoint, coap_req->token, + coap_req->token_len); + } + } } - } - return dup; + return dup; } /*---------------------------------------------------------------------------*/ +void +coap_observe_init(void) +{ + os_mempool_init(&coap_observers, COAP_MAX_OBSERVERS, + sizeof(coap_observer_t), coap_observer_area, "coap_obs"); +} #endif /* OC_SERVER */ http://git-wip-us.apache.org/repos/asf/incubator-mynewt-core/blob/408077eb/net/oic/src/messaging/coap/observe.h ---------------------------------------------------------------------- diff --git a/net/oic/src/messaging/coap/observe.h b/net/oic/src/messaging/coap/observe.h index b1eadc2..095f21e 100644 --- a/net/oic/src/messaging/coap/observe.h +++ b/net/oic/src/messaging/coap/observe.h @@ -80,6 +80,8 @@ int coap_notify_observers(oc_resource_t *resource, int coap_observe_handler(void *request, void *response, oc_resource_t *resource, oc_endpoint_t *endpoint); +void coap_observe_init(void); + #ifdef __cplusplus } #endif