From 447366fa6615bb750194ba5983e5e168bae24687 Mon Sep 17 00:00:00 2001 From: Joseph Hickey Date: Fri, 12 Mar 2021 09:58:50 -0500 Subject: [PATCH] Fix #855, check against FD_SETSIZE in bsd-select The select() implementation utilizes its own set size limit that should be checked. --- src/os/portable/os-impl-bsd-select.c | 56 ++++++++++++------- .../inc/ut-adaptor-portable-posix-io.h | 1 + .../src/ut-adaptor-portable-posix-io.c | 5 ++ .../portable/src/coveragetest-bsd-select.c | 28 ++++++++++ .../ut-stubs/inc/OCS_sys_select.h | 2 + .../ut-stubs/override_inc/sys/select.h | 2 + 6 files changed, 73 insertions(+), 21 deletions(-) diff --git a/src/os/portable/os-impl-bsd-select.c b/src/os/portable/os-impl-bsd-select.c index dbfb19622..666262155 100644 --- a/src/os/portable/os-impl-bsd-select.c +++ b/src/os/portable/os-impl-bsd-select.c @@ -72,16 +72,16 @@ * * returns: Highest numbered file descriptor in the output fd_set *-----------------------------------------------------------------*/ -static int OS_FdSet_ConvertIn_Impl(fd_set *os_set, OS_FdSet *OSAL_set) +static int32 OS_FdSet_ConvertIn_Impl(int *os_maxfd, fd_set *os_set, OS_FdSet *OSAL_set) { size_t offset; size_t bit; osal_index_t id; uint8 objids; int osfd; - int maxfd; + int32 status; - maxfd = -1; + status = OS_SUCCESS; for (offset = 0; offset < sizeof(OSAL_set->object_ids); ++offset) { objids = OSAL_set->object_ids[offset]; @@ -94,10 +94,18 @@ static int OS_FdSet_ConvertIn_Impl(fd_set *os_set, OS_FdSet *OSAL_set) osfd = OS_impl_filehandle_table[id].fd; if (osfd >= 0 && OS_impl_filehandle_table[id].selectable) { - FD_SET(osfd, os_set); - if (osfd > maxfd) + if (osfd >= FD_SETSIZE) + { + /* out of range of select() implementation */ + status = OS_ERR_OPERATION_NOT_SUPPORTED; + } + else { - maxfd = osfd; + FD_SET(osfd, os_set); + if (osfd > *os_maxfd) + { + *os_maxfd = osfd; + } } } } @@ -106,7 +114,7 @@ static int OS_FdSet_ConvertIn_Impl(fd_set *os_set, OS_FdSet *OSAL_set) } } - return maxfd; + return status; } /* end OS_FdSet_ConvertIn_Impl */ /*---------------------------------------------------------------- @@ -267,6 +275,12 @@ int32 OS_SelectSingle_Impl(const OS_object_token_t *token, uint32 *SelectFlags, return OS_ERR_OPERATION_NOT_SUPPORTED; } + if (impl->fd >= FD_SETSIZE) + { + /* out of range of select() implementation */ + return OS_ERR_OPERATION_NOT_SUPPORTED; + } + if (*SelectFlags != 0) { FD_ZERO(&wr_set); @@ -319,34 +333,26 @@ int32 OS_SelectMultiple_Impl(OS_FdSet *ReadSet, OS_FdSet *WriteSet, int32 msecs) { fd_set wr_set; fd_set rd_set; - int osfd; int maxfd; int32 return_code; - /* - * This return code will be used if the set(s) do not - * contain any file handles capable of select(). It - * will be overwritten with the real result of the - * select call, if selectable file handles were specified. - */ - return_code = OS_ERR_OPERATION_NOT_SUPPORTED; FD_ZERO(&rd_set); FD_ZERO(&wr_set); maxfd = -1; if (ReadSet != NULL) { - osfd = OS_FdSet_ConvertIn_Impl(&rd_set, ReadSet); - if (osfd > maxfd) + return_code = OS_FdSet_ConvertIn_Impl(&maxfd, &rd_set, ReadSet); + if (return_code != OS_SUCCESS) { - maxfd = osfd; + return return_code; } } if (WriteSet != NULL) { - osfd = OS_FdSet_ConvertIn_Impl(&wr_set, WriteSet); - if (osfd > maxfd) + return_code = OS_FdSet_ConvertIn_Impl(&maxfd, &wr_set, WriteSet); + if (return_code != OS_SUCCESS) { - maxfd = osfd; + return return_code; } } @@ -354,6 +360,14 @@ int32 OS_SelectMultiple_Impl(OS_FdSet *ReadSet, OS_FdSet *WriteSet, int32 msecs) { return_code = OS_DoSelect(maxfd, &rd_set, &wr_set, msecs); } + else + { + /* + * This return code will be used if the set(s) were + * both empty/NULL or otherwise did not contain valid filehandles. + */ + return_code = OS_ERR_INVALID_ID; + } if (return_code == OS_SUCCESS) { diff --git a/src/unit-test-coverage/portable/adaptors/inc/ut-adaptor-portable-posix-io.h b/src/unit-test-coverage/portable/adaptors/inc/ut-adaptor-portable-posix-io.h index a4cadae82..5acb4ff73 100644 --- a/src/unit-test-coverage/portable/adaptors/inc/ut-adaptor-portable-posix-io.h +++ b/src/unit-test-coverage/portable/adaptors/inc/ut-adaptor-portable-posix-io.h @@ -39,5 +39,6 @@ * *****************************************************/ void UT_PortablePosixIOTest_Set_Selectable(osal_index_t local_id, bool is_selectable); +void UT_PortablePosixIOTest_Set_FD(osal_index_t local_id, int fd); #endif /* UT_ADAPTOR_PORTABLE_POSIX_IO_H */ diff --git a/src/unit-test-coverage/portable/adaptors/src/ut-adaptor-portable-posix-io.c b/src/unit-test-coverage/portable/adaptors/src/ut-adaptor-portable-posix-io.c index dea9b84d5..feba26469 100644 --- a/src/unit-test-coverage/portable/adaptors/src/ut-adaptor-portable-posix-io.c +++ b/src/unit-test-coverage/portable/adaptors/src/ut-adaptor-portable-posix-io.c @@ -35,3 +35,8 @@ void UT_PortablePosixIOTest_Set_Selectable(osal_index_t local_id, bool is_select { OS_impl_filehandle_table[local_id].selectable = is_selectable; } + +void UT_PortablePosixIOTest_Set_FD(osal_index_t local_id, int fd) +{ + OS_impl_filehandle_table[local_id].fd = fd; +} diff --git a/src/unit-test-coverage/portable/src/coveragetest-bsd-select.c b/src/unit-test-coverage/portable/src/coveragetest-bsd-select.c index fd2f7951c..0367ffb1c 100644 --- a/src/unit-test-coverage/portable/src/coveragetest-bsd-select.c +++ b/src/unit-test-coverage/portable/src/coveragetest-bsd-select.c @@ -71,6 +71,13 @@ void Test_OS_SelectSingle_Impl(void) UT_SetDataBuffer(UT_KEY(OCS_clock_gettime), &nowtime, sizeof(nowtime), false); UT_SetDataBuffer(UT_KEY(OCS_clock_gettime), &latertime, sizeof(latertime), false); OSAPI_TEST_FUNCTION_RC(OS_SelectSingle_Impl, (&token, &SelectFlags, 2100), OS_ERROR); + + /* Test cases where the FD exceeds FD_SETSIZE */ + SelectFlags = OS_STREAM_STATE_READABLE | OS_STREAM_STATE_WRITABLE; + UT_PortablePosixIOTest_Set_FD(UT_INDEX_0, OCS_FD_SETSIZE); + UT_PortablePosixIOTest_Set_Selectable(UT_INDEX_0, true); + OSAPI_TEST_FUNCTION_RC(OS_SelectSingle_Impl, (&token, &SelectFlags, 0), OS_ERR_OPERATION_NOT_SUPPORTED); + } /* end OS_SelectSingle_Impl */ void Test_OS_SelectMultiple_Impl(void) @@ -81,6 +88,9 @@ void Test_OS_SelectMultiple_Impl(void) OS_FdSet ReadSet; OS_FdSet WriteSet; + UT_PortablePosixIOTest_Set_FD(UT_INDEX_0, 0); + UT_PortablePosixIOTest_Set_Selectable(UT_INDEX_0, true); + memset(&ReadSet, 0, sizeof(ReadSet)); memset(&WriteSet, 0xff, sizeof(WriteSet)); OSAPI_TEST_FUNCTION_RC(OS_SelectMultiple_Impl, (&ReadSet, &WriteSet, 0), OS_SUCCESS); @@ -88,6 +98,24 @@ void Test_OS_SelectMultiple_Impl(void) memset(&WriteSet, 0, sizeof(WriteSet)); UT_SetDefaultReturnValue(UT_KEY(OCS_select), 0); OSAPI_TEST_FUNCTION_RC(OS_SelectMultiple_Impl, (&ReadSet, &WriteSet, 1), OS_ERROR_TIMEOUT); + + /* Test where the FD set is empty */ + memset(&ReadSet, 0, sizeof(ReadSet)); + memset(&WriteSet, 0, sizeof(WriteSet)); + OSAPI_TEST_FUNCTION_RC(OS_SelectMultiple_Impl, (NULL, NULL, 0), OS_ERR_INVALID_ID); + + /* Test cases where the FD exceeds FD_SETSIZE in the read set */ + UT_PortablePosixIOTest_Set_FD(UT_INDEX_0, OCS_FD_SETSIZE); + UT_PortablePosixIOTest_Set_Selectable(UT_INDEX_0, true); + memset(&ReadSet, 0xff, sizeof(ReadSet)); + memset(&WriteSet, 0, sizeof(WriteSet)); + OSAPI_TEST_FUNCTION_RC(OS_SelectMultiple_Impl, (&ReadSet, &WriteSet, 0), OS_ERR_OPERATION_NOT_SUPPORTED); + + /* Test cases where the FD exceeds FD_SETSIZE in the write set */ + memset(&ReadSet, 0, sizeof(ReadSet)); + memset(&WriteSet, 0xff, sizeof(WriteSet)); + OSAPI_TEST_FUNCTION_RC(OS_SelectMultiple_Impl, (&ReadSet, &WriteSet, 0), OS_ERR_OPERATION_NOT_SUPPORTED); + } /* end OS_SelectMultiple_Impl */ /* ------------------- End of test cases --------------------------------------*/ diff --git a/src/unit-test-coverage/ut-stubs/inc/OCS_sys_select.h b/src/unit-test-coverage/ut-stubs/inc/OCS_sys_select.h index 531cad372..70d141386 100644 --- a/src/unit-test-coverage/ut-stubs/inc/OCS_sys_select.h +++ b/src/unit-test-coverage/ut-stubs/inc/OCS_sys_select.h @@ -35,6 +35,8 @@ /* constants normally defined in sys/select.h */ /* ----------------------------------------- */ +#define OCS_FD_SETSIZE 10 + /* ----------------------------------------- */ /* types normally defined in sys/select.h */ /* ----------------------------------------- */ diff --git a/src/unit-test-coverage/ut-stubs/override_inc/sys/select.h b/src/unit-test-coverage/ut-stubs/override_inc/sys/select.h index 4a2ddb46f..e1e268a88 100644 --- a/src/unit-test-coverage/ut-stubs/override_inc/sys/select.h +++ b/src/unit-test-coverage/ut-stubs/override_inc/sys/select.h @@ -50,4 +50,6 @@ #define FD_CLR OCS_FD_CLR #define FD_ZERO OCS_FD_ZERO +#define FD_SETSIZE OCS_FD_SETSIZE + #endif /* SELECT_H */