Fix #855, check against FD_SETSIZE in bsd-select #897

jphickey · 2021-03-12T15:13:17Z

Describe the contribution
The select() implementation utilizes its own set size limit that should be checked.

Fixes #855

Testing performed
Build and run all unit tests, confirm coverage of select is testing new branches/returns.

Expected behavior changes
Calls to OS_SelectSingle/OS_SelectMultiple will fail if an FD within the set is outside the range of the underlying FD_SETSIZE from the C library.

System(s) tested on
Ubuntu 20.04

Additional context
Unfortunately this is a limitation/problem of the select() API - valid file handles may be effectively un-selectable due simply to the fact that FD_SETSIZE may be lower than the maximum number of open file descriptors.

The only alternative is to use poll() instead which uses a different API that does not have this limitation, but has a different issue in that it requires an array of struct pollfd data structures to be allocated somehow.

However in practice it is very unlikely to hit the FD_SETSIZE limit because this is (probably) much higher than the OS_MAX_NUM_OPEN_FILES limit.

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

The select() implementation utilizes its own set size limit that should be checked.

Fix nasa#855, check against FD_SETSIZE in bsd-select The select() implementation utilizes its own set size limit that should be checked.

nasa/osal#914 - Fix #752, Utilize UTASSERT_CASETYPE_NA to report OS_ERR_NOT_IMPLEMENTED nasa/osal#898 - Fix #857, correct interval calculation in DoSelect nasa/osal#909 - Fix #862, comments describing select after connect nasa/osal#902 - Fix #858, add check for EAGAIN in addition to EINTR nasa/osal#908 - Fix #861, compile time assert for sockaddr size nasa/osal#910 - Fix #863, check/report fcntl status nasa/osal#897 - Fix #855, Add assert for FD_SET_SIZE in relation to OSAL_set nasa/osal#903 - Fix #867, better error translation for ESPIPE errno nasa/osal#840 - Fix #416, add shell functional test nasa/osal#901 - Fix #869, rename OS_U32ValueWrapper_t nasa/osal#900 - Fix #876, break up logic in return statement nasa/osal#906 - Fix #886, return moduleInfoGet error nasa/osal#907 - Fix #889, report timer_gettime error nasa/osal#899 - Fix #883, remove unreachable test nasa/osal#905 - Fix #882, make module comment same as other services

