From dbfd1a6fa944670b6dba4b3e937b6ac61bf06996 Mon Sep 17 00:00:00 2001 From: Gilles Peskine Date: Sun, 25 Jan 2026 22:08:30 +0100 Subject: [PATCH 1/8] New test helper mbedtls_test_fork_run_child() Run some code in a child process. Propagate output from the child if the test succeeds, and propagate the test result information otherwise. Signed-off-by: Gilles Peskine --- tests/include/test/fork_helpers.h | 59 ++++++++++ tests/include/test/helpers.h | 19 ++++ tests/src/fork_helpers.c | 182 ++++++++++++++++++++++++++++++ tests/src/helpers.c | 22 ++++ 4 files changed, 282 insertions(+) create mode 100644 tests/include/test/fork_helpers.h create mode 100644 tests/src/fork_helpers.c diff --git a/tests/include/test/fork_helpers.h b/tests/include/test/fork_helpers.h new file mode 100644 index 000000000..4bbb12c88 --- /dev/null +++ b/tests/include/test/fork_helpers.h @@ -0,0 +1,59 @@ +/** Helper functions for testing with subprocesses. + */ +/* + * Copyright The Mbed TLS Contributors + * SPDX-License-Identifier: Apache-2.0 OR GPL-2.0-or-later + */ + +#ifndef TEST_FORK_HELPERS_H +#define TEST_FORK_HELPERS_H + +#include "test/helpers.h" + +/** Type of a function to run in a child process. + * + * The function can mark the test case as failed by calling + * mbedtls_test_fail(). This information will be reported to the parent. + * + * \param param Parameter passed to the callback. + * \param[out] output Buffer for data to pass to the parent. + * This data is ignored if the test case is marked + * as failed. + * \param output_size Size of \p output in bytes. + * \param[out] output_length Number of bytes written to \p output, to be + * passed to the parent. The default is \c 0. + */ +typedef void mbedtls_test_fork_child_callback_t( + void *param, + unsigned char *output, size_t output_size, size_t *output_length); + +/* Fork a child process and wait for it to collect some data. + * + * This is similar to backquotes or `$(...)` in a shell. + * + * This function blocks until the child exits. + * + * If the child marks the test as failed or skipped, the child's test + * information (test result and failure location) is propagated to the + * parent. + * + * \param child_callback Callback function to run in the child. + * \param param Parameter to pass to the callback function. + * \param[out] child_output On success, data retrieved from the child. + * Note that the data is only available if the + * child did not mark the test case as failed + * or skipped. + * \param child_output_size Size of \p child_output in bytes. + * \param[out] child_output_length On success, the number of bytes collected + * from the child in \c child_output. + * + * \return \c 0 on success. + * A nonzero value if the test case is marked as failed or skipped. + */ +int mbedtls_test_fork_run_child( + mbedtls_test_fork_child_callback_t *child_callback, + void *param, + unsigned char *child_output, size_t child_output_size, + size_t *child_output_length); + +#endif /* TEST_FORK_HELPERS_H */ diff --git a/tests/include/test/helpers.h b/tests/include/test/helpers.h index 01c47f3c3..0b21bdc32 100644 --- a/tests/include/test/helpers.h +++ b/tests/include/test/helpers.h @@ -147,6 +147,25 @@ void mbedtls_test_get_line1(char *line); */ void mbedtls_test_get_line2(char *line); +/** + * \brief Get a copy of the test result information. + * + * \param[out] out On output, contains a copy of the current test info. + */ +void mbedtls_test_info_save(mbedtls_test_info_t *out); + +/** + * \brief Overwrite the test result information. + * This is intended for some unusual scenarios. + * You probably shouldn't use this in a test function. + * + * \param[in] replacement + * The test info to use instead of the current one. + * The function copies the data, so the pointer does + * not need to be valid after this function returns. + */ +void mbedtls_test_info_overwrite(const mbedtls_test_info_t *replacement); + #if defined(MBEDTLS_TEST_MUTEX_USAGE) /** * \brief Get the current mutex usage error message diff --git a/tests/src/fork_helpers.c b/tests/src/fork_helpers.c new file mode 100644 index 000000000..4da359b74 --- /dev/null +++ b/tests/src/fork_helpers.c @@ -0,0 +1,182 @@ +/** \file fork_helpers.c + * + * \brief Helper functions for testing with subprocesses. + */ + +/* + * Copyright The Mbed TLS Contributors + * SPDX-License-Identifier: Apache-2.0 OR GPL-2.0-or-later + */ + +#include "test_common.h" +#include +#include + +#if defined(MBEDTLS_PLATFORM_IS_UNIXLIKE) + +#include + +#include +#include +#include + +/** Child exit code for mbedtls_test_fork_run_child(). + */ +typedef enum { + /** Reporting of the child output or the child test result through + * the pipe succeeded. + * + * The content sent on the pipe has the following format: + * - [1 byte] #mbedtls_test_result_t \c test_result + * - Case \c test_result: + * - If \c test_result == #MBEDTLS_TEST_RESULT_SUCCESS: + * the output from the child body function. + * - Otherwise: + * the child failure (or skip) information, a direct write of + * a #mbedtls_test_result_t structure. + */ + CHILD_EXIT_CODE_OK = 0, + /** Something went wrong, e.g. a write error on the pipe. */ + CHILD_EXIT_CODE_REPORTING_FAILED = 122, +} child_exit_code_t; + +#if defined(__GNUC__) +__attribute__((__noreturn__)) +#endif +static void run_child( + int write_fd, + mbedtls_test_fork_child_callback_t *child_callback, + void *param, + unsigned char *buf, size_t size) +{ + /* If something goes wrong while trying to report what happened + * in the child, exit with a nonzero status. */ + int child_exit_code = CHILD_EXIT_CODE_REPORTING_FAILED; + /* We'll use stdio to write to the pipe, so we don't have to + * manage EINTR and such. */ + FILE *file = fdopen(write_fd, "a"); + size_t length = 0; + + TEST_ASSERT_ERRNO(file != NULL); + + child_callback(param, buf, size, &length); + + char result_char = mbedtls_test_get_result(); + TEST_ASSERT(fputc(result_char, file) != EOF); + +exit: + if (mbedtls_test_get_result() == MBEDTLS_TEST_RESULT_SUCCESS) { + if (fwrite(buf, length, 1, file) != 1) { + goto write_done; + } + } else { + mbedtls_test_info_t test_info; + mbedtls_test_info_save(&test_info); + if (fwrite(&test_info, sizeof(test_info), 1, file) != 1) { + goto write_done; + } + } + if (fflush(file) != 0) { + goto write_done; + } + child_exit_code = CHILD_EXIT_CODE_OK; + +write_done: + _exit(child_exit_code); +} + +int mbedtls_test_fork_run_child( + mbedtls_test_fork_child_callback_t *child_callback, + void *param, + unsigned char *child_output, size_t child_output_size, + size_t *child_output_length) +{ + *child_output_length = 0; + + int ret = -1; + pid_t pid = -1; + int pipe_fd[2] = { -1, -1 }; + + /* Set up a pipe. The child will write to pipe_fd[1], and the + * parent will read from pipe_fd[0]. */ + TEST_ASSERT_ERRNO(pipe(pipe_fd) != -1); + + pid = fork(); + TEST_ASSERT_ERRNO(pid != -1); + + if (pid == 0) { + /* The child code */ + close(pipe_fd[0]); + run_child(pipe_fd[1], child_callback, param, + child_output, child_output_size); + /* Unreachable */ + } + /* Beyond this point, we're in the parent (original) process. */ + + close(pipe_fd[1]); + pipe_fd[1] = -1; + + unsigned char result_char; + mbedtls_test_info_t child_test_info; + /* Normally, the child should give us a 1-byte result, then either + * the child body's output or a test info. */ + ssize_t n = read(pipe_fd[0], &result_char, 1); + TEST_EQUAL(n, 1); + + /* Tentatively read what we were promised. Don't commit to anything + * until we have the child's exit status. */ + size_t offset = 0; + if (result_char == MBEDTLS_TEST_RESULT_SUCCESS) { + do { + n = read(pipe_fd[0], + child_output + offset, + child_output_size - offset); + if (n > 0) { + offset += n; + } + } while (n > 0 && offset < child_output_size); + TEST_ASSERT_ERRNO(n != -1); + } else { + do { + n = read(pipe_fd[0], + (unsigned char *) &child_test_info + offset, + sizeof(child_test_info) - offset); + if (n > 0) { + offset += n; + } + } while (n > 0 && offset < sizeof(child_test_info)); + TEST_ASSERT_ERRNO(n != -1); + } + /* Check that the child didn't write more than it should. */ + if (n > 0) { + unsigned char excess; + TEST_EQUAL(read(pipe_fd[0], &excess, 1), 0); + } + + /* Close the pipe. If we left it open, there could be a deadlock if the + * child tried to write more than it should, while the parent is just + * waiting for the child to exit. */ + close(pipe_fd[0]); + pipe_fd[0] = -1; + + int wstatus; + TEST_ASSERT_ERRNO(waitpid(pid, &wstatus, 0) == pid); + if (WIFEXITED(wstatus) && WEXITSTATUS(wstatus) == CHILD_EXIT_CODE_OK) { + if (result_char == MBEDTLS_TEST_RESULT_SUCCESS) { + *child_output_length = n; + ret = 0; + } else { + mbedtls_test_info_overwrite(&child_test_info); + } + } else { + /* Weird status, just report it. */ + TEST_EQUAL(wstatus, 0); + } + +exit: + close(pipe_fd[0]); + close(pipe_fd[1]); + return ret; +} + +#endif /* MBEDTLS_PLATFORM_IS_UNIXLIKE */ diff --git a/tests/src/helpers.c b/tests/src/helpers.c index 9bf9a05d5..2b0742052 100644 --- a/tests/src/helpers.c +++ b/tests/src/helpers.c @@ -460,6 +460,28 @@ void mbedtls_test_info_reset(void) #endif /* MBEDTLS_THREADING_C */ } +void mbedtls_test_info_save(mbedtls_test_info_t *out) +{ +#ifdef MBEDTLS_THREADING_C + mbedtls_mutex_lock(&mbedtls_test_info_mutex); +#endif /* MBEDTLS_THREADING_C */ + memcpy(out, &mbedtls_test_info, sizeof(mbedtls_test_info)); +#ifdef MBEDTLS_THREADING_C + mbedtls_mutex_unlock(&mbedtls_test_info_mutex); +#endif /* MBEDTLS_THREADING_C */ +} + +void mbedtls_test_info_overwrite(const mbedtls_test_info_t *replacement) +{ +#ifdef MBEDTLS_THREADING_C + mbedtls_mutex_lock(&mbedtls_test_info_mutex); +#endif /* MBEDTLS_THREADING_C */ + memcpy(&mbedtls_test_info, replacement, sizeof(mbedtls_test_info)); +#ifdef MBEDTLS_THREADING_C + mbedtls_mutex_unlock(&mbedtls_test_info_mutex); +#endif /* MBEDTLS_THREADING_C */ +} + int mbedtls_test_equal(const char *test, int line_no, const char *filename, unsigned long long value1, unsigned long long value2) { From 0384a5929a884f4469acd77adc062bbd8b9910cf Mon Sep 17 00:00:00 2001 From: Gilles Peskine Date: Mon, 9 Mar 2026 15:39:06 +0100 Subject: [PATCH 2/8] Give test code access to internal macros of the library Signed-off-by: Gilles Peskine --- tests/include/test/macros.h | 6 ------ tests/src/test_common.h | 7 +++++++ 2 files changed, 7 insertions(+), 6 deletions(-) diff --git a/tests/include/test/macros.h b/tests/include/test/macros.h index 66c8a137a..8bde989f8 100644 --- a/tests/include/test/macros.h +++ b/tests/include/test/macros.h @@ -22,12 +22,6 @@ #include "mbedtls/memory_buffer_alloc.h" #endif -#if defined(TF_PSA_CRYPTO_VERSION_NUMBER) -#include "tf_psa_crypto_common.h" -#else -#include "common.h" -#endif - /** * \brief This macro tests the expression passed to it as a test step or * individual test in a test case. diff --git a/tests/src/test_common.h b/tests/src/test_common.h index 40bdd4d65..c5cce8241 100644 --- a/tests/src/test_common.h +++ b/tests/src/test_common.h @@ -33,4 +33,11 @@ * is deemed necessary in test headers. */ #include +/* Give test code access to internal macros of the library. */ +#if defined(TF_PSA_CRYPTO_VERSION_NUMBER) +#include "tf_psa_crypto_common.h" +#else +#include "common.h" +#endif + #endif /* TEST_TEST_COMMON_H */ From 96c9dca216d4b84c5915e090e44e3a8238b3e8b1 Mon Sep 17 00:00:00 2001 From: Gilles Peskine Date: Mon, 16 Mar 2026 15:16:35 +0100 Subject: [PATCH 3/8] Fix exact-size check on failure in the child When reading data from the child, if the child reports a failure, the parent expects the child to write an `mbedtls_test_info_t` structure, no less, no more. To achieve this, we try reading at least one byte more, and check that we couldn't read more than the expected size. This commit fixes two bugs: * On success, don't require the child to fill the output buffer. This check was only intended for the failure case, but was accidentally put in the wrong place. * On failure, we weren't checking that the child had written at least the expected size, which could have been worse (we'd end up with a child_test_info structure that's only partially initialized). Signed-off-by: Gilles Peskine --- tests/src/fork_helpers.c | 20 ++++++++++---------- 1 file changed, 10 insertions(+), 10 deletions(-) diff --git a/tests/src/fork_helpers.c b/tests/src/fork_helpers.c index 4da359b74..7ede42354 100644 --- a/tests/src/fork_helpers.c +++ b/tests/src/fork_helpers.c @@ -117,7 +117,10 @@ int mbedtls_test_fork_run_child( pipe_fd[1] = -1; unsigned char result_char; - mbedtls_test_info_t child_test_info; + struct { + mbedtls_test_info_t child_test_info; + unsigned char excess; + } reading_on_failure; /* Normally, the child should give us a 1-byte result, then either * the child body's output or a test info. */ ssize_t n = read(pipe_fd[0], &result_char, 1); @@ -139,18 +142,15 @@ int mbedtls_test_fork_run_child( } else { do { n = read(pipe_fd[0], - (unsigned char *) &child_test_info + offset, - sizeof(child_test_info) - offset); + (unsigned char *) &reading_on_failure + offset, + sizeof(reading_on_failure) - offset); if (n > 0) { offset += n; } - } while (n > 0 && offset < sizeof(child_test_info)); + } while (n > 0 && offset < sizeof(reading_on_failure)); TEST_ASSERT_ERRNO(n != -1); - } - /* Check that the child didn't write more than it should. */ - if (n > 0) { - unsigned char excess; - TEST_EQUAL(read(pipe_fd[0], &excess, 1), 0); + /* Check that the child wrote the amount of data that what we expect. */ + TEST_EQUAL(offset, sizeof(reading_on_failure.child_test_info)); } /* Close the pipe. If we left it open, there could be a deadlock if the @@ -166,7 +166,7 @@ int mbedtls_test_fork_run_child( *child_output_length = n; ret = 0; } else { - mbedtls_test_info_overwrite(&child_test_info); + mbedtls_test_info_overwrite(&reading_on_failure.child_test_info); } } else { /* Weird status, just report it. */ From f7df78d3abba2ed0e81843bf44e9022ca1577967 Mon Sep 17 00:00:00 2001 From: Gilles Peskine Date: Mon, 16 Mar 2026 15:26:24 +0100 Subject: [PATCH 4/8] Fix reading of child output when it's fragmented In the success case, we were only reporting the correct data written by the child if the data was read in a single `read` call. Signed-off-by: Gilles Peskine --- tests/src/fork_helpers.c | 22 +++++++++++----------- 1 file changed, 11 insertions(+), 11 deletions(-) diff --git a/tests/src/fork_helpers.c b/tests/src/fork_helpers.c index 7ede42354..1c05838c6 100644 --- a/tests/src/fork_helpers.c +++ b/tests/src/fork_helpers.c @@ -128,29 +128,29 @@ int mbedtls_test_fork_run_child( /* Tentatively read what we were promised. Don't commit to anything * until we have the child's exit status. */ - size_t offset = 0; + size_t bytes_read = 0; if (result_char == MBEDTLS_TEST_RESULT_SUCCESS) { do { n = read(pipe_fd[0], - child_output + offset, - child_output_size - offset); + child_output + bytes_read, + child_output_size - bytes_read); if (n > 0) { - offset += n; + bytes_read += n; } - } while (n > 0 && offset < child_output_size); + } while (n > 0 && bytes_read < child_output_size); TEST_ASSERT_ERRNO(n != -1); } else { do { n = read(pipe_fd[0], - (unsigned char *) &reading_on_failure + offset, - sizeof(reading_on_failure) - offset); + (unsigned char *) &reading_on_failure + bytes_read, + sizeof(reading_on_failure) - bytes_read); if (n > 0) { - offset += n; + bytes_read += n; } - } while (n > 0 && offset < sizeof(reading_on_failure)); + } while (n > 0 && bytes_read < sizeof(reading_on_failure)); TEST_ASSERT_ERRNO(n != -1); /* Check that the child wrote the amount of data that what we expect. */ - TEST_EQUAL(offset, sizeof(reading_on_failure.child_test_info)); + TEST_EQUAL(bytes_read, sizeof(reading_on_failure.child_test_info)); } /* Close the pipe. If we left it open, there could be a deadlock if the @@ -163,7 +163,7 @@ int mbedtls_test_fork_run_child( TEST_ASSERT_ERRNO(waitpid(pid, &wstatus, 0) == pid); if (WIFEXITED(wstatus) && WEXITSTATUS(wstatus) == CHILD_EXIT_CODE_OK) { if (result_char == MBEDTLS_TEST_RESULT_SUCCESS) { - *child_output_length = n; + *child_output_length = bytes_read; ret = 0; } else { mbedtls_test_info_overwrite(&reading_on_failure.child_test_info); From f4677c89d6a4a97e79894f015272a09b79735fa0 Mon Sep 17 00:00:00 2001 From: Gilles Peskine Date: Mon, 16 Mar 2026 15:49:18 +0100 Subject: [PATCH 5/8] Fix null pointer dereference in the child if fdopen fails Signed-off-by: Gilles Peskine --- tests/src/fork_helpers.c | 6 +++++- 1 file changed, 5 insertions(+), 1 deletion(-) diff --git a/tests/src/fork_helpers.c b/tests/src/fork_helpers.c index 1c05838c6..dc4e4fea3 100644 --- a/tests/src/fork_helpers.c +++ b/tests/src/fork_helpers.c @@ -57,7 +57,11 @@ static void run_child( FILE *file = fdopen(write_fd, "a"); size_t length = 0; - TEST_ASSERT_ERRNO(file != NULL); + if (file == NULL) { + /* There's no way we can report anything other than the exit code. + * So we might as well quit without even running the child callback. */ + goto write_done; + } child_callback(param, buf, size, &length); From a55f15580f646252c56ea266c23ddeeb0094dad2 Mon Sep 17 00:00:00 2001 From: Gilles Peskine Date: Mon, 16 Mar 2026 15:49:48 +0100 Subject: [PATCH 6/8] Avoid a buffer overread if the child reports a wrong length The child code isn't supposed to cause memory corruption, but if it does, try to report a problem rather than mess up further. Adapt the code to report the failure to the parent accordingly. In particular, we need to make sure that the first byte written to the reporting pipe is the result code in all cases, so don't jump over the writing of the result code. Signed-off-by: Gilles Peskine --- tests/src/fork_helpers.c | 14 ++++++++++---- 1 file changed, 10 insertions(+), 4 deletions(-) diff --git a/tests/src/fork_helpers.c b/tests/src/fork_helpers.c index dc4e4fea3..4ef33bd1c 100644 --- a/tests/src/fork_helpers.c +++ b/tests/src/fork_helpers.c @@ -64,12 +64,16 @@ static void run_child( } child_callback(param, buf, size, &length); + TEST_LE_U(length, size); - char result_char = mbedtls_test_get_result(); - TEST_ASSERT(fputc(result_char, file) != EOF); - + /* Label called `exit`: this is where TEST_ASSERT() and friends jump to. */ exit: - if (mbedtls_test_get_result() == MBEDTLS_TEST_RESULT_SUCCESS) { + ; // label followed by a declaration is not portable C + char result_char = mbedtls_test_get_result(); + if (fputc(result_char, file) == EOF) { + goto write_done; + } + if (result_char == MBEDTLS_TEST_RESULT_SUCCESS) { if (fwrite(buf, length, 1, file) != 1) { goto write_done; } @@ -85,6 +89,8 @@ exit: } child_exit_code = CHILD_EXIT_CODE_OK; + /* Label for `_exit()` call: this is where we jump to if the failure + * reporting fails. */ write_done: _exit(child_exit_code); } From a2083218c76b0de70ba040608fcf4727f04eae82 Mon Sep 17 00:00:00 2001 From: Gilles Peskine Date: Mon, 16 Mar 2026 17:47:22 +0100 Subject: [PATCH 7/8] Keep exposing the library common.h in test/macros.h Partially revert "Give test code access to internal macros of the library". I think that it would be better not to require every user of `test/macros.h` to have access to the library source, but it's out of scope here. Signed-off-by: Gilles Peskine --- tests/include/test/macros.h | 6 ++++++ 1 file changed, 6 insertions(+) diff --git a/tests/include/test/macros.h b/tests/include/test/macros.h index 8bde989f8..66c8a137a 100644 --- a/tests/include/test/macros.h +++ b/tests/include/test/macros.h @@ -22,6 +22,12 @@ #include "mbedtls/memory_buffer_alloc.h" #endif +#if defined(TF_PSA_CRYPTO_VERSION_NUMBER) +#include "tf_psa_crypto_common.h" +#else +#include "common.h" +#endif + /** * \brief This macro tests the expression passed to it as a test step or * individual test in a test case. From 1a5bf10ca08c06be10857c224c5fd70ef38591f2 Mon Sep 17 00:00:00 2001 From: Gilles Peskine Date: Mon, 16 Mar 2026 23:45:49 +0100 Subject: [PATCH 8/8] Declare platform requirements for test helpers before including any system header If we rely on `tf_psa_crypto_common.h`, it's too late. And `common.h in 3.6 doesn't have platform requirements. Signed-off-by: Gilles Peskine --- tests/src/test_common.h | 12 ++++++++++++ 1 file changed, 12 insertions(+) diff --git a/tests/src/test_common.h b/tests/src/test_common.h index c5cce8241..7053fec64 100644 --- a/tests/src/test_common.h +++ b/tests/src/test_common.h @@ -14,6 +14,18 @@ #ifndef TEST_TEST_COMMON_H #define TEST_TEST_COMMON_H +#if !defined(_POSIX_C_SOURCE) +/* For standards-compliant access to + * clock_gettime(), gmtime_r(), ... + */ +#define _POSIX_C_SOURCE 200112L +#endif + +/* With GNU libc, define all the things, even when compiling with -pedantic. */ +#if !defined(_GNU_SOURCE) +#define _GNU_SOURCE +#endif + /* On Mingw-w64, force the use of a C99-compliant printf() and friends. * This is necessary on older versions of Mingw and/or Windows runtimes * where snprintf does not always zero-terminate the buffer, and does