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 #1222, protect if OS_FDGetInfo called on socket #1223

Merged
merged 1 commit into from
Feb 25, 2022

Conversation

jphickey
Copy link
Contributor

Describe the contribution
Datagram sockets do not have a name_entry, it is set NULL. If the user calls OS_FDGetInfo() on this type of ID, it will
pass the first test, so this needs to be checked for non-NULL.

Fixes #1222

Testing performed
Build and sanity check OSAL
Run all tests
Confirm coverage test is exercising the new check

Expected behavior changes
Calling OS_FDGetInfo on a datagram socket returns an empty string in the Path field, it does not segfault.

System(s) tested on
Ubuntu

Contributor Info - All information REQUIRED for consideration of pull request
Joseph Hickey, Vantage Systems, Inc.

Datagram sockets do not have a name_entry, it is set NULL.
If the user calls OS_FDGetInfo() on this type of ID, it will
pass the first test, so this needs to be checked for non-NULL.
@jphickey jphickey added the CCB:Ready Pull request is ready for discussion at the Configuration Control Board (CCB) label Feb 23, 2022
Copy link
Contributor

@skliper skliper left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Strange the name wasn't used before. Approved.

@jphickey
Copy link
Contributor Author

Strange the name wasn't used before. Approved.

Yeah, I was surprised by that too. Every existing caller was passing "ABC" as the name, so the fact that the name parameter wasn't actually used wasn't noticed, until I tried to add a call where it was set NULL....

@astrogeco astrogeco added CCB:Approved Indicates code review and approval by community CCB and removed CCB:Ready Pull request is ready for discussion at the Configuration Control Board (CCB) labels Feb 23, 2022
@astrogeco
Copy link
Contributor

astrogeco commented Feb 23, 2022

CCB:2022-02-23 APPROVED

  • Open issue to name datagrams

@dmccomas
Copy link

Changes correct the issue I was having.

astrogeco added a commit that referenced this pull request Feb 25, 2022
Part of IC:Caelum+dev4, nasa/cFS#432

**Includes**

PR #1215
- Resolve UT uninitialized variable warnings

PR #1219
- Add ut_assert to doxygen and fix warnings

PR #1223
- Protect if OS_FDGetInfo called on socket
astrogeco added a commit to nasa/cFS that referenced this pull request Feb 25, 2022
**Combines**

- nasa/cFE#2061, v7.0.0-rc4+dev80
- nasa/osal#1226, v6.0.0-rc4+dev42
- nasa/PSP#326, v1.6.0-rc4+dev14

*Apps*
- ci_lab v2.5.0-rc4+dev10
- sample_app v1.3.0-rc4+dev9
- sch_lab v2.5.0-rc4+dev16
- to_lab v2.5.0-rc4+dev9

*Libs*
- sample_lib v1.3.0-rc4+dev9

*Tools*
- cFS-GroundSystem v3.0.0-rc4+dev12
- elf2cfetbl v3.3.0-rc4+dev11
- tblCRCTool v1.3.0-rc4+dev7

**Includes**

- nasa/cFE#2048, Add CFE_ES_CreateChildTask default handler

- nasa/osal#1215, Resolve UT uninitialized variable warnings

- nasa/osal#1219, Add ut_assert to doxygen and fix warnings

- nasa/osal#1223, protect if OS_FDGetInfo called on socket

- nasa/cFE#2056, Missing SB include for v2 msgid

- nasa/cFE#2052, Resolve doxygen doc warnings and enforce in CI

*Apply Header Guard Standard*

- nasa/PSP#322

- nasa/ci_lab#103
- nasa/to_lab#114
- nasa/sch_lab#107

- nasa/sample_lib#74

- nasa/elf2cfetbl#100
- nasa/cFS-GroundSystem#205

*Remove explicit filename doxygen comments*

- nasa/cFE#2050
- nasa/osal#1217
- nasa/PSP#324

- nasa/sample_lib#76

- nasa/tblCRCTool#62
- nasa/elf2cfetbl#102
@astrogeco astrogeco merged commit 7369007 into main Feb 25, 2022
astrogeco added a commit to nasa/cFS that referenced this pull request Feb 25, 2022
**Combines**

