Skip to content

Commit d58c815

Browse files
committed
Fix issues reported by Coverity
Fix issue 525174: Read from pointer after free in oc_cloud_deregister.c Fix issue 525173: Read from pointer after free in oc_event_callback.c Fix issue 525172: Read from pointer after free in oc_oscore_engine.c
1 parent c3794a6 commit d58c815

File tree

7 files changed

+253
-228
lines changed

7 files changed

+253
-228
lines changed

api/cloud/oc_cloud_deregister.c

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -229,8 +229,8 @@ cloud_deregister_refreshed_token_async(void *data)
229229
if (oc_cloud_check_accesstoken_for_deregister(p->ctx)) {
230230
if (cloud_deregister_by_request(p, p->timeout, true) != 0) {
231231
OC_CLOUD_ERR("Failed to deregister from cloud");
232-
oc_cloud_api_free_param(p);
233232
cloud_context_clear(p->ctx);
233+
oc_cloud_api_free_param(p);
234234
}
235235
return OC_EVENT_DONE;
236236
}
@@ -239,8 +239,8 @@ cloud_deregister_refreshed_token_async(void *data)
239239
if (oc_cloud_do_login(p->ctx, cloud_deregister_try_logged_in, p,
240240
p->timeout) != 0) {
241241
OC_CLOUD_ERR("Failed to login to cloud for deregister");
242-
oc_cloud_api_free_param(p);
243242
cloud_context_clear(p->ctx);
243+
oc_cloud_api_free_param(p);
244244
return OC_EVENT_DONE;
245245
}
246246
return OC_EVENT_DONE;

api/oc_event_callback.c

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -198,13 +198,13 @@ event_callbacks_poll_timers(oc_list_t list, oc_memb_t *cb_pool)
198198
{
199199
oc_event_callback_t *event_cb = (oc_event_callback_t *)oc_list_head(list);
200200
while (event_cb != NULL) {
201-
oc_event_callback_t *next = event_cb->next;
202201
if (!oc_etimer_expired(&event_cb->timer)) {
203-
event_cb = next;
202+
event_cb = event_cb->next;
204203
continue;
205204
}
206205
g_currently_processed_event_cb = event_cb;
207206
g_currently_processed_event_cb_delete = false;
207+
g_currently_processed_event_on_delete = NULL;
208208
if ((event_cb->callback(event_cb->data) == OC_EVENT_DONE) ||
209209
g_currently_processed_event_cb_delete) {
210210
oc_list_remove(list, event_cb);

api/oc_message.c

Lines changed: 28 additions & 10 deletions
Original file line numberDiff line numberDiff line change
@@ -192,11 +192,29 @@ oc_message_add_ref(oc_message_t *message)
192192
}
193193
}
194194

195+
uint8_t
196+
oc_message_refcount(const oc_message_t *message)
197+
{
198+
return OC_ATOMIC_LOAD8(message->ref_count);
199+
}
200+
195201
void
196-
oc_message_unref(oc_message_t *message)
202+
oc_message_deallocate(oc_message_t *message)
203+
{
204+
oc_memb_t *pool = message->pool;
205+
message_deallocate(message, pool);
206+
OC_TRACE("buffer: deallocated message(%p) from pool(%p)", (void *)message,
207+
(void *)pool);
208+
#ifdef OC_HAS_FEATURE_ALLOCATOR_MUTEX
209+
OC_TRACE("buffer: freed TX/RX buffer; num free: %d", oc_memb_numfree(pool));
210+
#endif /* OC_HAS_FEATURE_ALLOCATOR_MUTEX*/
211+
}
212+
213+
bool
214+
oc_message_unref2(oc_message_t *message)
197215
{
198216
if (message == NULL) {
199-
return;
217+
return false;
200218
}
201219
bool dealloc = false;
202220
uint8_t count = OC_ATOMIC_LOAD8(message->ref_count);
@@ -214,16 +232,16 @@ oc_message_unref(oc_message_t *message)
214232
if (!dealloc) {
215233
OC_TRACE("buffer: message(%p) unreferenced, ref_count=%d", (void *)message,
216234
(int)new_count);
217-
return;
235+
return false;
218236
}
237+
oc_message_deallocate(message);
238+
return true;
239+
}
219240

220-
oc_memb_t *pool = message->pool;
221-
message_deallocate(message, pool);
222-
OC_TRACE("buffer: deallocated message(%p) from pool(%p)", (void *)message,
223-
(void *)pool);
224-
#ifdef OC_HAS_FEATURE_ALLOCATOR_MUTEX
225-
OC_TRACE("buffer: freed TX/RX buffer; num free: %d", oc_memb_numfree(pool));
226-
#endif /* OC_HAS_FEATURE_ALLOCATOR_MUTEX*/
241+
void
242+
oc_message_unref(oc_message_t *message)
243+
{
244+
oc_message_unref2(message);
227245
}
228246

229247
#ifdef OC_HAS_FEATURE_MESSAGE_DYNAMIC_BUFFER

api/oc_message_internal.h

Lines changed: 11 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -22,6 +22,7 @@
2222
#include "port/oc_connectivity.h"
2323
#include "util/oc_features.h"
2424

25+
#include <stdbool.h>
2526
#include <stddef.h>
2627

2728
#ifdef __cplusplus
@@ -68,6 +69,16 @@ oc_message_t *oc_message_allocate_outgoing_with_size(size_t size);
6869
*/
6970
size_t oc_message_max_buffer_size(void);
7071

72+
/** @brief Get the current reference count */
73+
uint8_t oc_message_refcount(const oc_message_t *message) OC_NONNULL();
74+
75+
/** @brief Decrease reference count and return true if message was
76+
* deallocated.*/
77+
bool oc_message_unref2(oc_message_t *message);
78+
79+
/** @brief Deallocate the message */
80+
void oc_message_deallocate(oc_message_t *message) OC_NONNULL();
81+
7182
/**
7283
* @brief Get size of the message buffer
7384
*

messaging/coap/engine.c

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -158,8 +158,8 @@ coap_send_empty_response(coap_message_type_t type, uint16_t mid,
158158

159159
message->length = len;
160160
coap_send_message(message);
161-
if (message->ref_count == 0) {
162-
oc_message_unref(message);
161+
if (oc_message_refcount(message) == 0) {
162+
oc_message_deallocate(message);
163163
}
164164
}
165165

messaging/coap/separate.c

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -154,8 +154,8 @@ coap_separate_accept(const coap_packet_t *request,
154154
memcpy(&message->endpoint, endpoint, sizeof(oc_endpoint_t));
155155
coap_send_message(message);
156156

157-
if (message->ref_count == 0) {
158-
oc_message_unref(message);
157+
if (oc_message_refcount(message) == 0) {
158+
oc_message_deallocate(message);
159159
}
160160
}
161161

0 commit comments

Comments
 (0)