From fe1c7e1076643faeb1bdb4731b6e3a119cee7715 Mon Sep 17 00:00:00 2001 From: Walker Date: Wed, 13 May 2026 13:52:12 -0500 Subject: [PATCH 1/3] Part cFS/workflows#129, Use Updated Static Analysis Workflow --- .github/workflows/static-analysis.yml | 5 +++-- 1 file changed, 3 insertions(+), 2 deletions(-) diff --git a/.github/workflows/static-analysis.yml b/.github/workflows/static-analysis.yml index 9c279a0..85dccf2 100644 --- a/.github/workflows/static-analysis.yml +++ b/.github/workflows/static-analysis.yml @@ -1,6 +1,5 @@ name: Static Analysis -# Run on all push and pull requests on: push: branches: @@ -16,4 +15,6 @@ on: jobs: static-analysis: name: Static Analysis - uses: nasa/cFS/.github/workflows/app-static-analysis-reusable.yml@dev \ No newline at end of file + uses: nasa/cFS/.github/workflows/static-analysis-reusable.yml@dev + with: + source-dir: 'source/fsw' \ No newline at end of file From 99cec13adb57ddcda367633902644c854ab762ca Mon Sep 17 00:00:00 2001 From: Joseph Hickey Date: Fri, 5 Jun 2026 14:03:36 -0400 Subject: [PATCH 2/3] Fix #162, change RTS IDs to correct pattern Changes the definition to the proper "default" pattern which allows the value to be overridden on a mission config basis. --- docs/dox_src/cfs_sc.dox | 4 ++-- fsw/inc/sc_internal_cfg.h | 6 ++++-- fsw/src/sc_app.c | 4 ++-- unit-test/sc_app_tests.c | 4 ++-- 4 files changed, 10 insertions(+), 8 deletions(-) diff --git a/docs/dox_src/cfs_sc.dox b/docs/dox_src/cfs_sc.dox index 0ffb7bc..1fcadb3 100644 --- a/docs/dox_src/cfs_sc.dox +++ b/docs/dox_src/cfs_sc.dox @@ -139,11 +139,11 @@

Power On Resets

- The RTS ID defined by #RTS_ID_AUTO_POWER_ON is started (if non-zero). + The RTS ID defined by #SC_RTS_ID_AUTO_POWER_ON is started (if non-zero).

Processor Resets

- The RTS ID defined by #RTS_ID_AUTO_PROCESSOR is started (if non-zero). + The RTS ID defined by #SC_RTS_ID_AUTO_PROCESSOR is started (if non-zero).

Absolute Time Processor (ATP)

