Merge pull request nasa#412 from dmknutsen/Issue_392

Fix nasa#392, seg fault - invalid file destination
thnkslprpt · Oct 19, 2023 · 046a70a · 046a70a
2 parents 3d46fad + 0ae687d
commit 046a70a
Show file tree

Hide file tree

Showing 4 changed files with 27 additions and 0 deletions.
diff --git a/docs/cf_FunctionalRequirements.csv b/docs/cf_FunctionalRequirements.csv
@@ -17,6 +17,7 @@ CF2002.1.2,CF2002.1.2,"CF shall detect the following scenarios and identify them
 4. File-Size Error
 5. NAK Limit Reached
 6. Inactivity Limit Reached",Fault scenarios explicitly listed and tested for specification compliance.
+CF2002.1.3,CF2002.1.3,"CF shall issue issue a DEBUG message if an attempt is made to reset a transaction that has already been freed"
 CF3000,CF3000,"When CF receives a ""Transfer File"" command, CF shall play back the file indicated by the command-specified: filename, source path, destination path, keep/delete flag, service class, priority, channel, and peer-entity id. ","Also referred to as ""playback file"" command. Basic function of file transfer required to operate cFS flight systems. "
 CF3000.1,CF3000.1,"When CF receives a ""Transfer File"" command, if the command-specified is open, CF reject the command.",Open files are in a uncertain state and may change during transfer potential containing erroneous data or cause other undefined behaviors
 CF3000.2,CF3000.2,

diff --git a/fsw/inc/cf_events.h b/fsw/inc/cf_events.h
@@ -363,6 +363,19 @@
  * CF_CFDP event IDs - Engine
  */
 
+/**
+ * \brief Attempt to reset a transaction that has already been freed
+ *
+ *  \par Type: DEBUG
+ *
+ *  \par Cause:
+ *
+ *  Can be induced via various off-nominal conditions - such as sending a META-data PDU
+ *  with an invalid file destination.
+ *  
+ */
+#define CF_EID_DBG_RESET_FREED_XACT (59)
+
 /**
  * \brief CF PDU Received Without Existing Transaction, Dropped Due To Max RX Reached Event ID
  *

diff --git a/fsw/src/cf_cfdp.c b/fsw/src/cf_cfdp.c
@@ -1610,6 +1610,13 @@ void CF_CFDP_ResetTransaction(CF_Transaction_t *txn, int keep_history)
     CF_Channel_t *chan   = &CF_AppData.engine.channels[txn->chan_num];
     CF_Assert(txn->chan_num < CF_NUM_CHANNELS);
 
+    if ( txn->flags.com.q_index == CF_QueueIdx_FREE)
+    {
+        CFE_EVS_SendEvent(CF_EID_DBG_RESET_FREED_XACT, CFE_EVS_EventType_DEBUG, \
+        "CF: attempt to reset a transaction that has already been freed");
+        return;
+    }
+
     CF_CFDP_SendEotPkt(txn);
 
     CF_DequeueTransaction(txn);

diff --git a/unit-test/cf_cfdp_tests.c b/unit-test/cf_cfdp_tests.c
@@ -1360,6 +1360,12 @@ void Test_CF_CFDP_ResetTransaction(void)
 
     memset(&pb, 0, sizeof(pb));
 
+    /* Attempt to reset a transaction that has already been freed*/
+    UT_ResetState(UT_KEY(CF_FreeTransaction));
+    UT_CFDP_SetupBasicTestState(UT_CF_Setup_RX, NULL, NULL, NULL, &txn, NULL);
+    txn->flags.com.q_index = CF_QueueIdx_FREE;
+    UtAssert_VOIDCALL(CF_CFDP_ResetTransaction(txn, 0));  
+
     /* nominal call */
     UT_CFDP_SetupBasicTestState(UT_CF_Setup_NONE, NULL, NULL, NULL, &txn, NULL);
     CF_AppData.hk.channel_hk[UT_CFDP_CHANNEL].q_size[txn->flags.com.q_index] = 10;