Skip to content

Fix Year 2038 Problem in Sntp_ConvertToUnixTime #104

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

Merged
merged 20 commits into from
Apr 15, 2025
Merged
Show file tree
Hide file tree
Changes from 19 commits
Commits
File filter

Filter by extension

Filter by extension


Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
10 changes: 5 additions & 5 deletions .github/workflows/ci.yml
Original file line number Diff line number Diff line change
Expand Up @@ -79,8 +79,8 @@ jobs:

- name: Build
run: |
sudo apt-get install -y lcov sed
# Build with logging enabled.
sudo apt-get install -y lcov
cmake -S test -B build/ \
-G "Unix Makefiles" \
-DCMAKE_BUILD_TYPE=Debug \
Expand All @@ -97,9 +97,9 @@ jobs:
- name: Run Coverage
run: |
make -C build/ coverage
declare -a EXCLUDE=("\*test\*" "\*CMakeCCompilerId\*" "\*mocks\*")
echo ${EXCLUDE[@]} | xargs lcov --rc lcov_branch_coverage=1 -r build/coverage.info -o build/coverage.info
lcov --rc lcov_branch_coverage=1 --list build/coverage.info
declare -a EXCLUDE=("\*test\*" "\*CMakeCCompilerId.c")
echo ${EXCLUDE[@]} | xargs lcov --rc branch_coverage=1 --ignore-errors empty --ignore-errors source -r build/coverage.info -o build/coverage.info
lcov --rc branch_coverage=1 --ignore-errors empty --ignore-errors source --list build/coverage.info