nasa/cFE#1243 v6.8.0-rc1+dev436 nasa/cFE#1225, Add coverage test fix nasa/cFE#1218, Adds a local definition of `SOFTWARE_BIG/LITTLE_BIT_ORDER` directly inside `cfe_endian.h` to provide a compatible symbol for apps that still require this. This allows CFE to build and run successfully when OSAL stops providing this in `common_types.h`. nasa/cFE#1193, Removes incorrect statements from Application Developers Guide nasa/cFE#1235, Fixes truncation handling on vsnprintf error by adding a cast to avoid implicit conversion nasa/cFE#1220, Clarify the documentation on SB MsgId regarding requirements for command and telemetry messages nasa/cFE#1230, Avoids undefined behavior and resolves static analysis warnings by casting `isspace` input to `unsigned char`. nasa/cFE#1231, Updates message module and msgid v1, `CFE_MSG_SetMsgId`, to use mask instead of cast to alter value. Resolves static analysis warning. nasa/cFE#1232, Updates `CFE_ES_FileWriteByteCntErr` to report status, not a `size_t` actual since `OS_write` returns `int32`. Use `int16` for local type from `CFE_TBL_FindTableInRegistry` since it's an index, not a status. nasa/cFE#1228, Replaces `<>` with `"` in local `#include`s nasa/cFE#1227, Adds `CONTRIBUING.md` that links to the main cFS contributing guide. nasa/PSP#273 v1.5.0-rc1+dev90 nasa/PSP#264, modular psp implementation nasa/PSP#272, Use quotes for local includes nasa/PSP#271, Add Contributing Guide nasa/osal#917 v5.1.0-rc1+dev347 nasa/osal#890, Move copyblock size to a #define and add comments nasa/osal#891, Removed rogue while loop nasa/osal#892, Scripted replacement for #include <os and #include <OSC_ matches of < and > with " nasa/osal#893, Consolidates the duplicated switch in OS_SocketOpen_Impl nasa/osal#894, Add `const` to input pointers nasa/osal#895, Removed network prototypes defined in osapi_sockets.h that are also in osapi_network.h nasa/osal#896, Removes NULL redefine from common_types.h nasa/osal#912, Add Contributing Guide nasa/osal#914, Utilize UTASSERT_CASETYPE_NA to report OS_ERR_NOT_IMPLEMENTED nasa/osal#898, correct interval calculation in DoSelect nasa/osal#909, comments describing select after connect nasa/osal#902, add check for EAGAIN in addition to EINTR nasa/osal#908, compile time assert for sockaddr size nasa/osal#910, check/report fcntl status nasa/osal#897, Add assert for FD_SET_SIZE in relation to OSAL_set nasa/osal#903, better error translation for ESPIPE errno nasa/osal#840, add shell functional test nasa/osal#901, rename OS_U32ValueWrapper_t nasa/osal#900, break up logic in return statement nasa/osal#906, return moduleInfoGet error nasa/osal#907, report timer_gettime error nasa/osal#899, remove unreachable test nasa/osal#905, make module comment same as other services nasa/osal#920 to fix test error check index inside fdset conversions nasa/osal#922, make non-selectable FD an error nasa/sample_app#137 v1.2.0-rc1+dev54 nasa/sample_app#134, Convert from <> to " for local includes nasa/sample_app#136, Added a contributing guide that links to the main cFS contributing guide. nasa/sample_app#132, Add context to the values for MsgIDs nasa/sample_lib#55 v1.2.0-rc1+dev30 nasa/sample_lib#54, Replace <> with " for local includes nasa/sample_lib#53, Adds CONTRIBUTING.md that links to the main cFS contributing guide. nasa/cFS-GroundSystem#171 v2.2.0-rc1+dev41 nasa/cFS-GroundSystem#166, Updated TBL and SB tlm for an operational TLM display nasa/cFS-GroundSystem#170, Add Contributing Guide nasa/cFS-GroundSystem#137, Create package for cfs-groundsystem

