From 0ae687d024819cc689eaf3f6d1c4df1a24be1c37 Mon Sep 17 00:00:00 2001 From: Dan Date: Tue, 17 Oct 2023 13:43:37 -0400 Subject: [PATCH] Fix #392, seg fault - invalid file destination --- docs/cf_FunctionalRequirements.csv | 1 + fsw/inc/cf_events.h | 13 +++++++++++++ fsw/src/cf_cfdp.c | 7 +++++++ unit-test/cf_cfdp_tests.c | 6 ++++++ 4 files changed, 27 insertions(+) diff --git a/docs/cf_FunctionalRequirements.csv b/docs/cf_FunctionalRequirements.csv index cc294896..c24127a7 100644 --- 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 index 9910ef0a..a5233a2e 100644 --- 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 index 33d86cc9..ae3d1649 100644 --- 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 index f1d0615c..b164394b 100644 --- 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;