From: Jakub Zelenka Date: Tue, 7 Jul 2026 11:38:16 +0000 (+0200) Subject: property: replace property_memfail with in-tree mfail tests X-Git-Url: http://git.ipfire.org/gitweb/?a=commitdiff_plain;h=76e33ae38fdafce7a8a693063dcfe6d70aa5c790;p=thirdparty%2Fopenssl.git property: replace property_memfail with in-tree mfail tests The standalone property_memfail.c program is superseded by memory-failure tests added directly to property_test.c using the MFAIL harness. They cover the same property store API surface under allocation failure injection: ossl_method_store_new, ossl_method_store_add, ossl_method_store_cache_set and the providerless ossl_method_store_cache_get lookup, plus the method == NULL cache_set branch. Unlike the old NO_CHECK-only program, the new tests run as checked mfail tests, verifying both clean error propagation and the absence of reference leaks on every failure path. The old program also relied on a stale, pre-lockless STORED_ALGORITHMS layout to poke the cache directly, which no longer matches property.c. Drop property_memfail.c along with its wiring in test/build.info and the 90-test_memfail.t recipe. Assisted-by: Claude:claude-opus-4-8 Reviewed-by: Nikola Pajkovsky Reviewed-by: Tomas Mraz MergeDate: Wed Jul 8 13:05:16 2026 (Merged from https://github.com/openssl/openssl/pull/31880) --- diff --git a/test/build.info b/test/build.info index 874786ee332..c40c14e8f14 100644 --- a/test/build.info +++ b/test/build.info @@ -82,7 +82,7 @@ IF[{- !$disabled{tests} -}] ENDIF IF[{- !$disabled{'allocfail-tests'} -}] - PROGRAMS{noinst}=handshake-memfail x509-memfail load_key_certs_crls_memfail property-memfail + PROGRAMS{noinst}=handshake-memfail x509-memfail load_key_certs_crls_memfail ENDIF IF[{- !$disabled{quic} -}] @@ -651,10 +651,6 @@ IF[{- !$disabled{tests} -}] INCLUDE[load_key_certs_crls_memfail]=.. ../include ../apps/include DEPEND[load_key_certs_crls_memfail]=libtestutil.a ../libcrypto.a ../libssl.a - SOURCE[property-memfail]=property_memfail.c - INCLUDE[property-memfail]=../include ../apps/include - DEPEND[property-memfail]=../libcrypto.a - SOURCE[ssl_handshake_rtt_test]=ssl_handshake_rtt_test.c helpers/ssltestlib.c INCLUDE[ssl_handshake_rtt_test]=../include ../apps/include .. DEPEND[ssl_handshake_rtt_test]=../libcrypto.a ../libssl.a libtestutil.a diff --git a/test/property_memfail.c b/test/property_memfail.c deleted file mode 100644 index caedc51c830..00000000000 --- a/test/property_memfail.c +++ /dev/null @@ -1,196 +0,0 @@ -/* - * Copyright 2026 The OpenSSL Project Authors. All Rights Reserved. - * - * Licensed under the Apache License 2.0 (the "License"). You may not use - * this file except in compliance with the License. You can obtain a copy - * in the file LICENSE in the source distribution or at - * https://www.openssl.org/source/license.html - */ - -#include -#include -#include - -#include -#include "internal/hashtable.h" -#include "internal/property.h" -#include "internal/refcount.h" - -#define TEST_NID 1024 - -/* - * TEST_NID maps to shard zero with the property cache's current power-of-two - * shard count, so this partial view is enough to reach the cache table. - */ -typedef struct { - void *algs; - HT *cache; -} TEST_STORED_ALGORITHMS; - -struct ossl_method_store_st { - OSSL_LIB_CTX *ctx; - TEST_STORED_ALGORITHMS *algs; - CRYPTO_RWLOCK *biglock; -}; - -typedef struct { - HT_KEY key_header; -} QUERY_KEY; - -/* - * We make our OSSL_PROVIDER for testing purposes. The property cache only - * uses the provider pointer as a key, except when tracing asks for its name. - */ -struct ossl_provider_st { - unsigned int flag_initialized : 1; - unsigned int flag_activated : 1; - CRYPTO_RWLOCK *flag_lock; - CRYPTO_REF_COUNT refcnt; - CRYPTO_RWLOCK *activatecnt_lock; - int activatecnt; - char *name; -}; - -static long alloc_count; -static long fail_at; -static int fail_enabled; -static int method_refs; - -static void *test_malloc(size_t num, const char *file, int line) -{ - (void)file; - (void)line; - - if (fail_enabled && ++alloc_count == fail_at) - return NULL; - - return malloc(num); -} - -static void *test_realloc(void *ptr, size_t num, const char *file, int line) -{ - (void)file; - (void)line; - - if (fail_enabled && ++alloc_count == fail_at) - return NULL; - - return realloc(ptr, num); -} - -static void test_free(void *ptr, const char *file, int line) -{ - (void)file; - (void)line; - - free(ptr); -} - -static int up_ref(void *p) -{ - (void)p; - - method_refs++; - return 1; -} - -static void down_ref(void *p) -{ - (void)p; - - method_refs--; -} - -static int delete_providerless_cache_entry(OSSL_METHOD_STORE *store) -{ - QUERY_KEY key; - uint8_t keybuf[sizeof(int)]; - size_t keylen = 0; - int nid = TEST_NID; - - memcpy(&keybuf[keylen], &nid, sizeof(nid)); - keylen += sizeof(nid); - HT_INIT_KEY_EXTERNAL(&key, keybuf, keylen); - - return ossl_ht_delete(store->algs->cache, TO_HT_KEY(&key)); -} - -static int property_cache_workload(int expect_success) -{ - static struct ossl_provider_st prov = { - .flag_initialized = 1, - .flag_activated = 1, - .name = "property-memfail" - }; - OSSL_METHOD_STORE *store = NULL; - int method = 1; - void *result = NULL; - int ret = 0; - - method_refs = 0; - - if ((store = ossl_method_store_new(NULL)) == NULL) - goto end; - if (!ossl_method_store_add(store, (OSSL_PROVIDER *)&prov, TEST_NID, "", - &method, up_ref, down_ref)) - goto end; - /* - * Restrict failure injection to the cache paths. Store setup exercises - * unrelated global initialization and platform lock allocation. - */ - alloc_count = 0; - fail_enabled = 1; - if (!ossl_method_store_cache_set(store, (OSSL_PROVIDER *)&prov, TEST_NID, - "", &method, up_ref, down_ref)) - goto end; - if (!delete_providerless_cache_entry(store)) - goto end; - if (!ossl_method_store_cache_get(store, NULL, TEST_NID, "", &result) - || result != &method) - goto end; - ret = 1; - -end: - fail_enabled = 0; - if (result != NULL) - down_ref(result); - ossl_method_store_free(store); - - if (method_refs != 0) { - fprintf(stderr, "method reference leak: %d\n", method_refs); - return 0; - } - - return expect_success ? ret : 1; -} - -int main(int argc, char **argv) -{ - int ret = EXIT_FAILURE; - - if (argc < 2) { - fprintf(stderr, "usage: %s count | run \n", argv[0]); - return EXIT_FAILURE; - } - - if (!CRYPTO_set_mem_functions(test_malloc, test_realloc, test_free)) { - fprintf(stderr, "failed to set memory functions\n"); - return EXIT_FAILURE; - } - - if (strcmp(argv[1], "count") == 0) { - if (property_cache_workload(1)) { - fprintf(stderr, "skip: 0 count %ld\n", alloc_count); - ret = EXIT_SUCCESS; - } - } else if (strcmp(argv[1], "run") == 0 && argc == 3) { - fail_at = strtol(argv[2], NULL, 10); - if (fail_at > 0 && property_cache_workload(0)) - ret = EXIT_SUCCESS; - } else { - fprintf(stderr, "usage: %s count | run \n", argv[0]); - } - - OPENSSL_cleanup(); - return ret; -} diff --git a/test/property_test.c b/test/property_test.c index 602651df254..ed868dbe8ca 100644 --- a/test/property_test.c +++ b/test/property_test.c @@ -747,6 +747,123 @@ err: return res; } +/* Memory-failure coverage for store creation. */ +static int test_query_store_new_mfail(void) +{ + OSSL_METHOD_STORE *store; + int rc; + + MFAIL_start(); + store = ossl_method_store_new(NULL); + MFAIL_end(); + + rc = store != NULL ? 1 : 0; + ossl_method_store_free(store); + return rc; +} + +/* Memory-failure coverage for method registration. */ +static int test_query_store_add_mfail(void) +{ + static OSSL_PROVIDER prov = { + .flag_initialized = 1, + .flag_activated = 1, + .name = "add-mfail-provider" + }; + OSSL_METHOD_STORE *store = NULL; + int refs = 0; + int rc = -1; + + if (!TEST_ptr(store = ossl_method_store_new(NULL))) + goto end; + + MFAIL_start(); + rc = ossl_method_store_add(store, &prov, 1, "", &refs, + counted_up_ref, counted_down_ref) + ? 1 + : 0; + MFAIL_end(); + +end: + ossl_method_store_free(store); + if (rc >= 0 && !TEST_int_eq(refs, 0)) + rc = -1; + return rc; +} + +/* A NULL method archives the matching entry instead of caching a new one. */ +static int test_query_cache_set_null(void) +{ + static OSSL_PROVIDER prov = { + .flag_initialized = 1, + .flag_activated = 1, + .name = "null-set-provider" + }; + OSSL_METHOD_STORE *store = NULL; + int refs = 0; + void *result = NULL; + int res = 0; + + if (!TEST_ptr(store = ossl_method_store_new(NULL)) + || !TEST_true(ossl_method_store_add(store, &prov, 1, "", &refs, + counted_up_ref, counted_down_ref)) + || !TEST_true(ossl_method_store_cache_set(store, &prov, 1, "", &refs, + counted_up_ref, counted_down_ref)) + || !TEST_true(ossl_method_store_cache_set(store, &prov, 1, "", NULL, + counted_up_ref, counted_down_ref)) + || !TEST_false(ossl_method_store_cache_get(store, &prov, 1, "", + &result))) + goto err; + + res = 1; + +err: + ossl_method_store_free(store); + if (!TEST_int_eq(refs, 0)) + res = 0; + return res; +} + +/* Memory-failure coverage for the cache set and providerless lookup. */ +static int test_query_cache_set_mfail(void) +{ + static OSSL_PROVIDER prov = { + .flag_initialized = 1, + .flag_activated = 1, + .name = "mfail-provider" + }; + OSSL_METHOD_STORE *store = NULL; + int refs = 0; + void *result = NULL; + int rc = -1; + + if (!TEST_ptr(store = ossl_method_store_new(NULL)) + || !TEST_true(ossl_method_store_add(store, &prov, 1, "", &refs, + counted_up_ref, counted_down_ref))) + goto end; + + /* Cache the method, then resolve it via the "any provider" (NULL) lookup. */ + MFAIL_start(); + rc = ossl_method_store_cache_set(store, &prov, 1, "", &refs, + counted_up_ref, counted_down_ref) + && ossl_method_store_cache_get(store, NULL, 1, "", &result) + && result == &refs + ? 1 + : 0; + MFAIL_end(); + +#ifdef OPENSSL_NO_CACHED_FETCH + if (result != NULL) + counted_down_ref(result); +#endif + +end: + ossl_method_store_free(store); + if (rc >= 0 && !TEST_int_eq(refs, 0)) + rc = -1; + return rc; +} + static int test_fips_mode(void) { int ret = 0; @@ -861,6 +978,10 @@ int setup_tests(void) ADD_TEST(test_query_cache_stochastic); ADD_TEST(test_query_cache_set_duplicate); ADD_TEST(test_query_cache_provider_order); + ADD_TEST(test_query_cache_set_null); + ADD_MFAIL_TEST(test_query_store_new_mfail); + ADD_MFAIL_TEST(test_query_store_add_mfail); + ADD_MFAIL_TEST(test_query_cache_set_mfail); ADD_TEST(test_fips_mode); ADD_ALL_TESTS(test_property_list_to_string, OSSL_NELEM(to_string_tests)); ADD_TEST(test_property_list_to_string_bounds); diff --git a/test/recipes/90-test_memfail.t b/test/recipes/90-test_memfail.t index f89b1b9e1ec..fefc2771b65 100644 --- a/test/recipes/90-test_memfail.t +++ b/test/recipes/90-test_memfail.t @@ -35,8 +35,6 @@ run(test(["x509-memfail", "count", srctop_file("test", "certs", "servercert.pem" run(test(["load_key_certs_crls_memfail", "count", srctop_file("test", "certs", "servercert.pem")], stderr => "$resultdir/load_key_certs_crls_countinfo.txt")); -run(test(["property-memfail", "count"], stderr => "$resultdir/propertycountinfo.txt")); - sub get_count_info { my ($infile) = @_; my ($skipcount, $malloccount) = (0, 0); @@ -63,10 +61,8 @@ my ($x509skipcount, $x509malloccount) = get_count_info("$resultdir/x509countinfo my ($load_key_certs_crls_skipcount, $load_key_certs_crls_malloccount) = get_count_info("$resultdir/load_key_certs_crls_countinfo.txt"); -my (undef, $propertymalloccount) = get_count_info("$resultdir/propertycountinfo.txt"); - my $total_malloccount = $hsmalloccount + $x509malloccount - + $load_key_certs_crls_malloccount + $propertymalloccount; + + $load_key_certs_crls_malloccount; plan skip_all => "could not get malloc counts (one or more count runs failed or output format changed)" if $total_malloccount == 0; @@ -101,7 +97,3 @@ run_memfail_test($hsskipcount, $hsmalloccount, ["handshake-memfail", "run", srct run_memfail_test($x509skipcount, $x509malloccount, ["x509-memfail", "run", srctop_file("test", "certs", "servercert.pem")]); run_memfail_test($load_key_certs_crls_skipcount, $load_key_certs_crls_malloccount, ["load_key_certs_crls_memfail", "run", srctop_file("test", "certs", "servercert.pem")]); - -for my $idx (1..$propertymalloccount) { - ok(run(test(["property-memfail", "run", $idx]))); -}