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_ES_FindCDSInRegistry() infinte loop if registry size is zero #823

Closed
jphickey opened this issue Aug 20, 2020 · 0 comments · Fixed by #857 or #861
Closed

CFE_ES_FindCDSInRegistry() infinte loop if registry size is zero #823

jphickey opened this issue Aug 20, 2020 · 0 comments · Fixed by #857 or #861
Assignees
Labels
Milestone

Comments

@jphickey
Copy link
Contributor

Describe the bug
The loop inside this function has a poorly-constructed condition for ending the loop. It is comparing a uint32 type to an int32 type, and in the event that the MaxNumRegEntries is zero, it becomes impossible for this condition to be true and the loop runs forever.

To Reproduce
Easy to see in unit test if one clears the CFE_ES_Global data structure between tests.

Expected behavior
Loops should never have ending conditions that are impossible to reach unless they are supposed to be infinite. In the event that CFE_ES_Global.CDSVars.MaxNumRegEntries is 0, it should exit immediately.

Code snips

} while ( (RegIndx == CFE_ES_CDS_NOT_FOUND) && (i < (CFE_ES_Global.CDSVars.MaxNumRegEntries-1)) );

It is generally a bad idea to do any sort of relational comparison (greater than/less than) between signed and unsigned types, C++ actually errors about this but C does not.

System observed on:
Ubuntu 20.04

Additional context
This variable is initialized in FSW from the config CFE_PLATFORM_ES_CDS_MAX_NUM_ENTRIES which does say that the value needs to be at least 8. But during unit test the value can be zero.

Interestingly, CFE_ES_FindFreeCDSRegistryEntry swaps the weirdly-structured do-while for a normal while loop, so it is OK however it still does a signed/unsigned compare which should be fixed.

Reporter Info
Joseph Hickey, Vantage Systems. Inc.

@jphickey jphickey self-assigned this Aug 20, 2020
@jphickey jphickey added the bug label Aug 20, 2020
jphickey added a commit to jphickey/cFE that referenced this issue Sep 2, 2020
The CFE_ES_FindCDSInRegistry function had an unusual loop control
structure with mixed types of signed and unsigned.

This has the possibility of being infinite if the MaxNumRegEntries
is zero due to the way the end condition is structured.

Simplify to be like other loops and use unsigned int control variable.
yammajamma added a commit that referenced this issue Sep 2, 2020
Fix #823, avoid infinite loop in CDS registry find
@astrogeco astrogeco added this to the 7.0.0 milestone Oct 1, 2020
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Projects
None yet
2 participants