From 7297687634390248421f8e7ca796bcc7016923b7 Mon Sep 17 00:00:00 2001 From: Gilles Peskine Date: Fri, 3 Apr 2026 17:39:10 +0200 Subject: [PATCH 1/8] New test helper TEST_STRSTR Assert on strstr(), and print the strings on failure. Signed-off-by: Gilles Peskine --- tests/include/test/helpers.h | 25 ++++++++++++++++++++++ tests/include/test/macros.h | 17 +++++++++++++++ tests/src/helpers.c | 41 ++++++++++++++++++++++++++++++++++++ 3 files changed, 83 insertions(+) diff --git a/tests/include/test/helpers.h b/tests/include/test/helpers.h index 0b21bdc32..88e0921b4 100644 --- a/tests/include/test/helpers.h +++ b/tests/include/test/helpers.h @@ -378,6 +378,31 @@ int mbedtls_test_le_u(const char *test, int line_no, const char *filename, int mbedtls_test_le_s(const char *test, int line_no, const char *filename, long long value1, long long value2); +/** + * \brief Record the current test case as a failure based + * on a substring search. + * + * This function is usually called via the macro + * #TEST_STRSTR. + * + * \param test Description of the failure or assertion that failed. This + * MUST be a string literal. This normally has the form + * "strstr(EXPR1, EXPR2)" where EXPR1 has the value + * \p haystack and EXPR2 has the value \p needle. + * \param line_no Line number where the failure originated. + * \param filename Filename where the failure originated. + * \param haystack The null-terminated string to look in. + * Alternatively, this can be a null pointer, + * which is treated as if it was an empty string. + * \param needle The null-terminated string to look for. + * Alternatively, this can be a null pointer, + * which is treated as if it was an empty string. + * + * \return \c 1 on success, otherwise \c 0. + */ +int mbedtls_test_strstr(const char *test, int line_no, const char *filename, + const char *haystack, const char *needle); + /** * \brief This function decodes the hexadecimal representation of * data. diff --git a/tests/include/test/macros.h b/tests/include/test/macros.h index 66c8a137a..474f3dad0 100644 --- a/tests/include/test/macros.h +++ b/tests/include/test/macros.h @@ -121,6 +121,23 @@ goto exit; \ } while (0) +/** Evaluate two string expressions and fail the test case + * if the second is not a substring of the first. + * + * Either value can be null, which is treated as if it was an empy string. + * + * \param expr1 An string-valued expression to evaluate. + * \param expr2 Another string-valued expression to evaluate. + */ +#define TEST_STRSTR(expr1, expr2) \ + do { \ + if (!mbedtls_test_strstr("strstr(" #expr1 ", " #expr2 ")", \ + __LINE__, __FILE__, \ + expr1, expr2)) { \ + goto exit; \ + } \ + } while (0) + /** Allocate memory dynamically and fail the test case if this fails. * The allocated memory will be filled with zeros. * diff --git a/tests/src/helpers.c b/tests/src/helpers.c index 2b0742052..7fbf08b14 100644 --- a/tests/src/helpers.c +++ b/tests/src/helpers.c @@ -596,6 +596,47 @@ int mbedtls_test_le_s(const char *test, int line_no, const char *filename, return 0; } +int mbedtls_test_strstr(const char *test, int line_no, const char *filename, + const char *haystack, const char *needle) +{ + if (haystack == NULL) { + haystack = ""; + } + if (needle == NULL) { + return 1; + } + if (strstr(haystack, needle)) { + return 1; + } + +#ifdef MBEDTLS_THREADING_C + mbedtls_mutex_lock(&mbedtls_test_info_mutex); +#endif /* MBEDTLS_THREADING_C */ + + /* Don't use accessor, we already hold mutex. */ + if (mbedtls_test_info.result != MBEDTLS_TEST_RESULT_FAILED) { + /* If we've already recorded the test as having failed then don't + * overwrite any previous information about the failure. */ + + char buf[MBEDTLS_TEST_LINE_LENGTH]; + mbedtls_test_fail_internal(test, line_no, filename); + (void) mbedtls_snprintf(buf, sizeof(buf), + "haystack= \"%s\"", + haystack); + mbedtls_test_set_line1_internal(buf); + (void) mbedtls_snprintf(buf, sizeof(buf), + "needle = \"%s\"", + needle); + mbedtls_test_set_line2_internal(buf); + } + +#ifdef MBEDTLS_THREADING_C + mbedtls_mutex_unlock(&mbedtls_test_info_mutex); +#endif /* MBEDTLS_THREADING_C */ + + return 0; +} + void mbedtls_test_fail_errno(const char *test, int line_no, const char *filename) { From 1cf84c68e07d56f4076fdfa2cf5fededdfc609c0 Mon Sep 17 00:00:00 2001 From: Gilles Peskine Date: Fri, 3 Apr 2026 18:17:11 +0200 Subject: [PATCH 2/8] Note about test info requiring string literals in practice Signed-off-by: Gilles Peskine --- tests/include/test/helpers.h | 14 ++++++++++++++ 1 file changed, 14 insertions(+) diff --git a/tests/include/test/helpers.h b/tests/include/test/helpers.h index 88e0921b4..b9835237f 100644 --- a/tests/include/test/helpers.h +++ b/tests/include/test/helpers.h @@ -150,6 +150,13 @@ void mbedtls_test_get_line2(char *line); /** * \brief Get a copy of the test result information. * + * \note This is a shallow copy: in places where the test info structure + * contains a pointer, the pointer is copied. The test framework + * requires these strings to be valid for the duration of the + * test case (including after the test function returns), and does + * not provide any opportunity to deallocate them, so in practice + * they are string literals. + * * \param[out] out On output, contains a copy of the current test info. */ void mbedtls_test_info_save(mbedtls_test_info_t *out); @@ -159,6 +166,13 @@ void mbedtls_test_info_save(mbedtls_test_info_t *out); * This is intended for some unusual scenarios. * You probably shouldn't use this in a test function. * + * \note This is a shallow copy: in places where the test info structure + * contains a pointer, the pointer is copied. The test framework + * requires these strings to be valid for the duration of the + * test case (including after the test function returns), and does + * not provide any opportunity to deallocate them, so in practice + * they are string literals. + * * \param[in] replacement * The test info to use instead of the current one. * The function copies the data, so the pointer does From ff7c392d8208964d2d69d036790463f76edc4edb Mon Sep 17 00:00:00 2001 From: Gilles Peskine Date: Fri, 3 Apr 2026 15:18:55 +0200 Subject: [PATCH 3/8] mbedtls_test_fork_run_child: Fix failure when the child outputs nothing Signed-off-by: Gilles Peskine --- tests/src/fork_helpers.c | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/tests/src/fork_helpers.c b/tests/src/fork_helpers.c index 92a0a328c..546ff69b5 100644 --- a/tests/src/fork_helpers.c +++ b/tests/src/fork_helpers.c @@ -97,7 +97,7 @@ static void run_child( goto write_done; } if (result_char == MBEDTLS_TEST_RESULT_SUCCESS) { - if (fwrite(buf, length, 1, file) != 1) { + if (length != 0 && fwrite(buf, length, 1, file) != 1) { goto write_done; } } else { From e1573d039a0b9ffc3ba8cfc02b3cf1fb8f1dd22d Mon Sep 17 00:00:00 2001 From: Gilles Peskine Date: Fri, 3 Apr 2026 16:34:30 +0200 Subject: [PATCH 4/8] mbedtls_test_fork_run_child: Use temporary file instead of pipe Use a temporary file instead of a pipe to communicate the test result and the output from the child to the parent. This makes the data flow and the control flow easier because the code can seek back and forth, whereas a pipe required doing things in order. In particular, there are now fewer special cases to manage, and the parent can read the data in the most convenient order. Signed-off-by: Gilles Peskine --- tests/src/fork_helpers.c | 203 +++++++++++++++++++-------------------- 1 file changed, 99 insertions(+), 104 deletions(-) diff --git a/tests/src/fork_helpers.c b/tests/src/fork_helpers.c index 546ff69b5..108e33a6a 100644 --- a/tests/src/fork_helpers.c +++ b/tests/src/fork_helpers.c @@ -16,6 +16,7 @@ #include +#include #include #include #include @@ -25,20 +26,20 @@ /** 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 child successfully reported its test result in the temporary + * file, and its output if the test passed. Alternatively, the + * child died before writing anything to the temporary file. * - * 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. + * The content of the file has the following format: + * - mbedtls_test_info_t structure; + * - on pass, the ouptut (up to the end of the file). */ CHILD_EXIT_CODE_OK = 0, - /** Something went wrong, e.g. a write error on the pipe. */ + /** Something went wrong in a way that could not be recorded + * in the temporary file. Pick a value that's nonzero (not a success), + * less than 128 (conventionally the upper bound for exit statuses), + * and unlikely to be the exit code of some other program if the + * child calls execve(). */ CHILD_EXIT_CODE_REPORTING_FAILED = 122, } child_exit_code_t; @@ -67,7 +68,7 @@ static int probably_running_under_valgrind(void) __attribute__((__noreturn__)) #endif static void run_child( - int write_fd, + FILE *file, mbedtls_test_fork_child_callback_t *child_callback, void *param, unsigned char *buf, size_t size) @@ -75,37 +76,32 @@ static void run_child( /* 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; - 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); - TEST_LE_U(length, size); - /* Label called `exit`: this is where TEST_ASSERT() and friends jump to. */ + if (mbedtls_test_get_result() == MBEDTLS_TEST_RESULT_SUCCESS && length != 0) { + /* Write the output. This could fail on a full disk. Remember to + * flush (otherwise the output would likely be truncated). */ + errno = 0; + size_t n = fwrite(buf, 1, length, file); + TEST_ASSERT_ERRNO(n == length); + TEST_ASSERT_ERRNO(fflush(file) == 0); + } + + /* Report our final status (covering the child callback and the output + * reporting). The file is already large enough, so a failure at this + * stage is very unlikely. */ exit: - ; // label followed by a declaration is not portable C - char result_char = mbedtls_test_get_result(); - if (fputc(result_char, file) == EOF) { + /* can't put a label on a declaration */; + mbedtls_test_info_t test_info; + mbedtls_test_info_save(&test_info); + + if (fseek(file, 0, SEEK_SET) != 0) { goto write_done; } - if (result_char == MBEDTLS_TEST_RESULT_SUCCESS) { - if (length != 0 && 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 (fwrite(&test_info, sizeof(test_info), 1, file) != 1) { + goto write_done; } if (fflush(file) != 0) { goto write_done; @@ -155,90 +151,89 @@ int mbedtls_test_fork_run_child( { *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); + FILE *file = NULL; + /* Create a temporary file where the child can write the test result + * and its output. We unlink the file as soon as we've created it, + * but we keep it open. This way, both the parent and the child process + * can keep using the file until they close it, but the file will + * be removed when both processes have closed it. + */ + /* Make up a unique name using the current (parent) process ID and + * a stack address inside that process. */ + char filename[sizeof("mbedtls_test_fork_run_child-%ld-%p.tmp") + + 3 * sizeof(long) + 2 * sizeof(void *)]; + mbedtls_snprintf(filename, sizeof(filename), + "mbedtls_test_fork_run_child-%ld-%p.tmp", + (long) getpid(), (void *) filename); + file = fopen(filename, "w+"); + TEST_ASSERT_ERRNO(file != NULL); + unlink(filename); + + /* The temporary file will contain the test result from the child, + * followed by the output from the child callback. + * + * Pre-fill a failure result. + * The file position is at the beginning of the expected output. + */ + mbedtls_test_info_t child_test_info; + memset(&child_test_info, 0, sizeof(child_test_info)); + child_test_info.result = MBEDTLS_TEST_RESULT_FAILED; + child_test_info.test = "Child died without reporting status"; + child_test_info.step = -1; + child_test_info.filename = __FILE__; + child_test_info.line_no = __LINE__; + TEST_ASSERT_ERRNO(fwrite(&child_test_info, sizeof(child_test_info), 1, file) > 0); + TEST_ASSERT_ERRNO(fflush(file) == 0); + + /* Fork the child */ 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, + run_child(file, 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; - 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); - 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 bytes_read = 0; - if (result_char == MBEDTLS_TEST_RESULT_SUCCESS) { - do { - n = read(pipe_fd[0], - child_output + bytes_read, - child_output_size - bytes_read); - if (n > 0) { - bytes_read += n; - } - } 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 + bytes_read, - sizeof(reading_on_failure) - bytes_read); - if (n > 0) { - bytes_read += n; - } - } 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(bytes_read, sizeof(reading_on_failure.child_test_info)); - } - - /* 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; - + /* Reap the child */ 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 = bytes_read; - ret = 0; - } else { - mbedtls_test_info_overwrite(&reading_on_failure.child_test_info); - } - } else { - /* Weird status, just report it. */ - TEST_EQUAL(wstatus, 0); + TEST_EQUAL(wstatus, CHILD_EXIT_CODE_OK); + + /* The child exited normally. Obtain the test result from the child. */ + TEST_ASSERT_ERRNO(fseek(file, 0, SEEK_SET) == 0); + TEST_ASSERT_ERRNO(fread(&child_test_info, 1, sizeof(child_test_info), file) > 0); + + if (child_test_info.result != MBEDTLS_TEST_RESULT_SUCCESS) { + /* Skip or failure in the child. Transfer the child's test + * result into the parent. */ + mbedtls_test_info_overwrite(&child_test_info); + goto exit; } + /* The child succeeded. Read the output. */ + TEST_ASSERT_ERRNO(fseek(file, 0, SEEK_END) == 0); + long pos = ftell(file); + TEST_ASSERT_ERRNO(pos >= 0); + TEST_LE_U(sizeof(child_test_info), pos); + size_t len = pos - sizeof(child_test_info); + TEST_LE_U(len, child_output_size); + TEST_ASSERT_ERRNO(fseek(file, sizeof(child_test_info), SEEK_SET) == 0); + TEST_EQUAL(fread(child_output, 1, len, file), len); + *child_output_length = len; + + fclose(file); + return 0; + exit: - close(pipe_fd[0]); - close(pipe_fd[1]); - return ret; + if (file != NULL) { + fclose(file); + } + mbedtls_test_fork_child_fd = -1; + return -1; } #endif /* MBEDTLS_PLATFORM_IS_UNIXLIKE */ From e713bae1ff7aae8c71d328d383ead3234ff614c2 Mon Sep 17 00:00:00 2001 From: Gilles Peskine Date: Fri, 3 Apr 2026 17:54:09 +0200 Subject: [PATCH 5/8] mbedtls_test_fork_run_child: Expose file to invasive metatest Signed-off-by: Gilles Peskine --- tests/src/fork_helpers.c | 6 ++++++ 1 file changed, 6 insertions(+) diff --git a/tests/src/fork_helpers.c b/tests/src/fork_helpers.c index 108e33a6a..d131e9591 100644 --- a/tests/src/fork_helpers.c +++ b/tests/src/fork_helpers.c @@ -143,6 +143,10 @@ static void run_child( _exit(child_exit_code); } +/* Do not use this variable unless you are doing debugging or fault injection + * in a metatest of mbedtls_test_fork_run_child() */ +int mbedtls_test_fork_child_fd = -1; + int mbedtls_test_fork_run_child( mbedtls_test_fork_child_callback_t *child_callback, void *param, @@ -169,6 +173,7 @@ int mbedtls_test_fork_run_child( (long) getpid(), (void *) filename); file = fopen(filename, "w+"); TEST_ASSERT_ERRNO(file != NULL); + mbedtls_test_fork_child_fd = fileno(file); unlink(filename); /* The temporary file will contain the test result from the child, @@ -226,6 +231,7 @@ int mbedtls_test_fork_run_child( *child_output_length = len; fclose(file); + mbedtls_test_fork_child_fd = -1; return 0; exit: From cd6fb181a2d1ec0718958ce3a675aae3cf4216df Mon Sep 17 00:00:00 2001 From: Gilles Peskine Date: Sat, 4 Apr 2026 19:50:28 +0200 Subject: [PATCH 6/8] Validate output length in the child If the child reports an output length that's larger than the buffer, report it rather than overread the buffer. The parent would catch the excess length anyway, but having the child catch it makes the behavior more uniform with respect to the presence or absence of sanitizers. Signed-off-by: Gilles Peskine --- tests/src/fork_helpers.c | 2 ++ 1 file changed, 2 insertions(+) diff --git a/tests/src/fork_helpers.c b/tests/src/fork_helpers.c index d131e9591..ea4db22fb 100644 --- a/tests/src/fork_helpers.c +++ b/tests/src/fork_helpers.c @@ -80,6 +80,8 @@ static void run_child( child_callback(param, buf, size, &length); + TEST_LE_U(length, size); + if (mbedtls_test_get_result() == MBEDTLS_TEST_RESULT_SUCCESS && length != 0) { /* Write the output. This could fail on a full disk. Remember to * flush (otherwise the output would likely be truncated). */ From 7cd8a874be0cb240b78ad80582bdf6e4bedfde62 Mon Sep 17 00:00:00 2001 From: Gilles Peskine Date: Wed, 8 Apr 2026 16:19:32 +0200 Subject: [PATCH 7/8] Improve robustness against read errors from the temporary file In the unlikely event of a corrupted file or I/O error, don't accept a partially written status. Signed-off-by: Gilles Peskine --- tests/src/fork_helpers.c | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/tests/src/fork_helpers.c b/tests/src/fork_helpers.c index ea4db22fb..32e35b7d5 100644 --- a/tests/src/fork_helpers.c +++ b/tests/src/fork_helpers.c @@ -212,7 +212,7 @@ int mbedtls_test_fork_run_child( /* The child exited normally. Obtain the test result from the child. */ TEST_ASSERT_ERRNO(fseek(file, 0, SEEK_SET) == 0); - TEST_ASSERT_ERRNO(fread(&child_test_info, 1, sizeof(child_test_info), file) > 0); + TEST_ASSERT_ERRNO(fread(&child_test_info, sizeof(child_test_info), 1, file) == 1); if (child_test_info.result != MBEDTLS_TEST_RESULT_SUCCESS) { /* Skip or failure in the child. Transfer the child's test From f61edfd639390fea15756361d83466078ee000e8 Mon Sep 17 00:00:00 2001 From: Gilles Peskine Date: Wed, 8 Apr 2026 16:20:49 +0200 Subject: [PATCH 8/8] Slightly simplify reading the child output Rather than seek forth and back, read up to the buffer size, and then check that the file wasn't larger than the buffer size. Signed-off-by: Gilles Peskine --- tests/src/fork_helpers.c | 15 +++++++-------- 1 file changed, 7 insertions(+), 8 deletions(-) diff --git a/tests/src/fork_helpers.c b/tests/src/fork_helpers.c index 32e35b7d5..326255b8d 100644 --- a/tests/src/fork_helpers.c +++ b/tests/src/fork_helpers.c @@ -222,14 +222,13 @@ int mbedtls_test_fork_run_child( } /* The child succeeded. Read the output. */ - TEST_ASSERT_ERRNO(fseek(file, 0, SEEK_END) == 0); - long pos = ftell(file); - TEST_ASSERT_ERRNO(pos >= 0); - TEST_LE_U(sizeof(child_test_info), pos); - size_t len = pos - sizeof(child_test_info); - TEST_LE_U(len, child_output_size); - TEST_ASSERT_ERRNO(fseek(file, sizeof(child_test_info), SEEK_SET) == 0); - TEST_EQUAL(fread(child_output, 1, len, file), len); + size_t len = fread(child_output, 1, child_output_size, file); + /* Error out on read error */ + TEST_ASSERT_ERRNO(!ferror(file)); + /* Error out if the child wrote more than child_output_size bytes */ + int c = getc(file); + TEST_ASSERT(c == -1); + /* All good! */ *child_output_length = len; fclose(file);