From 161e88a3efdb8e308319d7ce2e2efa3f96c84e64 Mon Sep 17 00:00:00 2001 From: Michael Zillgith Date: Fri, 11 Jun 2021 17:10:23 +0200 Subject: [PATCH] - MMS server: fixed potential crash when client connection closed during file upload (LIB61850-2) - MMS client: fixed problem - doesn't close file when the setFile (obtainFile) service is interrupted e.g. due to connection loss (LIB61850-230) --- src/mms/inc_private/mms_client_internal.h | 11 +++-- src/mms/inc_private/mms_server_internal.h | 3 ++ src/mms/inc_private/mms_server_libinternal.h | 3 ++ .../iso_mms/client/mms_client_connection.c | 22 ++++++++++ src/mms/iso_mms/client/mms_client_files.c | 13 ++++++ src/mms/iso_mms/server/mms_file_service.c | 17 +++++--- src/mms/iso_mms/server/mms_server.c | 43 ++++++++++++++++--- 7 files changed, 96 insertions(+), 16 deletions(-) diff --git a/src/mms/inc_private/mms_client_internal.h b/src/mms/inc_private/mms_client_internal.h index 81f68e91..4dd10ded 100644 --- a/src/mms/inc_private/mms_client_internal.h +++ b/src/mms/inc_private/mms_client_internal.h @@ -370,10 +370,13 @@ mmsClient_handleFileReadRequest( LIB61850_INTERNAL void mmsClient_handleFileCloseRequest( -MmsConnection connection, -uint8_t* buffer, int bufPos, int maxBufPos, -uint32_t invokeId, -ByteBuffer* response); + MmsConnection connection, + uint8_t* buffer, int bufPos, int maxBufPos, + uint32_t invokeId, + ByteBuffer* response); + +LIB61850_INTERNAL void +mmsClient_closeOutstandingOpenFiles(MmsConnection connection); LIB61850_INTERNAL MmsOutstandingCall mmsClient_getMatchingObtainFileRequest(MmsConnection self, const char* filename); diff --git a/src/mms/inc_private/mms_server_internal.h b/src/mms/inc_private/mms_server_internal.h index df06d35f..5e2f458d 100644 --- a/src/mms/inc_private/mms_server_internal.h +++ b/src/mms/inc_private/mms_server_internal.h @@ -98,6 +98,9 @@ struct sMmsObtainFileTask { uint64_t nextTimeout; int32_t frmsId; int state; +#if (CONFIG_MMS_THREADLESS_STACK != 1) + Semaphore taskLock; +#endif }; #endif /* (MMS_OBTAIN_FILE_SERVICE == 1) */ diff --git a/src/mms/inc_private/mms_server_libinternal.h b/src/mms/inc_private/mms_server_libinternal.h index 2e18f4a2..ef8f95f4 100644 --- a/src/mms/inc_private/mms_server_libinternal.h +++ b/src/mms/inc_private/mms_server_libinternal.h @@ -193,4 +193,7 @@ MmsServer_getConnectionCounter(MmsServer self); LIB61850_INTERNAL void MmsServer_stopListeningThreadless(MmsServer self); +LIB61850_INTERNAL const char* +MmsServer_getFilesystemBasepath(MmsServer self); + #endif /* MMS_SERVER_LIBINTERNAL_H_ */ diff --git a/src/mms/iso_mms/client/mms_client_connection.c b/src/mms/iso_mms/client/mms_client_connection.c index 5644912d..241ddcb4 100644 --- a/src/mms/iso_mms/client/mms_client_connection.c +++ b/src/mms/iso_mms/client/mms_client_connection.c @@ -1035,6 +1035,25 @@ mmsIsoCallback(IsoIndication indication, void* parameter, ByteBuffer* payload) if (self->connectionLostHandler != NULL) self->connectionLostHandler(self, self->connectionLostHandlerParameter); + + /* Cleanup outstanding calls */ + { + int i; + + for (i = 0; i < OUTSTANDING_CALLS; i++) { + if (self->outstandingCalls[i].isUsed) { + + if (self->outstandingCalls[i].type != MMS_CALL_TYPE_NONE) + handleAsyncResponse(self, NULL, 0, &(self->outstandingCalls[i]), MMS_ERROR_SERVICE_TIMEOUT); + + self->outstandingCalls[i].isUsed = false; + break; + } + } + } + + Semaphore_post(self->outstandingCallsLock); + return true; } @@ -1541,6 +1560,9 @@ MmsConnection_destroy(MmsConnection self) if (self->filestoreBasepath != NULL) GLOBAL_FREEMEM(self->filestoreBasepath); #endif + + /* Close outstanding open files */ + mmsClient_closeOutstandingOpenFiles(self); #endif GLOBAL_FREEMEM(self); diff --git a/src/mms/iso_mms/client/mms_client_files.c b/src/mms/iso_mms/client/mms_client_files.c index d9215abc..307ab534 100644 --- a/src/mms/iso_mms/client/mms_client_files.c +++ b/src/mms/iso_mms/client/mms_client_files.c @@ -224,6 +224,19 @@ mmsClient_handleFileCloseRequest( mmsMsg_createServiceErrorPdu(invokeId, response, MMS_ERROR_FILE_OTHER); } +void +mmsClient_closeOutstandingOpenFiles(MmsConnection connection) +{ + int i; + + for (i = 0; i < CONFIG_MMS_MAX_NUMBER_OF_OPEN_FILES_PER_CONNECTION; i++) { + if (connection->frsms[i].fileHandle != NULL) { + FileSystem_closeFile(connection->frsms[i].fileHandle); + connection->frsms[i].fileHandle = NULL; + } + } +} + #endif /* (MMS_OBTAIN_FILE_SERVICE == 1) */ diff --git a/src/mms/iso_mms/server/mms_file_service.c b/src/mms/iso_mms/server/mms_file_service.c index d569fab6..eb3cfbab 100644 --- a/src/mms/iso_mms/server/mms_file_service.c +++ b/src/mms/iso_mms/server/mms_file_service.c @@ -431,7 +431,7 @@ mmsServer_fileUploadTask(MmsServer self, MmsObtainFileTask task) FileSystem_closeFile(task->fileHandle); task->fileHandle = NULL; } - deleteFile(MmsServerConnection_getFilesystemBasepath(task->connection), task->destinationFilename); + deleteFile(MmsServer_getFilesystemBasepath(self), task->destinationFilename); } } break; @@ -471,7 +471,7 @@ mmsServer_fileUploadTask(MmsServer self, MmsObtainFileTask task) FileSystem_closeFile(task->fileHandle); task->fileHandle = NULL; } - deleteFile(MmsServerConnection_getFilesystemBasepath(task->connection), task->destinationFilename); + deleteFile(MmsServer_getFilesystemBasepath(self), task->destinationFilename); } break; @@ -510,7 +510,7 @@ mmsServer_fileUploadTask(MmsServer self, MmsObtainFileTask task) FileSystem_closeFile(task->fileHandle); task->fileHandle = NULL; - deleteFile(MmsServerConnection_getFilesystemBasepath(task->connection), task->destinationFilename); + deleteFile(MmsServer_getFilesystemBasepath(self), task->destinationFilename); } break; @@ -536,7 +536,7 @@ mmsServer_fileUploadTask(MmsServer self, MmsObtainFileTask task) task->fileHandle = NULL; } - deleteFile(MmsServerConnection_getFilesystemBasepath(task->connection), task->destinationFilename); + deleteFile(MmsServer_getFilesystemBasepath(self), task->destinationFilename); if (DEBUG_MMS_SERVER) printf("MMS_SERVER: ObtainFile service: failed to open file from client\n"); @@ -565,7 +565,7 @@ mmsServer_fileUploadTask(MmsServer self, MmsObtainFileTask task) task->fileHandle = NULL; if (task->destinationFilename[0]) - deleteFile(MmsServerConnection_getFilesystemBasepath(task->connection), task->destinationFilename); + deleteFile(MmsServer_getFilesystemBasepath(self), task->destinationFilename); } if (DEBUG_MMS_SERVER) @@ -606,7 +606,7 @@ mmsServer_fileUploadTask(MmsServer self, MmsObtainFileTask task) task->fileHandle = NULL; if (task->destinationFilename[0]) - deleteFile(MmsServerConnection_getFilesystemBasepath(task->connection), task->destinationFilename); + deleteFile(MmsServer_getFilesystemBasepath(self), task->destinationFilename); } task->state = MMS_FILE_UPLOAD_STATE_NOT_USED; } @@ -628,8 +628,13 @@ mmsServerConnection_stopFileUploadTasks(MmsServerConnection self) if (server->fileUploadTasks[i].state != 0) { if (server->fileUploadTasks[i].connection == self) { + + Semaphore_wait(server->fileUploadTasks[i].taskLock); + /* stop file upload task */ server->fileUploadTasks[i].state = MMS_FILE_UPLOAD_STATE_INTERRUPTED; + + Semaphore_post(server->fileUploadTasks[i].taskLock); } } diff --git a/src/mms/iso_mms/server/mms_server.c b/src/mms/iso_mms/server/mms_server.c index 0908babb..cfed8824 100644 --- a/src/mms/iso_mms/server/mms_server.c +++ b/src/mms/iso_mms/server/mms_server.c @@ -296,6 +296,8 @@ MmsServer_getObtainFileTask(MmsServer self) if (self->fileUploadTasks[i].state == 0) { self->fileUploadTasks[i].state = 1; + if (self->fileUploadTasks[i].taskLock == NULL) + self->fileUploadTasks[i].taskLock = Semaphore_create(1); return &(self->fileUploadTasks[i]); } @@ -409,7 +411,7 @@ MmsServer_destroy(MmsServer self) Map_deleteDeep(self->openConnections, false, closeConnection); Map_deleteDeep(self->valueCaches, false, (void (*) (void*)) deleteSingleCache); - #if (CONFIG_MMS_THREADLESS_STACK != 1) +#if (CONFIG_MMS_THREADLESS_STACK != 1) if (self->openConnectionsLock) Semaphore_destroy(self->openConnectionsLock); @@ -418,15 +420,23 @@ MmsServer_destroy(MmsServer self) if (self->transmitBufferMutex) Semaphore_destroy(self->transmitBufferMutex); - #endif +#endif if (self->transmitBuffer) ByteBuffer_destroy(self->transmitBuffer); - #if (CONFIG_SET_FILESTORE_BASEPATH_AT_RUNTIME == 1) +#if (CONFIG_SET_FILESTORE_BASEPATH_AT_RUNTIME == 1) if (self->filestoreBasepath != NULL) GLOBAL_FREEMEM(self->filestoreBasepath); - #endif +#endif + +#if (MMS_OBTAIN_FILE_SERVICE == 1) + int i; + for (i = 0; i < CONFIG_MMS_SERVER_MAX_GET_FILE_TASKS; i++) { + if (self->fileUploadTasks[i].taskLock) + Semaphore_destroy(self->fileUploadTasks[i].taskLock); + } +#endif GLOBAL_FREEMEM(self); } @@ -699,8 +709,15 @@ MmsServer_handleBackgroundTasks(MmsServer self) int i; for (i = 0; i < CONFIG_MMS_SERVER_MAX_GET_FILE_TASKS; i++) { - if (self->fileUploadTasks[i].state != 0) - mmsServer_fileUploadTask(self, &(self->fileUploadTasks[i])); + if (self->fileUploadTasks[i].state != 0) { + + Semaphore_wait(self->fileUploadTasks[i].taskLock); + + if (self->fileUploadTasks[i].state != 0) + mmsServer_fileUploadTask(self, &(self->fileUploadTasks[i])); + + Semaphore_post(self->fileUploadTasks[i].taskLock); + } } #endif /* (MMS_OBTAIN_FILE_SERVICE == 1) */ @@ -750,3 +767,17 @@ MmsServer_stopListeningThreadless(MmsServer self) } } +const char* +MmsServer_getFilesystemBasepath(MmsServer self) +{ +#if (CONFIG_SET_FILESTORE_BASEPATH_AT_RUNTIME == 1) + if (self->filestoreBasepath != NULL) + return self->filestoreBasepath; + else + return CONFIG_VIRTUAL_FILESTORE_BASEPATH; +#else + return CONFIG_VIRTUAL_FILESTORE_BASEPATH; +#endif +} + +