nasa/cFE#1243 v6.8.0-rc1+dev436 nasa/cFE#1225, Add coverage test fix nasa/cFE#1218, bit order macros nasa/cFE#1193, Removes incorrect statements from Application Developers Guide nasa/cFE#1235, Fixes truncation handling on vsnprintf error by adding a cast to avoid implicit conversion nasa/cFE#1220, Clarify the documentation on SB MsgId regarding requirements for command and telemetry messages nasa/cFE#1230, Cast isspace input to unsigned char to avoid undefined behavior nasa/cFE#1231, Updated message module, msgid v1 to use mask instead of cast to alter value nasa/cFE#1232, Coercion alters value caused by incorrect type - static analysis warning nasa/cFE#1228, Replaces `<>` with `"` in local `#include`s nasa/cFE#1227, Adds `CONTRIBUING.md` that links to the main cFS contributing guide. nasa/PSP#273 v1.5.0-rc1+dev90 nasa/PSP#264, modular psp implementation nasa/PSP#272, Use quotes for local includes nasa/PSP#271, Add Contributing Guide nasa/osal#917 v5.1.0-rc1+dev347 nasa/osal#890, Move copyblock size to a #define and add comments nasa/osal#891, Removed rogue while loop nasa/osal#892, Scripted replacement for #include <os and #include <OSC_ matches of < and > with " nasa/osal#893, Consolidates the duplicated switch in OS_SocketOpen_Impl nasa/osal#894, Add `const` to input pointers nasa/osal#895, Removed network prototypes defined in osapi_sockets.h that are also in osapi_network.h nasa/osal#896, Removes NULL redefine from common_types.h nasa/osal#912, Add Contributing Guide nasa/osal#914, Utilize UTASSERT_CASETYPE_NA to report OS_ERR_NOT_IMPLEMENTED nasa/osal#898, correct interval calculation in DoSelect nasa/osal#909, comments describing select after connect nasa/osal#902, add check for EAGAIN in addition to EINTR nasa/osal#908, compile time assert for sockaddr size nasa/osal#910, check/report fcntl status nasa/osal#897, Add assert for FD_SET_SIZE in relation to OSAL_set nasa/osal#903, better error translation for ESPIPE errno nasa/osal#840, add shell functional test nasa/osal#901, rename OS_U32ValueWrapper_t nasa/osal#900, break up logic in return statement nasa/osal#906, return moduleInfoGet error nasa/osal#907, report timer_gettime error nasa/osal#899, remove unreachable test nasa/osal#905, make module comment same as other services nasa/osal#920 to fix test error check index inside fdset conversions nasa/osal#922, make non-selectable FD an error nasa/sample_app#137 v1.2.0-rc1+dev54 nasa/sample_app#134, Convert from <> to " for local includes nasa/sample_app#136, Added a contributing guide that links to the main cFS contributing guide. nasa/sample_app#132, Add context to the values for MsgIDs nasa/sample_lib#55 v1.2.0-rc1+dev30 nasa/sample_lib#54, Replace <> with " for local includes nasa/sample_lib#53, Adds CONTRIBUTING.md that links to the main cFS contributing guide. nasa/cFS-GroundSystem#171 v2.2.0-rc1+dev41 nasa/cFS-GroundSystem#166, Updated TBL and SB tlm for an operational TLM display nasa/cFS-GroundSystem#170, Add Contributing Guide nasa/cFS-GroundSystem#137, Create package for cfs-groundsystem

Do not use EventCount to track whether an "unregistered" event was generated, because that by definition came from a different app than the one that "owns" that field. This just adds a separate field to track that state, so it doesn't potentially modify the counter for an unrelated app.

Fix nasa#897, EVS unregistered AppID

Fix nasa#855, check against FD_SETSIZE in bsd-select

447366f

The select() implementation utilizes its own set size limit that should be checked.

jphickey force-pushed the fix-855-fd-setsize branch from cd9393f to 447366f Compare March 12, 2021 15:15

jphickey added the CCB:Ready Pull request is ready for discussion at the Configuration Control Board (CCB) label Mar 12, 2021

jphickey requested a review from a user March 12, 2021 15:16

skliper approved these changes Mar 12, 2021

View reviewed changes

skliper added this to the 6.0.0 milestone Mar 12, 2021

astrogeco added IC:2021-03-23 and removed CCB:Ready Pull request is ready for discussion at the Configuration Control Board (CCB) labels Mar 18, 2021

astrogeco changed the base branch from main to integration-candidate March 18, 2021 21:00

astrogeco added CCB:Ready Pull request is ready for discussion at the Configuration Control Board (CCB) conflicts labels Mar 18, 2021

astrogeco mentioned this pull request Mar 18, 2021

osal Integration Candidate: 2021-03-23 #917

Merged

astrogeco merged commit cd7c3d1 into nasa:integration-candidate Mar 19, 2021

astrogeco removed CCB:Ready Pull request is ready for discussion at the Configuration Control Board (CCB) conflicts labels Mar 19, 2021

jphickey deleted the fix-855-fd-setsize branch April 28, 2021 18:58

jphickey pushed a commit to jphickey/osal that referenced this pull request Aug 10, 2022

Merge pull request nasa#898 from jphickey/fix-897-evs-unreg-appid

5a64dd7

Fix nasa#897, EVS unregistered AppID

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Fix #855, check against FD_SETSIZE in bsd-select #897

Fix #855, check against FD_SETSIZE in bsd-select #897

jphickey commented Mar 12, 2021

Fix #855, check against FD_SETSIZE in bsd-select #897

Fix #855, check against FD_SETSIZE in bsd-select #897

Conversation

jphickey commented Mar 12, 2021