From da0b8a84b4b2f1bd0275c8af482575ccf827ee50 Mon Sep 17 00:00:00 2001 From: Aleksandr Korolev Date: Tue, 16 Nov 2021 11:52:49 +0300 Subject: [PATCH] [IE][VPU] Fix memory leak (#8554) [VPU] heap-use-after-free fix --- .../thirdparty/movidius/mvnc/src/mvnc_api.c | 78 ++++++++----------- 1 file changed, 32 insertions(+), 46 deletions(-) diff --git a/inference-engine/thirdparty/movidius/mvnc/src/mvnc_api.c b/inference-engine/thirdparty/movidius/mvnc/src/mvnc_api.c index de5d39bb7c8..d91cd98f8a0 100644 --- a/inference-engine/thirdparty/movidius/mvnc/src/mvnc_api.c +++ b/inference-engine/thirdparty/movidius/mvnc/src/mvnc_api.c @@ -103,7 +103,11 @@ static int global_lock_fd = -1; #define STRINGIFY(_text) #_text #define CASE(entry) case entry: return STRINGIFY(entry); - +#define FREE_AND_NULL(fDynMem, fPointer, sDynMem, sPointer) \ + free(fDynMem); \ + fPointer = NULL; \ + free(sDynMem); \ + sPointer = NULL // To suppress warning in the macro below #if defined __GNUC__ || defined __clang__ @@ -2199,10 +2203,7 @@ ncStatus_t ncGraphDestroy(struct ncGraphHandle_t ** graphHandle) CHECK_HANDLE_CORRECT_WINFO(g, MVLOG_ERROR, "Graph handle is corrupt or has been destroyed"); if (g->state == NC_GRAPH_CREATED || g->state == NC_GRAPH_DEALLOCATED) { - free(g); - gh->private_data = NULL; - free(gh); - *graphHandle = NULL; + FREE_AND_NULL(g, gh->private_data, gh, *graphHandle); return NC_OK; } GLOBAL_LOCK(); @@ -2220,28 +2221,24 @@ ncStatus_t ncGraphDestroy(struct ncGraphHandle_t ** graphHandle) cmd.id = g->id; CHECK_MUTEX_SUCCESS(pthread_mutex_lock(&d->graph_stream_m)); rc = trySendCommand(d->graph_monitor_stream_id, &cmd, sizeof(cmd)); - if(rc != 0){ - CHECK_MUTEX_SUCCESS(pthread_mutex_unlock(&d->graph_stream_m)); - return rc; - } - if (checkGraphMonitorResponse(d->graph_monitor_stream_id)) { - CHECK_MUTEX_SUCCESS(pthread_mutex_unlock(&d->graph_stream_m)); - return NC_ERROR; + if (rc != 0){ + mvLog(MVLOG_WARN, "can't send command\n"); + rc = NC_ERROR; + } else if (checkGraphMonitorResponse(d->graph_monitor_stream_id)) { + mvLog(MVLOG_WARN, "myriad NACK\n"); + rc = NC_ERROR; } XLinkCloseStream(g->graph_stream_id); CHECK_MUTEX_SUCCESS(pthread_mutex_unlock(&d->graph_stream_m)); CHECK_MUTEX_SUCCESS(pthread_mutex_lock(&d->dev_data_m)); if (deallocateGraph(gh->private_data)) { mvLog(MVLOG_ERROR, "This graph has already been destroyed"); - CHECK_MUTEX_SUCCESS(pthread_mutex_unlock(&d->dev_data_m)); - return NC_INVALID_PARAMETERS; + rc = NC_INVALID_PARAMETERS; } CHECK_MUTEX_SUCCESS(pthread_mutex_unlock(&d->dev_data_m)); - free(g); - gh->private_data = NULL; - free(gh); - *graphHandle = NULL; - return NC_OK; + FREE_AND_NULL(g, gh->private_data, gh, *graphHandle); + + return rc; } ncStatus_t ncGraphSetOption(struct ncGraphHandle_t * graphHandle, @@ -3021,10 +3018,6 @@ ncStatus_t ncFifoAllocate(struct ncFifoHandle_t * fifoHandle, handle->datasize = handle->host_tensor_desc.totalSize; - if (d->fifos) - handle->next = d->fifos; - d->fifos = handle; - bufferAllocateCommand_t cmd; cmd.type = GRAPH_BUFFER_ALLOCATE_CMD; struct tensorDescriptor_t privateDesc; @@ -3077,6 +3070,10 @@ ncStatus_t ncFifoAllocate(struct ncFifoHandle_t * fifoHandle, mvLog(MVLOG_ERROR, "myriad NACK\n"); return NC_ERROR; } + if (d->fifos) + handle->next = d->fifos; + d->fifos = handle; + CHECK_MUTEX_SUCCESS(pthread_mutex_unlock(&d->graph_stream_m)); handle->state = NC_FIFO_ALLOCATED; @@ -3094,7 +3091,7 @@ ncStatus_t ncFifoDestroy(struct ncFifoHandle_t ** fifoHandle) } struct _fifoPrivate_t *handle = fh->private_data; - + ncStatus_t rc = NC_OK; if (handle->state == NC_FIFO_CREATED || handle->state == NC_FIFO_DEALLOCATED) { pthread_mutex_t * fifo_mutex = &fh->private_data->fifo_mutex; #if !(defined(_WIN32) || defined(_WIN64)) @@ -3116,18 +3113,14 @@ ncStatus_t ncFifoDestroy(struct ncFifoHandle_t ** fifoHandle) CHECK_MUTEX_SUCCESS(pthread_mutex_unlock(fifo_mutex)); CHECK_MUTEX_SUCCESS(pthread_mutex_destroy(fifo_mutex)); - free(fh->private_data); - fh->private_data = NULL; - - free(fh); - *fifoHandle = NULL; + FREE_AND_NULL(fh->private_data, fh->private_data, fh, *fifoHandle); return NC_OK; } if (!findFifo(handle)) { mvLog(MVLOG_ERROR, "fifo handle seems to be corrupt or has been destroyed"); - return NC_INVALID_HANDLE; + rc = NC_INVALID_HANDLE; } //clean up fifo /*if (fifoReadAccess(handle)) { @@ -3146,11 +3139,10 @@ ncStatus_t ncFifoDestroy(struct ncFifoHandle_t ** fifoHandle) if (XLinkWriteData(handle->streamId, (uint8_t *) & msg, sizeof(msg)) != 0) { mvLog(MVLOG_ERROR, "Failed to write to fifo before deleting it!"); - return NC_ERROR; + rc = NC_ERROR; } } - ncStatus_t rc = NC_OK; graphCommonCommand_t cmd; cmd.type = GRAPH_BUFFER_DEALLOCATE_CMD; cmd.id = handle->id; @@ -3158,30 +3150,24 @@ ncStatus_t ncFifoDestroy(struct ncFifoHandle_t ** fifoHandle) struct _devicePrivate_t *d = handle->dev; CHECK_MUTEX_SUCCESS(pthread_mutex_lock(&d->graph_stream_m)); rc = trySendCommand(d->graph_monitor_stream_id, &cmd, sizeof(cmd)); - if(rc != 0){ - CHECK_MUTEX_SUCCESS(pthread_mutex_unlock(&d->graph_stream_m)); + if (rc != 0) { mvLog(MVLOG_WARN, "can't send command\n"); - return rc; - } - if (checkGraphMonitorResponse(d->graph_monitor_stream_id)) { - CHECK_MUTEX_SUCCESS(pthread_mutex_unlock(&d->graph_stream_m)); + rc = NC_ERROR; + } else if (checkGraphMonitorResponse(d->graph_monitor_stream_id)) { mvLog(MVLOG_WARN, "myriad NACK\n"); - return NC_ERROR; + rc = NC_ERROR; } CHECK_MUTEX_SUCCESS(pthread_mutex_unlock(&d->graph_stream_m)); CHECK_MUTEX_SUCCESS(pthread_mutex_lock(&d->dev_data_m)); if (deallocateFifo(handle)) { - CHECK_MUTEX_SUCCESS(pthread_mutex_unlock(&d->dev_data_m)); - return NC_INVALID_PARAMETERS; + mvLog(MVLOG_WARN, "failed deallocateFifo\n"); + rc = NC_INVALID_PARAMETERS; } CHECK_MUTEX_SUCCESS(pthread_mutex_unlock(&d->dev_data_m)); - free(fh->private_data); - fh->private_data = NULL; - free(fh); - *fifoHandle = NULL; - return NC_OK; + FREE_AND_NULL(fh->private_data, fh->private_data, fh, *fifoHandle); + return rc; }