From ebdc086b8eeb1ffb336cd7ddb05104e82b6555bb 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 | 23 ++++++++---- src/mms/iso_mms/server/mms_server.c | 37 ++++++++++++++++++- 7 files changed, 98 insertions(+), 14 deletions(-) diff --git a/src/mms/inc_private/mms_client_internal.h b/src/mms/inc_private/mms_client_internal.h index a1b14e5f..a4cfedec 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 ce144dfd..eb64f432 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 b939abb5..e749733e 100644 --- a/src/mms/inc_private/mms_server_libinternal.h +++ b/src/mms/inc_private/mms_server_libinternal.h @@ -187,4 +187,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 cbbbca81..e4c2c78d 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; } @@ -1539,6 +1558,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 2c1831f1..cf5d3122 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; @@ -535,7 +535,8 @@ 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); if (DEBUG_MMS_SERVER) printf("MMS_SERVER: ObtainFile service: failed to open file from client\n"); @@ -563,8 +564,8 @@ mmsServer_fileUploadTask(MmsServer self, MmsObtainFileTask task) FileSystem_closeFile(task->fileHandle); task->fileHandle = NULL; - if (task->destinationFilename) - deleteFile(MmsServerConnection_getFilesystemBasepath(task->connection), task->destinationFilename); + if (task->destinationFilename[0]) + deleteFile(MmsServer_getFilesystemBasepath(self), task->destinationFilename); } if (DEBUG_MMS_SERVER) @@ -602,8 +603,9 @@ mmsServer_fileUploadTask(MmsServer self, MmsObtainFileTask task) if (task->fileHandle){ FileSystem_closeFile(task->fileHandle); task->fileHandle = NULL; - if (task->destinationFilename) - deleteFile(MmsServerConnection_getFilesystemBasepath(task->connection), task->destinationFilename); + + if (task->destinationFilename[0]) + deleteFile(MmsServer_getFilesystemBasepath(self), task->destinationFilename); } task->state = MMS_FILE_UPLOAD_STATE_NOT_USED; } @@ -625,8 +627,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 93622031..718d67b1 100644 --- a/src/mms/iso_mms/server/mms_server.c +++ b/src/mms/iso_mms/server/mms_server.c @@ -202,6 +202,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]); } @@ -314,6 +316,16 @@ MmsServer_destroy(MmsServer self) GLOBAL_FREEMEM(self->filestoreBasepath); #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); } @@ -508,8 +520,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) */ @@ -535,3 +554,17 @@ MmsServer_stopListeningThreadless(MmsServer self) IsoServer_stopListeningThreadless(self->isoServer); } +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 +} + +