diff --git a/fsw/inc/sc_internal_cfg.h b/fsw/inc/sc_internal_cfg.h index 854452c..fe3f866 100644 --- a/fsw/inc/sc_internal_cfg.h +++ b/fsw/inc/sc_internal_cfg.h @@ -333,7 +333,8 @@ * \par Limits: * Must be a valid RTS ID or 0 */ -#define RTS_ID_AUTO_POWER_ON 1 +#define SC_RTS_ID_AUTO_POWER_ON SC_INTERNAL_CFGVAL(RTS_ID_AUTO_POWER_ON) +#define DEFAULT_SC_INTERNAL_RTS_ID_AUTO_POWER_ON 0 /** * \brief Autostart RTS ID after processor reset @@ -345,7 +346,8 @@ * \par Limits: * Must be a valid RTS ID or 0 */ -#define RTS_ID_AUTO_PROCESSOR 2 +#define SC_RTS_ID_AUTO_PROCESSOR SC_INTERNAL_CFGVAL(RTS_ID_AUTO_PROCESSOR) +#define DEFAULT_SC_INTERNAL_RTS_ID_AUTO_PROCESSOR 0 /** * \brief Mission specific version number for SC application diff --git a/fsw/src/sc_app.c b/fsw/src/sc_app.c index 7b8c2f8..b367e18 100644 --- a/fsw/src/sc_app.c +++ b/fsw/src/sc_app.c @@ -174,11 +174,11 @@ CFE_Status_t SC_AppInit(void) /* Select auto-exec RTS to start during first HK request */ if (CFE_ES_GetResetType(NULL) == CFE_PSP_RST_TYPE_POWERON) { - SC_AppData.AutoStartRTS = SC_RTS_NUM_C(RTS_ID_AUTO_POWER_ON); + SC_AppData.AutoStartRTS = SC_RTS_NUM_C(SC_RTS_ID_AUTO_POWER_ON); } else { - SC_AppData.AutoStartRTS = SC_RTS_NUM_C(RTS_ID_AUTO_PROCESSOR); + SC_AppData.AutoStartRTS = SC_RTS_NUM_C(SC_RTS_ID_AUTO_PROCESSOR); } /* Must be able to register for events */ diff --git a/unit-test/sc_app_tests.c b/unit-test/sc_app_tests.c index 35cb710..6401ac2 100644 --- a/unit-test/sc_app_tests.c +++ b/unit-test/sc_app_tests.c @@ -146,7 +146,7 @@ void SC_AppInit_Test_NominalPowerOnReset(void) Expected_SC_AppData.NextCmdTime[SC_Process_ATP] = SC_MAX_TIME; Expected_SC_AppData.NextCmdTime[SC_Process_RTP] = SC_MAX_WAKEUP_CNT; - Expected_SC_AppData.AutoStartRTS = SC_RTS_NUM_C(RTS_ID_AUTO_POWER_ON); + Expected_SC_AppData.AutoStartRTS = SC_RTS_NUM_C(SC_RTS_ID_AUTO_POWER_ON); UT_SetDataBuffer(UT_KEY(CFE_MSG_GetMsgId), &TestMsgId, sizeof(TestMsgId), false); UT_SetDataBuffer(UT_KEY(CFE_MSG_GetSize), &MsgSize, sizeof(MsgSize), false); @@ -223,7 +223,7 @@ void SC_AppInit_Test_Nominal(void) Expected_SC_AppData.NextCmdTime[SC_Process_ATP] = SC_MAX_TIME; Expected_SC_AppData.NextCmdTime[SC_Process_RTP] = SC_MAX_WAKEUP_CNT; - Expected_SC_AppData.AutoStartRTS = SC_RTS_NUM_C(RTS_ID_AUTO_PROCESSOR); + Expected_SC_AppData.AutoStartRTS = SC_RTS_NUM_C(SC_RTS_ID_AUTO_PROCESSOR); Expected_SC_OperData.HkPacket.Payload.ContinueAtsOnFailureFlag = SC_AtsCont_TRUE; From a781f2c59b0b8c6dc6d8f77e07469c07e5b1a258 Mon Sep 17 00:00:00 2001 From: nurdmuny Date: Thu, 11 Jun 2026 07:02:05 -0700 Subject: [PATCH 3/3] sc: do not publish uninitialised TblPtrNew when CFE_TBL_GetAddress fails `SC_ManageTable` re-acquires a table address after `CFE_TBL_Manage`: void *TblPtrNew; ... Result = CFE_TBL_GetAddress(&TblPtrNew, TblHandle); *TblAddr = TblPtrNew; /* sets this to NULL if it fails */ The inline comment is wrong. Per `cfe_tbl.h`, `CFE_TBL_GetAddress` leaves the output pointer **undefined** on any non-success return, not NULL. Documented failures: - CFE_TBL_ERR_NEVER_LOADED - CFE_TBL_ERR_INVALID_HANDLE - CFE_TBL_ERR_NO_ACCESS - CFE_ES_ERR_RESOURCEID_NOT_VALID - CFE_TBL_BAD_ARGUMENT Only `CFE_SUCCESS` and `CFE_TBL_INFO_UPDATED` set the pointer. `TblPtrNew` is a local declared without initialiser. On any of the failure paths above the unconditional `*TblAddr = TblPtrNew;` writes whatever was on the stack at that slot into `SC_OperData.AtsTblAddr[i]` / `RtsTblAddr[i]` / `AppendTblAddr`. Subsequent code in `sc_loads.c`, `sc_atsrq.c`, and `sc_cmds.c` reads those slots via `SC_GetAtsEntryAtOffset(...)` and dereferences a wild pointer. Fix: 1. Initialise `TblPtrNew = NULL` before the call so a failing `CFE_TBL_GetAddress` cannot leave stack garbage in it. 2. Only publish `TblPtrNew` to `*TblAddr` on `CFE_SUCCESS` or `CFE_TBL_INFO_UPDATED`; otherwise write NULL so downstream code sees a clean NULL (which it can null-check) rather than a wild pointer. 3. Remove the misleading inline comment and document the actual `cfe_tbl.h` contract. Same defense-in-depth shape as nasa/MM#116 (DataSize switch with no default case) and nasa/FM#143 (the analogous narrow-error check in `FM_SetTableStateCmd` / `FM_MonitorFilesystemSpaceCmd` / `FM_AcquireTablePointers`). Surfaced by scj-hunt + Gigi ATTEND on the `cfs_apps_sc_v01` fiber bundle. `SC_ManageTable` ranked in the top cluster at risk_score 8.8; the underlying mistake about the `CFE_TBL_GetAddress` contract was the same root cause as the FM finding. --- fsw/src/sc_cmds.c | 23 ++++++++++++++++++++--- 1 file changed, 20 insertions(+), 3 deletions(-) diff --git a/fsw/src/sc_cmds.c b/fsw/src/sc_cmds.c index 57e2687..1733443 100644 --- a/fsw/src/sc_cmds.c +++ b/fsw/src/sc_cmds.c @@ -745,9 +745,26 @@ void SC_ManageTable(SC_TableType type, int32 ArrayIndex) /* Allow cFE to manage table */ CFE_TBL_Manage(TblHandle); - /* Re-acquire table data pointer */ - Result = CFE_TBL_GetAddress(&TblPtrNew, TblHandle); - *TblAddr = TblPtrNew; /* Note that CFE_TBL_GetAddress() sets this to NULL if it fails */ + /* + ** Re-acquire table data pointer. Per cfe_tbl.h, CFE_TBL_GetAddress leaves + ** TblPtrNew *undefined* on any non-success return -- it does NOT set it + ** to NULL (the inline comment that used to live here was wrong). Only + ** CFE_SUCCESS and CFE_TBL_INFO_UPDATED set the pointer. Default to NULL + ** before the call and only publish the address on success so a failed + ** acquire can't write uninitialised stack memory into the shared + ** SC_OperData.*TblAddr slot (where downstream code in sc_loads.c / + ** sc_atsrq.c would dereference it as a wild pointer). + */ + TblPtrNew = NULL; + Result = CFE_TBL_GetAddress(&TblPtrNew, TblHandle); + if (Result == CFE_SUCCESS || Result == CFE_TBL_INFO_UPDATED) + { + *TblAddr = TblPtrNew; + } + else + { + *TblAddr = NULL; + } if (Result == CFE_TBL_INFO_UPDATED) { /* Process new table data */