- nasa/cFE#2061, v7.0.0-rc4+dev80
- nasa/osal#1226, v6.0.0-rc4+dev42
- nasa/PSP#326, v1.6.0-rc4+dev14

*Apps*
- ci_lab v2.5.0-rc4+dev10
- sample_app v1.3.0-rc4+dev9
- sch_lab v2.5.0-rc4+dev16
- to_lab v2.5.0-rc4+dev9

*Libs*
- sample_lib v1.3.0-rc4+dev9

*Tools*
- cFS-GroundSystem v3.0.0-rc4+dev12
- elf2cfetbl v3.3.0-rc4+dev11
- tblCRCTool v1.3.0-rc4+dev7

**Includes**

- nasa/cFE#2048, Add CFE_ES_CreateChildTask default handler

- nasa/osal#1215, Resolve UT uninitialized variable warnings

- nasa/osal#1219, Add ut_assert to doxygen and fix warnings

- nasa/osal#1223, protect if OS_FDGetInfo called on socket

- nasa/cFE#2056, Missing SB include for v2 msgid

- nasa/cFE#2052, Resolve doxygen doc warnings and enforce in CI

*Apply Header Guard Standard*

- nasa/PSP#322

- nasa/ci_lab#103
- nasa/to_lab#114
- nasa/sch_lab#107

- nasa/sample_lib#74

- nasa/elf2cfetbl#100
- nasa/cFS-GroundSystem#205

*Remove explicit filename doxygen comments*

- nasa/cFE#2050
- nasa/osal#1217
- nasa/PSP#324

- nasa/sample_lib#76

- nasa/tblCRCTool#62
- nasa/elf2cfetbl#102
astrogeco added a commit to nasa/cFS that referenced this pull request Feb 25, 2022
**Combines**

- nasa/cFE#2061, v7.0.0-rc4+dev80
- nasa/osal#1226, v6.0.0-rc4+dev42
- nasa/PSP#326, v1.6.0-rc4+dev14

*Apps*
- ci_lab v2.5.0-rc4+dev10
- sample_app v1.3.0-rc4+dev9
- sch_lab v2.5.0-rc4+dev16
- to_lab v2.5.0-rc4+dev9

*Libs*
- sample_lib v1.3.0-rc4+dev9

*Tools*
- cFS-GroundSystem v3.0.0-rc4+dev12
- elf2cfetbl v3.3.0-rc4+dev11
- tblCRCTool v1.3.0-rc4+dev7

**Includes**

- nasa/cFE#2048, Add CFE_ES_CreateChildTask default handler

- nasa/osal#1215, Resolve UT uninitialized variable warnings

- nasa/osal#1219, Add ut_assert to doxygen and fix warnings

- nasa/osal#1223, protect if OS_FDGetInfo called on socket

- nasa/cFE#2056, Missing SB include for v2 msgid

- nasa/cFE#2052, Resolve doxygen doc warnings and enforce in CI

*Apply Header Guard Standard*

- nasa/PSP#322

- nasa/ci_lab#103
- nasa/to_lab#114
- nasa/sch_lab#107

- nasa/sample_lib#74

- nasa/elf2cfetbl#100
- nasa/cFS-GroundSystem#205

*Remove explicit filename doxygen comments*

- nasa/cFE#2050
- nasa/osal#1217
- nasa/PSP#324

- nasa/sample_lib#76

- nasa/tblCRCTool#62
- nasa/elf2cfetbl#102

Co-authored-by: Jacob Hageman   <skliper@users.noreply.github.com>
Co-authored-by: Joseph Hickey   <jphickey@users.noreply.github.com>
@jphickey jphickey deleted the fix-1222-fdgetinfo-socket branch March 4, 2022 13:41
@skliper skliper added this to the Draco milestone Mar 25, 2022
jphickey pushed a commit to jphickey/osal that referenced this pull request Aug 10, 2022
jphickey pushed a commit to jphickey/osal that referenced this pull request Aug 10, 2022
…o_long

Fix nasa#1223, nasa#1264 shorten TestRunner function name and update cfe_es_startup.scr docs
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
CCB:Approved Indicates code review and approval by community CCB draco-rc1
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Segfault when calling OS_FDGetInfo on datagram socket ID
4 participants