- name: Check Coverage
uses: FreeRTOS/CI-CD-Github-Actions/coverage-cop@main
Expand Down Expand Up @@ -198,7 +198,7 @@ jobs:
- name: Install Python3
uses: actions/setup-python@v3
with:
python-version: "3.7.x"
python-version: "3.8"
- name: Measure sizes
uses: FreeRTOS/CI-CD-Github-Actions/memory_statistics@main
with:
Expand Down
2 changes: 1 addition & 1 deletion docs/doxygen/code_examples/example_sntp_client_posix.c
Original file line number Diff line number Diff line change
Expand Up @@ -139,7 +139,7 @@ static void sntpClient_SetTime( const SntpServerInfo_t * pTimeServer,
SntpLeapSecondInfo_t leapSecondInfo )
{
/* @[code_example_sntp_converttounixtime] */
uint32_t unixSecs;
UnixTime_t unixSecs;
uint32_t unixMs;
SntpStatus_t status = Sntp_ConvertToUnixTime( pServerTime, &unixSecs, &unixMs );

Expand Down
4 changes: 2 additions & 2 deletions docs/doxygen/include/size_table.md
Original file line number Diff line number Diff line change
Expand Up @@ -10,7 +10,7 @@
<tr>
<td>core_sntp_client.c</td>
<td><center>1.7K</center></td>
<td><center>1.4K</center></td>
<td><center>1.3K</center></td>
</tr>
<tr>
<td>core_sntp_serializer.c</td>
Expand All @@ -20,6 +20,6 @@
<tr>
<td><b>Total estimates</b></td>
<td><b><center>2.7K</center></b></td>
<td><b><center>2.2K</center></b></td>
<td><b><center>2.1K</center></b></td>
</tr>
</table>
4 changes: 0 additions & 4 deletions source/core_sntp_client.c
Original file line number Diff line number Diff line change
Expand Up @@ -959,10 +959,6 @@ const char * Sntp_StatusToStr( SntpStatus_t status )
pString = "SntpZeroPollInterval";
break;

case SntpErrorTimeNotSupported:
pString = "SntpErrorTimeNotSupported";
break;

case SntpErrorDnsFailure:
pString = "SntpErrorDnsFailure";
break;
Expand Down
16 changes: 5 additions & 11 deletions source/core_sntp_serializer.c
Original file line number Diff line number Diff line change
Expand Up @@ -812,7 +812,7 @@ SntpStatus_t Sntp_CalculatePollInterval( uint16_t clockFreqTolerance,
}

SntpStatus_t Sntp_ConvertToUnixTime( const SntpTimestamp_t * pSntpTime,
uint32_t * pUnixTimeSecs,
UnixTime_t * pUnixTimeSecs,
uint32_t * pUnixTimeMicrosecs )
{
SntpStatus_t status = SntpSuccess;
Expand All @@ -821,13 +821,6 @@ SntpStatus_t Sntp_ConvertToUnixTime( const SntpTimestamp_t * pSntpTime,
{
status = SntpErrorBadParameter;
}
/* Check if passed time does not lie in the [UNIX epoch in 1970, UNIX time overflow in 2038] time range. */
else if( ( pSntpTime->seconds > SNTP_TIME_AT_LARGEST_UNIX_TIME_SECS ) &&
( pSntpTime->seconds < SNTP_TIME_AT_UNIX_EPOCH_SECS ) )
{
/* The SNTP timestamp is outside the supported time range for conversion. */
status = SntpErrorTimeNotSupported;
}
else
{
/* Handle case when timestamp represents date in SNTP era 1
Expand All @@ -839,12 +832,13 @@ SntpStatus_t Sntp_ConvertToUnixTime( const SntpTimestamp_t * pSntpTime,
* +
* Sntp Time since Era 1 Epoch
*/
*pUnixTimeSecs = UNIX_TIME_SECS_AT_SNTP_ERA_1_SMALLEST_TIME + pSntpTime->seconds;
*pUnixTimeSecs = ( UnixTime_t ) ( UNIX_TIME_SECS_AT_SNTP_ERA_1_SMALLEST_TIME + pSntpTime->seconds );
Copy link
Member

@tony-josi-aws tony-josi-aws Apr 14, 2025

Choose a reason for hiding this comment

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

Shouldn't we also update the seconds field of SntpTimestamp_t or cast it to UnixTime_t (if seconds with uint32_t itself doesn't overflow) before the addition takes place in order to make this change effective? Otherwise, I believe the addition should be happening as uint32_t and the overflow must have happened before the UnixTime_t cast of the result.

Copy link
Member Author

Choose a reason for hiding this comment

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

updated.

}

/* Handle case when SNTP timestamp is in SNTP era 1 time range. */
else
if( pSntpTime->seconds >= SNTP_TIME_AT_UNIX_EPOCH_SECS )
{
*pUnixTimeSecs = pSntpTime->seconds - SNTP_TIME_AT_UNIX_EPOCH_SECS;
*pUnixTimeSecs = ( UnixTime_t ) ( pSntpTime->seconds - SNTP_TIME_AT_UNIX_EPOCH_SECS );
Copy link
Member

Choose a reason for hiding this comment

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

Similar to above comment.

Copy link
Member Author

Choose a reason for hiding this comment

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

updated.

}

/* Convert SNTP fractions to microseconds for UNIX time. */
Expand Down
46 changes: 27 additions & 19 deletions source/include/core_sntp_serializer.h
Original file line number Diff line number Diff line change
Expand Up @@ -43,6 +43,20 @@
#endif
/* *INDENT-ON* */

/**
* @brief Type representing seconds since Unix epoch (January 1, 1970 UTC).
*
* The width of this type depends on the configuration macro USE_LEGACY_TIME_API:
* - If USE_LEGACY_TIME_API is defined, a 32-bit unsigned integer is used.
* This limits date representation to the year 2038 (Y2038 limitation).
* - Otherwise, a 64-bit unsigned integer is used for Y2038 compliance.
*/
#ifdef USE_LEGACY_TIME_API
typedef uint32_t UnixTime_t; /**< 32-bit Unix time for legacy systems. */
#else
typedef uint64_t UnixTime_t; /**< 64-bit Unix time for Y2038 compliance. */
#endif

/**
* @ingroup sntp_constants
* @brief The base packet size of request and response of the (S)NTP protocol.
Expand Down Expand Up @@ -189,12 +203,6 @@ typedef enum SntpStatus
*/
SntpZeroPollInterval,

/**
* @brief SNTP timestamp cannot be converted to UNIX time as time does not lie
* in time range supported by Sntp_ConvertToUnixTime.
*/
SntpErrorTimeNotSupported,

/**
* @brief The user-defined DNS resolution interface, @ref SntpResolveDns_t, failed to resolve
* address for a time server. This status is returned by the @ref Sntp_SendTimeRequest API.
Expand Down Expand Up @@ -496,17 +504,18 @@ SntpStatus_t Sntp_CalculatePollInterval( uint16_t clockFreqTolerance,
* @brief Utility to convert SNTP timestamp (that uses 1st Jan 1900 as the epoch) to
* UNIX timestamp (that uses 1st Jan 1970 as the epoch).
*
* @note This function can ONLY handle conversions of SNTP timestamps that lie in the
* range from 1st Jan 1970 0h 0m 0s, the UNIX epoch time, to 19th Jan 2038 3h 14m 7s,
* the maximum UNIX time that can be represented in a signed 32 bit integer. (The
* limitation is to support systems that use signed 32-bit integer to represent the
* seconds part of the UNIX time.)
* @note This function converts SNTP timestamps to UNIX time supporting both 32-bit and
* 64-bit representations based on the configuration macro USE_LEGACY_TIME_API.
*
* - If USE_LEGACY_TIME_API is defined, the conversion is limited to the date range
* from 1st Jan 1970 0h 0m 0s (UNIX epoch) to 19th Jan 2038 3h 14m 7s, due to the
* 32-bit width limitation.
*
* @note This function supports overflow of the SNTP timestamp (from the 7 Feb 2036
* 6h 28m 16s time, i.e. SNTP era 1) by treating the timestamps with seconds part
* in the range [0, 61,505,152] seconds where the upper limit represents the UNIX
* overflow time (i.e. 19 Jan 2038 3h 14m 7s) for systems that use signed 32-bit
* integer to represent time.
* - If USE_LEGACY_TIME_API is not defined, 64-bit UNIX time representation is used,
* allowing conversion of SNTP timestamps beyond the year 2038 (Y2038 problem mitigated).
*
* @note The function also correctly handles SNTP era overflow (from 7 Feb 2036 6h 28m 16s,
* i.e., SNTP era 1) to ensure accurate conversion across SNTP eras.
*
* @param[in] pSntpTime The SNTP timestamp to convert to UNIX time.
* @param[out] pUnixTimeSecs This will be filled with the seconds part of the
Expand All @@ -517,15 +526,14 @@ SntpStatus_t Sntp_CalculatePollInterval( uint16_t clockFreqTolerance,
* @return Returns one of the following:
* - #SntpSuccess if conversion to UNIX time is successful
* - #SntpErrorBadParameter if any of the passed parameters are NULL.
* - #SntpErrorTimeNotSupported if the passed SNTP time does not lie in the
* supported time range.
*/
/* @[define_sntp_converttounixtime] */
SntpStatus_t Sntp_ConvertToUnixTime( const SntpTimestamp_t * pSntpTime,
uint32_t * pUnixTimeSecs,
UnixTime_t * pUnixTimeSecs,
uint32_t * pUnixTimeMicrosecs );
/* @[define_sntp_converttounixtime] */


/* *INDENT-OFF* */
#ifdef __cplusplus
}
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -32,15 +32,15 @@
void harness()
{
SntpTimestamp_t * pSntpTime;
uint32_t * pUnixTimeSecs;
UnixTime_t * pUnixTimeSecs;
uint32_t * pUnixTimeMicrosecs;
SntpStatus_t sntpStatus;

pSntpTime = malloc( sizeof( SntpTimestamp_t ) );
pUnixTimeSecs = malloc( sizeof( uint32_t ) );
pUnixTimeSecs = malloc( sizeof( UnixTime_t ) );
pUnixTimeMicrosecs = malloc( sizeof( uint32_t ) );

sntpStatus = Sntp_ConvertToUnixTime( pSntpTime, pUnixTimeSecs, pUnixTimeMicrosecs );

__CPROVER_assert( ( sntpStatus == SntpErrorBadParameter || sntpStatus == SntpErrorTimeNotSupported || sntpStatus == SntpSuccess ), "The return value is not a valid SNTP Status" );
__CPROVER_assert( ( sntpStatus == SntpErrorBadParameter || sntpStatus == SntpSuccess ), "The return value is not a valid SNTP Status" );
}
1 change: 0 additions & 1 deletion test/unit-test/core_sntp_client_utest.c
Original file line number Diff line number Diff line change
Expand Up @@ -1374,7 +1374,6 @@ void test_StatusToStr( void )
TEST_ASSERT_EQUAL_STRING( "SntpErrorBufferTooSmall", Sntp_StatusToStr( SntpErrorBufferTooSmall ) );
TEST_ASSERT_EQUAL_STRING( "SntpInvalidResponse", Sntp_StatusToStr( SntpInvalidResponse ) );
TEST_ASSERT_EQUAL_STRING( "SntpZeroPollInterval", Sntp_StatusToStr( SntpZeroPollInterval ) );
TEST_ASSERT_EQUAL_STRING( "SntpErrorTimeNotSupported", Sntp_StatusToStr( SntpErrorTimeNotSupported ) );
TEST_ASSERT_EQUAL_STRING( "SntpErrorDnsFailure", Sntp_StatusToStr( SntpErrorDnsFailure ) );
TEST_ASSERT_EQUAL_STRING( "SntpErrorNetworkFailure", Sntp_StatusToStr( SntpErrorNetworkFailure ) );
TEST_ASSERT_EQUAL_STRING( "SntpServerNotAuthenticated", Sntp_StatusToStr( SntpServerNotAuthenticated ) );
Expand Down
22 changes: 5 additions & 17 deletions test/unit-test/core_sntp_serializer_utest.c
Original file line number Diff line number Diff line change
Expand Up @@ -877,32 +877,20 @@ void test_ConvertToUnixTime_InvalidParams( void )

/* Use same memory for UNIX seconds and microseconds as we are not
* testing those values. */
uint32_t unixTime;
UnixTime_t unixTime;
uint32_t unixTimeMs;

/* Test with NULL SNTP time. */
TEST_ASSERT_EQUAL( SntpErrorBadParameter, Sntp_ConvertToUnixTime( NULL,
&unixTime,
&unixTime ) );
&unixTimeMs ) );
/* Test with NULL output parameters. */
TEST_ASSERT_EQUAL( SntpErrorBadParameter, Sntp_ConvertToUnixTime( &sntpTime,
NULL,
&unixTime ) );
&unixTimeMs ) );
TEST_ASSERT_EQUAL( SntpErrorBadParameter, Sntp_ConvertToUnixTime( &sntpTime,
&unixTime,
NULL ) );

