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

Fix #2529, Reduces CFE_EVS_MAX_PORT_MSG_LENGTH to prevent new line character truncation #2566

Merged
merged 1 commit into from
Jul 2, 2024

Conversation

chillfig
Copy link
Contributor

@chillfig chillfig commented May 23, 2024

Checklist (Please check before submitting)

Describe the contribution
A clear and concise description of what the contribution is.

  • Fixes Possible PortMessage buffer truncation #2529, Adds snprintf check return value and buffer to PortMessage within EVS_SendViaPorts() to ensure enough space for format string characters in the downstream EVS_OutputPort() printing.

Testing performed
Started cFS and reviewed startup messages.

Expected behavior changes
Enough space should now be allocated to PortMessage such that it is null terminated and ends with a new line character when printed out via ports.

System(s) tested on

  • OS: Ubuntu 22.04

Additional context
Alternative solutions considered

  • Enlarging CFE_EVS_MAX_PORT_MSG_LENGTH to allow for more space to PortMessage and therefore no unintended truncation
    • Thoughts: Trying not to use more resources if its not necessary
  • Reducing the size of the messages passed in by CI_LAB_APP, CI_LAB_APP, and TO_LAB_APP (VS_PktPtr->Payload.Message) as these were the messages causing truncated strings on every cFS build and run.
    • Thoughts: Would work but wouldn't prevent future app message changes that are too long from truncating

cFS_startup_cpu1_PrePR.txt

This is what the startup messages look like after including this PR.
cFS_startup_cpu1_19buf.txt

This is what the startup messages look like after including code to check for snprintf() return value.
This Note that CI_LAB_APP, CI_LAB_APP, and TO_LAB_APP all now have Truncation warnings and the message isn't concatenated to the next discrete message but ends with a *\0
cFS_startup_cpu1_PostPR.txt

Third party code
N/A

Contributor Info - All information REQUIRED for consideration of pull request
Justin Figueroa, Vantage Systems

@chillfig chillfig added bug CCB:Ready Ready for discussion at the Configuration Control Board (CCB) labels May 23, 2024
@chillfig chillfig self-assigned this May 23, 2024
@chillfig
Copy link
Contributor Author

chillfig commented May 30, 2024

CCB 5/30/2024: This is a configuration issue. Amend so that configuration parameters that work together and not add arbitrary limitations. Consider giving user control as to what CFE_EVS_MAX_PORT_MSG_LENGTH is. Consider that cfe_evs_task.h compile-time check for CFE_EVS_MAX_PORT_MSG_LENGTH looks at extra characters needed.

@chillfig chillfig force-pushed the portTruncation branch 5 times, most recently from b0d1ee1 to 9e50005 Compare June 13, 2024 15:22
@chillfig chillfig changed the title Fix #2529, Adds snprintf check return value in EVS_SendViaPorts Fix #2529, Reduces CFE_EVS_MAX_PORT_MSG_LENGTH to prevent new line character truncation Jun 14, 2024
@dzbaker dzbaker added CCB:Approved Indicates code review and approval by community CCB and removed CCB:Ready Ready for discussion at the Configuration Control Board (CCB) labels Jun 27, 2024
@chillfig
Copy link
Contributor Author

Action: Write new low priority ticket for representing the 11 bytes reduced in CFE_EVS_MAX_PORT_MSG_LENGTH with this pull request

@chillfig
Copy link
Contributor Author

Action: Write new low priority ticket for representing the 11 bytes reduced in CFE_EVS_MAX_PORT_MSG_LENGTH with this pull request

Done. #2571

dzbaker added a commit to nasa/cFS that referenced this pull request Jul 2, 2024
*Combines:*

cFE equuleus-rc1+dev137
osal equuleus-rc1+dev66
elf2cfetbl equuleus-rc1+dev56

**Includes:**

*cFS*
- #707
- #773

*cFE*
- nasa/cFE#2560
- nasa/cFE#2344
- nasa/cFE#2359
- nasa/cFE#2376
- nasa/cFE#2496
- nasa/cFE#2554
- nasa/cFE#2568
- nasa/cFE#2566

*osal*
- nasa/osal#1456
- nasa/osal#1465

*elf2cfetbl*
- nasa/elf2cfetbl#147
- nasa/elf2cfetbl#124
- nasa/elf2cfetbl#125
- nasa/elf2cfetbl#127

Co-authored by: Avi Weiss <[email protected]>
Co-authored by: Justin Figueroa <[email protected]>
Co-authored by: Joseph Hickey <[email protected]>
Co-authored by: Jacob Hageman <[email protected]>
Co-authored by: Anh Van <[email protected]>
@dzbaker dzbaker mentioned this pull request Jul 2, 2024
2 tasks
dzbaker added a commit to nasa/cFS that referenced this pull request Jul 2, 2024
*Combines:*

cFE equuleus-rc1+dev137
osal equuleus-rc1+dev66
elf2cfetbl equuleus-rc1+dev56

**Includes:**

*cFS*
- #707
- #773

*cFE*
- nasa/cFE#2560
- nasa/cFE#2344
- nasa/cFE#2359
- nasa/cFE#2376
- nasa/cFE#2496
- nasa/cFE#2554
- nasa/cFE#2568
- nasa/cFE#2566

*osal*
- nasa/osal#1456
- nasa/osal#1465

*elf2cfetbl*
- nasa/elf2cfetbl#147
- nasa/elf2cfetbl#124
- nasa/elf2cfetbl#125
- nasa/elf2cfetbl#127

Co-authored by: Avi Weiss <[email protected]>
Co-authored by: Justin Figueroa <[email protected]>
Co-authored by: Joseph Hickey <[email protected]>
Co-authored by: Jacob Hageman <[email protected]>
Co-authored by: Anh Van <[email protected]>
@dzbaker dzbaker merged commit 6b40010 into nasa:main Jul 2, 2024
21 checks passed
dzbaker added a commit to nasa/cFS that referenced this pull request Jul 2, 2024
*Combines:*

cFE equuleus-rc1+dev137
osal equuleus-rc1+dev66
elf2cfetbl equuleus-rc1+dev56

**Includes:**

*cFS*
- #707
- #773

*cFE*
- nasa/cFE#2560
- nasa/cFE#2344
- nasa/cFE#2359
- nasa/cFE#2376
- nasa/cFE#2496
- nasa/cFE#2554
- nasa/cFE#2568
- nasa/cFE#2566

*osal*
- nasa/osal#1456
- nasa/osal#1465

*elf2cfetbl*
- nasa/elf2cfetbl#147
- nasa/elf2cfetbl#124
- nasa/elf2cfetbl#125
- nasa/elf2cfetbl#127

Co-authored by: Avi Weiss <[email protected]>
Co-authored by: Justin Figueroa <[email protected]>
Co-authored by: Joseph Hickey <[email protected]>
Co-authored by: Jacob Hageman <[email protected]>
Co-authored by: Anh Van <[email protected]>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
bug CCB:Approved Indicates code review and approval by community CCB
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Possible PortMessage buffer truncation
2 participants