Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

CFE_PSP_Module_FindByName uses incorrect list limit/stop condition #353

Closed
jphickey opened this issue Jul 28, 2022 · 1 comment · Fixed by #354 or #356
Closed

CFE_PSP_Module_FindByName uses incorrect list limit/stop condition #353

jphickey opened this issue Jul 28, 2022 · 1 comment · Fixed by #354 or #356
Assignees
Labels
bug Something isn't working
Milestone

Comments

@jphickey
Copy link
Contributor

jphickey commented Jul 28, 2022

Describe the bug
The internal variable CFE_PSP_ModuleCount is used as the limit for searching for a matching name in the PSP module list here:

while (i < CFE_PSP_ModuleCount)

However, a while back the set of modules was split into two, so now we have separate lists:

CFE_PSP_BASE_MODULE_LIST
GLOBAL_CONFIGDATA.PspModuleList

The CFE_PSP_ModuleCount value reflects the total number of entries (sum) of both lists.
But the CFE_PSP_Module_FindByName function is only searching the second list (GLOBAL_CONFIGDATA.PspModuleList). This means that if there is no matching entry, it might read beyond the end of the list.

To Reproduce
Call CFE_PSP_Module_FindByName on a module name that does not actually exist in the system. The loop will segfault when it gets to the NULL entry that normally terminates the GLOBAL_CONFIGDATA.PspModuleList.

Expected behavior
The search should cover CFE_PSP_BASE_MODULE_LIST and GLOBAL_CONFIGDATA.PspModuleList like the init function does

Code snips
Init function uses a helper function that does each list at:

void CFE_PSP_ModuleInit(void)
{
/* First initialize the fixed set of modules for this PSP */
CFE_PSP_ModuleInitList(CFE_PSP_BASE_MODULE_LIST);
/* Then initialize any user-selected extension modules */
CFE_PSP_ModuleInitList(GLOBAL_CONFIGDATA.PspModuleList);
}

Search function should mirror this.

System observed on:
Ubuntu 22.04

Reporter Info
Joseph Hickey, Vantage Systems, Inc. on behalf of Alan Cudmore (GSFC)

@jphickey jphickey added the bug Something isn't working label Jul 28, 2022
@jphickey
Copy link
Contributor Author

Worth noting that this code in this file does NOT seem to have a coverage test for it ... this limit mismatch issue may have been caught by such a test.

@jphickey jphickey self-assigned this Jul 28, 2022
jphickey added a commit to jphickey/PSP that referenced this issue Jul 28, 2022
Correct the CFE_PSP_Module_FindByName and CFE_PSP_Module_GetAPIEntry
lookups to use the correct length of the config data 'PspModuleList'.

The internal variable that had been used reflected the total across
two lists and thus was not a correct limit.

Rather than having these functions search both lists, this instead
keeps it to only exposing the user-specified modules in PspModuleList,
it does not expose the built-in/base modules.  This makes the change
simpler, as typically the built in modules would not be accessed
through these functions at all.
dzbaker added a commit that referenced this issue Aug 3, 2022
Fix #353, correct PSP module IDs and lookups
@dmknutsen dmknutsen added this to the Draco milestone Jan 18, 2023
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
bug Something isn't working
Projects
None yet
2 participants