From 7c06680cba3fd6c046b9e6f8aec184b5bc8fef6e Mon Sep 17 00:00:00 2001 From: Michael Zillgith Date: Mon, 22 Aug 2022 11:29:23 +0200 Subject: [PATCH] - MMS server: fixed - possible deadlock in obtainFile-service/file upload task (LIB61850-351) --- src/mms/inc_private/mms_server_internal.h | 2 +- src/mms/iso_mms/server/mms_file_service.c | 274 +++++++++++----------- src/mms/iso_mms/server/mms_server.c | 14 +- 3 files changed, 141 insertions(+), 149 deletions(-) diff --git a/src/mms/inc_private/mms_server_internal.h b/src/mms/inc_private/mms_server_internal.h index 5e2f458d..62bcc81f 100644 --- a/src/mms/inc_private/mms_server_internal.h +++ b/src/mms/inc_private/mms_server_internal.h @@ -219,7 +219,7 @@ LIB61850_INTERNAL MmsObtainFileTask MmsServer_getObtainFileTask(MmsServer self); LIB61850_INTERNAL void -mmsServer_fileUploadTask(MmsServer self, MmsObtainFileTask task); +mmsServer_fileUploadTask(MmsServer self, MmsObtainFileTask task, int taskState); #endif LIB61850_INTERNAL ByteBuffer* diff --git a/src/mms/iso_mms/server/mms_file_service.c b/src/mms/iso_mms/server/mms_file_service.c index 7731db66..2a0b18ad 100644 --- a/src/mms/iso_mms/server/mms_file_service.c +++ b/src/mms/iso_mms/server/mms_file_service.c @@ -411,206 +411,198 @@ createObtainFileResponse(uint32_t invokeId, ByteBuffer* response) } void -mmsServer_fileUploadTask(MmsServer self, MmsObtainFileTask task) +mmsServer_fileUploadTask(MmsServer self, MmsObtainFileTask task, int taskState) { - switch (task->state) { + /* call locks in certain order (lock IsoConnection -> reserverTransmitBuffer, task->taskLock) to prevent potential deadlock */ - case MMS_FILE_UPLOAD_STATE_NOT_USED: - break; - - case MMS_FILE_UPLOAD_STATE_FILE_OPEN_SENT: - { - if (Hal_getTimeInMs() > task->nextTimeout) { + ByteBuffer* message = NULL; - if (DEBUG_MMS_SERVER) - printf("MMS_SERVER: file open timeout!\n"); + if (taskState == MMS_FILE_UPLOAD_STATE_SEND_FILE_READ || + taskState == MMS_FILE_UPLOAD_STATE_SEND_FILE_CLOSE || + taskState == MMS_FILE_UPLOAD_STATE_SEND_OBTAIN_FILE_ERROR_SOURCE || + taskState == MMS_FILE_UPLOAD_STATE_SEND_OBTAIN_FILE_ERROR_DESTINATION || + taskState == MMS_FILE_UPLOAD_STATE_SEND_OBTAIN_FILE_RESPONSE) + { + IsoConnection_lock(task->connection->isoConnection); - task->state = MMS_FILE_UPLOAD_STATE_SEND_OBTAIN_FILE_ERROR_SOURCE; - - if(task->fileHandle){ - FileSystem_closeFile(task->fileHandle); - task->fileHandle = NULL; - } - deleteFile(MmsServer_getFilesystemBasepath(self), task->destinationFilename); - } - } - break; - - case MMS_FILE_UPLOAD_STATE_SEND_FILE_READ: - { - IsoConnection_lock(task->connection->isoConnection); - - ByteBuffer* request = MmsServer_reserveTransmitBuffer(self); - - task->lastRequestInvokeId = MmsServerConnection_getNextRequestInvokeId(task->connection); - - mmsClient_createFileReadRequest(task->lastRequestInvokeId, request, task->frmsId); - - task->state = MMS_FILE_UPLOAD_STATE_FILE_READ_SENT; - IsoConnection_sendMessage(task->connection->isoConnection, request); - - MmsServer_releaseTransmitBuffer(self); + message = MmsServer_reserveTransmitBuffer(self); + } - IsoConnection_unlock(task->connection->isoConnection); +#if (CONFIG_MMS_THREADLESS_STACK != 1) + Semaphore_wait(task->taskLock); +#endif - task->nextTimeout = Hal_getTimeInMs() + 2000; /* timeout 2000 ms */ - } + if (task->state == taskState) { - break; + switch (task->state) { - case MMS_FILE_UPLOAD_STATE_FILE_READ_SENT: + case MMS_FILE_UPLOAD_STATE_NOT_USED: + break; - if (Hal_getTimeInMs() > task->nextTimeout) { + case MMS_FILE_UPLOAD_STATE_FILE_OPEN_SENT: + { + if (Hal_getTimeInMs() > task->nextTimeout) { - if (DEBUG_MMS_SERVER) - printf("MMS_SERVER: file read timeout!\n"); + if (DEBUG_MMS_SERVER) + printf("MMS_SERVER: file open timeout!\n"); - task->state = MMS_FILE_UPLOAD_STATE_SEND_OBTAIN_FILE_ERROR_SOURCE; + task->state = MMS_FILE_UPLOAD_STATE_SEND_OBTAIN_FILE_ERROR_SOURCE; - if(task->fileHandle){ - FileSystem_closeFile(task->fileHandle); - task->fileHandle = NULL; + if(task->fileHandle){ + FileSystem_closeFile(task->fileHandle); + task->fileHandle = NULL; + } + deleteFile(MmsServer_getFilesystemBasepath(self), task->destinationFilename); + } } - deleteFile(MmsServer_getFilesystemBasepath(self), task->destinationFilename); - } - - break; - - case MMS_FILE_UPLOAD_STATE_SEND_FILE_CLOSE: - { - IsoConnection_lock(task->connection->isoConnection); - - ByteBuffer* request = MmsServer_reserveTransmitBuffer(self); + break; - task->lastRequestInvokeId = MmsServerConnection_getNextRequestInvokeId(task->connection); + case MMS_FILE_UPLOAD_STATE_SEND_FILE_READ: + { + task->lastRequestInvokeId = MmsServerConnection_getNextRequestInvokeId(task->connection); - mmsClient_createFileCloseRequest(task->lastRequestInvokeId, request, task->frmsId); + mmsClient_createFileReadRequest(task->lastRequestInvokeId, message, task->frmsId); - task->state = MMS_FILE_UPLOAD_STATE_FILE_CLOSE_SENT; + task->state = MMS_FILE_UPLOAD_STATE_FILE_READ_SENT; + IsoConnection_sendMessage(task->connection->isoConnection, message); - IsoConnection_sendMessage(task->connection->isoConnection, request); + task->nextTimeout = Hal_getTimeInMs() + 2000; /* timeout 2000 ms */ + } - MmsServer_releaseTransmitBuffer(self); + break; - IsoConnection_unlock(task->connection->isoConnection); + case MMS_FILE_UPLOAD_STATE_FILE_READ_SENT: - task->nextTimeout = Hal_getTimeInMs() + 2000; /* timeout 2000 ms */ + if (Hal_getTimeInMs() > task->nextTimeout) { - } - break; + if (DEBUG_MMS_SERVER) + printf("MMS_SERVER: file read timeout!\n"); - case MMS_FILE_UPLOAD_STATE_FILE_CLOSE_SENT: + task->state = MMS_FILE_UPLOAD_STATE_SEND_OBTAIN_FILE_ERROR_SOURCE; - if (Hal_getTimeInMs() > task->nextTimeout) { + if(task->fileHandle){ + FileSystem_closeFile(task->fileHandle); + task->fileHandle = NULL; + } + deleteFile(MmsServer_getFilesystemBasepath(self), task->destinationFilename); + } - if (DEBUG_MMS_SERVER) - printf("MMS_SERVER: file close timeout!\n"); + break; - task->state = MMS_FILE_UPLOAD_STATE_SEND_OBTAIN_FILE_ERROR_SOURCE; + case MMS_FILE_UPLOAD_STATE_SEND_FILE_CLOSE: + { + task->lastRequestInvokeId = MmsServerConnection_getNextRequestInvokeId(task->connection); - FileSystem_closeFile(task->fileHandle); - task->fileHandle = NULL; - deleteFile(MmsServer_getFilesystemBasepath(self), task->destinationFilename); - } + mmsClient_createFileCloseRequest(task->lastRequestInvokeId, message, task->frmsId); - break; + task->state = MMS_FILE_UPLOAD_STATE_FILE_CLOSE_SENT; - case MMS_FILE_UPLOAD_STATE_SEND_OBTAIN_FILE_ERROR_SOURCE: + IsoConnection_sendMessage(task->connection->isoConnection, message); - { - /* send ObtainFileError */ - IsoConnection_lock(task->connection->isoConnection); + task->nextTimeout = Hal_getTimeInMs() + 2000; /* timeout 2000 ms */ - ByteBuffer* response = MmsServer_reserveTransmitBuffer(self); + } + break; - createServiceErrorObtainFileError(task->obtainFileRequestInvokeId, response, MMS_ERROR_FILE_FILE_NON_EXISTENT, 0); + case MMS_FILE_UPLOAD_STATE_FILE_CLOSE_SENT: - IsoConnection_sendMessage(task->connection->isoConnection, response); + if (Hal_getTimeInMs() > task->nextTimeout) { - MmsServer_releaseTransmitBuffer(self); + if (DEBUG_MMS_SERVER) + printf("MMS_SERVER: file close timeout!\n"); - IsoConnection_unlock(task->connection->isoConnection); + task->state = MMS_FILE_UPLOAD_STATE_SEND_OBTAIN_FILE_ERROR_SOURCE; - if(task->fileHandle){ FileSystem_closeFile(task->fileHandle); task->fileHandle = NULL; + deleteFile(MmsServer_getFilesystemBasepath(self), task->destinationFilename); } - - deleteFile(MmsServer_getFilesystemBasepath(self), task->destinationFilename); - if (DEBUG_MMS_SERVER) - printf("MMS_SERVER: ObtainFile service: failed to open file from client\n"); + break; - task->state = MMS_FILE_UPLOAD_STATE_NOT_USED; - } - break; + case MMS_FILE_UPLOAD_STATE_SEND_OBTAIN_FILE_ERROR_SOURCE: - case MMS_FILE_UPLOAD_STATE_SEND_OBTAIN_FILE_ERROR_DESTINATION: - { - /* send ObtainFileError */ - IsoConnection_lock(task->connection->isoConnection); + { + /* send ObtainFileError */ + createServiceErrorObtainFileError(task->obtainFileRequestInvokeId, message, MMS_ERROR_FILE_FILE_NON_EXISTENT, 0); - ByteBuffer* response = MmsServer_reserveTransmitBuffer(self); + IsoConnection_sendMessage(task->connection->isoConnection, message); - createServiceErrorObtainFileError(task->obtainFileRequestInvokeId, response, MMS_ERROR_FILE_OTHER, 1); + if(task->fileHandle){ + FileSystem_closeFile(task->fileHandle); + task->fileHandle = NULL; + } + + deleteFile(MmsServer_getFilesystemBasepath(self), task->destinationFilename); - IsoConnection_sendMessage(task->connection->isoConnection, response); + if (DEBUG_MMS_SERVER) + printf("MMS_SERVER: ObtainFile service: failed to open file from client\n"); - MmsServer_releaseTransmitBuffer(self); + task->state = MMS_FILE_UPLOAD_STATE_NOT_USED; + } + break; - IsoConnection_unlock(task->connection->isoConnection); + case MMS_FILE_UPLOAD_STATE_SEND_OBTAIN_FILE_ERROR_DESTINATION: + { + /* send ObtainFileError */ + createServiceErrorObtainFileError(task->obtainFileRequestInvokeId, message, MMS_ERROR_FILE_OTHER, 1); - if (task->fileHandle) { - FileSystem_closeFile(task->fileHandle); - task->fileHandle = NULL; + IsoConnection_sendMessage(task->connection->isoConnection, message); - if (task->destinationFilename[0]) - deleteFile(MmsServer_getFilesystemBasepath(self), task->destinationFilename); - } + if (task->fileHandle) { + FileSystem_closeFile(task->fileHandle); + task->fileHandle = NULL; - if (DEBUG_MMS_SERVER) - printf("MMS_SERVER: ObtainFile service: failed to create local file\n"); + if (task->destinationFilename[0]) + deleteFile(MmsServer_getFilesystemBasepath(self), task->destinationFilename); + } - task->state = MMS_FILE_UPLOAD_STATE_NOT_USED; - } + if (DEBUG_MMS_SERVER) + printf("MMS_SERVER: ObtainFile service: failed to create local file\n"); - break; + task->state = MMS_FILE_UPLOAD_STATE_NOT_USED; + } - case MMS_FILE_UPLOAD_STATE_SEND_OBTAIN_FILE_RESPONSE: - { - IsoConnection_lock(task->connection->isoConnection); + break; - ByteBuffer* response = MmsServer_reserveTransmitBuffer(self); + case MMS_FILE_UPLOAD_STATE_SEND_OBTAIN_FILE_RESPONSE: + { + createObtainFileResponse(task->obtainFileRequestInvokeId, message); - createObtainFileResponse(task->obtainFileRequestInvokeId, response); + task->state = MMS_FILE_UPLOAD_STATE_NOT_USED; - task->state = MMS_FILE_UPLOAD_STATE_NOT_USED; + IsoConnection_sendMessage(task->connection->isoConnection, message); - IsoConnection_sendMessage(task->connection->isoConnection, response); + if (self->getFileCompleteHandler) + self->getFileCompleteHandler(self->getFileCompleteHandlerParameter, task->connection, task->destinationFilename); + } + break; - MmsServer_releaseTransmitBuffer(self); + case MMS_FILE_UPLOAD_STATE_INTERRUPTED: + { + if (DEBUG_MMS_SERVER) + printf("MMS_SERVER: file service interrupted, due to client disconnection\n"); - IsoConnection_unlock(task->connection->isoConnection); + if (task->fileHandle){ + FileSystem_closeFile(task->fileHandle); + task->fileHandle = NULL; - if (self->getFileCompleteHandler) - self->getFileCompleteHandler(self->getFileCompleteHandlerParameter, task->connection, task->destinationFilename); - } - break; - case MMS_FILE_UPLOAD_STATE_INTERRUPTED: - { - if (DEBUG_MMS_SERVER) - printf("MMS_SERVER: file service interrupted, due to client disconnection\n"); + if (task->destinationFilename[0]) + deleteFile(MmsServer_getFilesystemBasepath(self), task->destinationFilename); + } + task->state = MMS_FILE_UPLOAD_STATE_NOT_USED; + } + break; + } + } - if (task->fileHandle){ - FileSystem_closeFile(task->fileHandle); - task->fileHandle = NULL; +#if (CONFIG_MMS_THREADLESS_STACK != 1) + Semaphore_post(task->taskLock); +#endif - if (task->destinationFilename[0]) - deleteFile(MmsServer_getFilesystemBasepath(self), task->destinationFilename); - } - task->state = MMS_FILE_UPLOAD_STATE_NOT_USED; - } - break; + if (message) { + MmsServer_releaseTransmitBuffer(self); + IsoConnection_unlock(task->connection->isoConnection); } } @@ -770,6 +762,10 @@ mmsServer_handleObtainFileRequest( task->state = MMS_FILE_UPLOAD_STATE_FILE_OPEN_SENT; } + +#if (CONFIG_MMS_THREADLESS_STACK != 1) + Semaphore_post(task->taskLock); +#endif } else goto exit_unavailable; diff --git a/src/mms/iso_mms/server/mms_server.c b/src/mms/iso_mms/server/mms_server.c index 2cee0021..e8e35036 100644 --- a/src/mms/iso_mms/server/mms_server.c +++ b/src/mms/iso_mms/server/mms_server.c @@ -316,10 +316,6 @@ MmsServer_getObtainFileTask(MmsServer self) if (self->fileUploadTasks[i].state == 0) { self->fileUploadTasks[i].state = 1; -#if (CONFIG_MMS_THREADLESS_STACK != 1) - Semaphore_post(self->fileUploadTasks[i].taskLock); -#endif - return &(self->fileUploadTasks[i]); } @@ -742,15 +738,15 @@ MmsServer_handleBackgroundTasks(MmsServer self) Semaphore_wait(self->fileUploadTasks[i].taskLock); #endif - if (self->fileUploadTasks[i].state != 0) { - - if (self->fileUploadTasks[i].state != 0) - mmsServer_fileUploadTask(self, &(self->fileUploadTasks[i])); - } + int taskState = self->fileUploadTasks[i].state; #if (CONFIG_MMS_THREADLESS_STACK != 1) Semaphore_post(self->fileUploadTasks[i].taskLock); #endif + + if (taskState != 0) { + mmsServer_fileUploadTask(self, &(self->fileUploadTasks[i]), taskState); + } } #endif /* (MMS_OBTAIN_FILE_SERVICE == 1) */