]> git.ipfire.org Git - thirdparty/openssl.git/commitdiff
property: replace property_memfail with in-tree mfail tests
authorJakub Zelenka <jakub.zelenka@openssl.foundation>
Tue, 7 Jul 2026 11:38:16 +0000 (13:38 +0200)
committerTomas Mraz <tomas@openssl.foundation>
Wed, 8 Jul 2026 13:05:01 +0000 (15:05 +0200)
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 <nikolap@openssl.org>
Reviewed-by: Tomas Mraz <tomas@openssl.foundation>
MergeDate: Wed Jul  8 13:05:16 2026
(Merged from https://github.com/openssl/openssl/pull/31880)

test/build.info
test/property_memfail.c [deleted file]
test/property_test.c
test/recipes/90-test_memfail.t

index 874786ee332ef1f00e8a9521608383aa4aa82ea6..c40c14e8f14815df0915e016673f8fa9fcbe66a3 100644 (file)
@@ -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 (file)
index caedc51..0000000
+++ /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 <stdio.h>
-#include <stdlib.h>
-#include <string.h>
-
-#include <openssl/crypto.h>
-#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 <allocation-number>\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 <allocation-number>\n", argv[0]);
-    }
-
-    OPENSSL_cleanup();
-    return ret;
-}
index 602651df254bc8714fa8b7a6dd2606b95c892e87..ed868dbe8ca6138084cb2a4c4214a975a72078ae 100644 (file)
@@ -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);
index f89b1b9e1ec420fa5bfd46e62235e8b16c0fa152..fefc2771b65cc42b56f87e9cd810e2d39f98b93c 100644 (file)
@@ -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])));
-}