/* Test with time before UNIX epoch or 1st Jan 1970 .*/
sntpTime.seconds = SNTP_TIME_AT_UNIX_EPOCH_SECS - 5;
TEST_ASSERT_EQUAL( SntpErrorTimeNotSupported, Sntp_ConvertToUnixTime( &sntpTime,
&unixTime,
&unixTime ) );

/* Test with timestamp that after largest UNIX time for signed 32-bit integer systems
* (i.e. after 18 Jan 2036 3:14:07) */
sntpTime.seconds = SNTP_TIME_AT_LARGEST_UNIX_TIME_SECS + 5;
TEST_ASSERT_EQUAL( SntpErrorTimeNotSupported, Sntp_ConvertToUnixTime( &sntpTime,
&unixTime,
&unixTime ) );
}

/**
Expand All @@ -912,7 +900,7 @@ void test_ConvertToUnixTime_InvalidParams( void )
void test_ConvertToUnixTime_Nominal( void )
{
SntpTimestamp_t sntpTime = TEST_TIMESTAMP;
uint32_t unixTimeSecs;
UnixTime_t unixTimeSecs;
uint32_t unixTimeMs;

#define TEST_SNTP_TO_UNIX_CONVERSION( sntpTimeSecs, sntpTimeFracs, \
Expand Down
20 changes: 16 additions & 4 deletions tools/cmock/coverage.cmake
Original file line number Diff line number Diff line change
Expand Up @@ -14,9 +14,12 @@ execute_process( COMMAND lcov --directory ${CMAKE_BINARY_DIR}
--base-directory ${CMAKE_BINARY_DIR}
--initial
--capture
--rc lcov_branch_coverage=1
--rc branch_coverage=1
--ignore-errors empty
--ignore-errors source
--rc genhtml_branch_coverage=1
--output-file=${CMAKE_BINARY_DIR}/base_coverage.info
--quiet
)
file(GLOB files "${CMAKE_BINARY_DIR}/bin/tests/*")

Expand Down Expand Up @@ -45,11 +48,14 @@ execute_process(COMMAND ruby
# capture data after running the tests
execute_process(
COMMAND lcov --capture
--rc lcov_branch_coverage=1
--rc branch_coverage=1
--ignore-errors empty
--ignore-errors source
--rc genhtml_branch_coverage=1
--base-directory ${CMAKE_BINARY_DIR}
--directory ${CMAKE_BINARY_DIR}
--output-file ${CMAKE_BINARY_DIR}/second_coverage.info
--quiet
)

# combile baseline results (zeros) with the one after running the tests
Expand All @@ -60,11 +66,17 @@ execute_process(
--add-tracefile ${CMAKE_BINARY_DIR}/second_coverage.info
--output-file ${CMAKE_BINARY_DIR}/coverage.info
--no-external
--rc lcov_branch_coverage=1
--rc branch_coverage=1
--ignore-errors empty
--ignore-errors source
--quiet
)
execute_process(
COMMAND genhtml --rc lcov_branch_coverage=1
COMMAND genhtml --rc branch_coverage=1
--ignore-errors empty
--ignore-errors source
--branch-coverage
--output-directory ${CMAKE_BINARY_DIR}/coverage
${CMAKE_BINARY_DIR}/coverage.info
--quiet
)