[openssl] master update

Matt Caswell matt at openssl.org
Thu Nov 19 15:10:08 UTC 2020


The branch master has been updated
       via  c34063d7a1e8e3e0f760fd998366165862730bae (commit)
       via  4e08ea6f111d7bdcef0659baca700a78aa867913 (commit)
      from  5b1d94c11c680c2b9527c3da55593468bcf65efd (commit)


- Log -----------------------------------------------------------------
commit c34063d7a1e8e3e0f760fd998366165862730bae
Author: Matt Caswell <matt at openssl.org>
Date:   Fri Nov 6 12:53:01 2020 +0000

    Add a test for setting, popping and clearing error marks
    
    Reviewed-by: Paul Dale <paul.dale at oracle.com>
    (Merged from https://github.com/openssl/openssl/pull/13335)

commit 4e08ea6f111d7bdcef0659baca700a78aa867913
Author: Matt Caswell <matt at openssl.org>
Date:   Fri Nov 6 11:43:44 2020 +0000

    Allow multiple nested marks
    
    Previously we only ever allowed one mark to be set against an error in the
    statck. If we attempted to nest them, then we would end up clearing all
    the errors in the stack when we popped to the mark.
    
    Reviewed-by: Paul Dale <paul.dale at oracle.com>
    (Merged from https://github.com/openssl/openssl/pull/13335)

-----------------------------------------------------------------------

Summary of changes:
 crypto/err/err.c         |  10 ++--
 crypto/err/err_local.h   |   1 +
 include/openssl/err.h.in |   1 +
 test/errtest.c           | 119 +++++++++++++++++++++++++++++++++++++++++++++++
 4 files changed, 126 insertions(+), 5 deletions(-)

diff --git a/crypto/err/err.c b/crypto/err/err.c
index 2c8240f0ba..a66ea63adf 100644
--- a/crypto/err/err.c
+++ b/crypto/err/err.c
@@ -838,7 +838,7 @@ int ERR_set_mark(void)
 
     if (es->bottom == es->top)
         return 0;
-    es->err_flags[es->top] |= ERR_FLAG_MARK;
+    es->err_marks[es->top]++;
     return 1;
 }
 
@@ -851,14 +851,14 @@ int ERR_pop_to_mark(void)
         return 0;
 
     while (es->bottom != es->top
-           && (es->err_flags[es->top] & ERR_FLAG_MARK) == 0) {
+           && es->err_marks[es->top] == 0) {
         err_clear(es, es->top, 0);
         es->top = es->top > 0 ? es->top - 1 : ERR_NUM_ERRORS - 1;
     }
 
     if (es->bottom == es->top)
         return 0;
-    es->err_flags[es->top] &= ~ERR_FLAG_MARK;
+    es->err_marks[es->top]--;
     return 1;
 }
 
@@ -873,13 +873,13 @@ int ERR_clear_last_mark(void)
 
     top = es->top;
     while (es->bottom != top
-           && (es->err_flags[top] & ERR_FLAG_MARK) == 0) {
+           && es->err_marks[top] == 0) {
         top = top > 0 ? top - 1 : ERR_NUM_ERRORS - 1;
     }
 
     if (es->bottom == top)
         return 0;
-    es->err_flags[top] &= ~ERR_FLAG_MARK;
+    es->err_marks[top]--;
     return 1;
 }
 
diff --git a/crypto/err/err_local.h b/crypto/err/err_local.h
index 2f9caf2e0e..cad67cc476 100644
--- a/crypto/err/err_local.h
+++ b/crypto/err/err_local.h
@@ -64,6 +64,7 @@ static ossl_inline void err_set_data(ERR_STATE *es, size_t i,
 static ossl_inline void err_clear(ERR_STATE *es, size_t i, int deall)
 {
     err_clear_data(es, i, (deall));
+    es->err_marks[i] = 0;
     es->err_flags[i] = 0;
     es->err_buffer[i] = 0;
     es->err_file[i] = NULL;
diff --git a/include/openssl/err.h.in b/include/openssl/err.h.in
index 35db02fad6..1f2fde8317 100644
--- a/include/openssl/err.h.in
+++ b/include/openssl/err.h.in
@@ -56,6 +56,7 @@ extern "C" {
 #  define ERR_NUM_ERRORS  16
 struct err_state_st {
     int err_flags[ERR_NUM_ERRORS];
+    int err_marks[ERR_NUM_ERRORS];
     unsigned long err_buffer[ERR_NUM_ERRORS];
     char *err_data[ERR_NUM_ERRORS];
     size_t err_data_size[ERR_NUM_ERRORS];
diff --git a/test/errtest.c b/test/errtest.c
index 443bd0a57d..247bc546a0 100644
--- a/test/errtest.c
+++ b/test/errtest.c
@@ -148,6 +148,124 @@ static int raised_error(void)
     return 1;
 }
 
+static int test_marks(void)
+{
+    unsigned long mallocfail, shouldnot;
+
+    /* Set an initial error */
+    ERR_raise(ERR_LIB_CRYPTO, ERR_R_MALLOC_FAILURE);
+    mallocfail = ERR_peek_last_error();
+    if (!TEST_ulong_gt(mallocfail, 0))
+        return 0;
+
+    /* Setting and clearing a mark should not affect the error */
+    if (!TEST_true(ERR_set_mark())
+            || !TEST_true(ERR_pop_to_mark())
+            || !TEST_ulong_eq(mallocfail, ERR_peek_last_error())
+            || !TEST_true(ERR_set_mark())
+            || !TEST_true(ERR_clear_last_mark())
+            || !TEST_ulong_eq(mallocfail, ERR_peek_last_error()))
+        return 0;
+
+    /* Test popping errors */
+    if (!TEST_true(ERR_set_mark()))
+        return 0;
+    ERR_raise(ERR_LIB_CRYPTO, ERR_R_INTERNAL_ERROR);
+    if (!TEST_ulong_ne(mallocfail, ERR_peek_last_error())
+            || !TEST_true(ERR_pop_to_mark())
+            || !TEST_ulong_eq(mallocfail, ERR_peek_last_error()))
+        return 0;
+
+    /* Nested marks should also work */
+    if (!TEST_true(ERR_set_mark())
+            || !TEST_true(ERR_set_mark()))
+        return 0;
+    ERR_raise(ERR_LIB_CRYPTO, ERR_R_INTERNAL_ERROR);
+    if (!TEST_ulong_ne(mallocfail, ERR_peek_last_error())
+            || !TEST_true(ERR_pop_to_mark())
+            || !TEST_true(ERR_pop_to_mark())
+            || !TEST_ulong_eq(mallocfail, ERR_peek_last_error()))
+        return 0;
+
+    if (!TEST_true(ERR_set_mark()))
+        return 0;
+    ERR_raise(ERR_LIB_CRYPTO, ERR_R_SHOULD_NOT_HAVE_BEEN_CALLED);
+    shouldnot = ERR_peek_last_error();
+    if (!TEST_ulong_ne(mallocfail, shouldnot)
+            || !TEST_true(ERR_set_mark()))
+        return 0;
+    ERR_raise(ERR_LIB_CRYPTO, ERR_R_INTERNAL_ERROR);
+    if (!TEST_ulong_ne(shouldnot, ERR_peek_last_error())
+            || !TEST_true(ERR_pop_to_mark())
+            || !TEST_ulong_eq(shouldnot, ERR_peek_last_error())
+            || !TEST_true(ERR_pop_to_mark())
+            || !TEST_ulong_eq(mallocfail, ERR_peek_last_error()))
+        return 0;
+
+    /* Setting and clearing a mark should not affect the errors on the stack */
+    if (!TEST_true(ERR_set_mark()))
+        return 0;
+    ERR_raise(ERR_LIB_CRYPTO, ERR_R_SHOULD_NOT_HAVE_BEEN_CALLED);
+    if (!TEST_true(ERR_clear_last_mark())
+            || !TEST_ulong_eq(shouldnot, ERR_peek_last_error()))
+        return 0;
+
+    /*
+     * Popping where no mark has been set should pop everything - but return
+     * a failure result
+     */
+    if (!TEST_false(ERR_pop_to_mark())
+            || !TEST_ulong_eq(0, ERR_peek_last_error()))
+        return 0;
+
+    /* Clearing where there is no mark should fail */
+    ERR_raise(ERR_LIB_CRYPTO, ERR_R_MALLOC_FAILURE);
+    if (!TEST_false(ERR_clear_last_mark())
+                /* "get" the last error to remove it */
+            || !TEST_ulong_eq(mallocfail, ERR_get_error())
+            || !TEST_ulong_eq(0, ERR_peek_last_error()))
+        return 0;
+
+    /*
+     * Setting a mark where there are no errors in the stack should fail.
+     * NOTE: This is somewhat surprising behaviour but is historically how this
+     * function behaves. In practice we typically set marks without first
+     * checking whether there is anything on the stack - but we also don't
+     * tend to check the success of this function. It turns out to work anyway
+     * because although setting a mark with no errors fails, a subsequent call
+     * to ERR_pop_to_mark() or ERR_clear_last_mark() will do the right thing
+     * anyway (even though they will report a failure result).
+     */
+    if (!TEST_false(ERR_set_mark()))
+        return 0;
+
+    ERR_raise(ERR_LIB_CRYPTO, ERR_R_MALLOC_FAILURE);
+    if (!TEST_true(ERR_set_mark()))
+        return 0;
+    ERR_raise(ERR_LIB_CRYPTO, ERR_R_INTERNAL_ERROR);
+    ERR_raise(ERR_LIB_CRYPTO, ERR_R_SHOULD_NOT_HAVE_BEEN_CALLED);
+
+    /* Should be able to "pop" past 2 errors */
+    if (!TEST_true(ERR_pop_to_mark())
+            || !TEST_ulong_eq(mallocfail, ERR_peek_last_error()))
+        return 0;
+
+    if (!TEST_true(ERR_set_mark()))
+        return 0;
+    ERR_raise(ERR_LIB_CRYPTO, ERR_R_INTERNAL_ERROR);
+    ERR_raise(ERR_LIB_CRYPTO, ERR_R_SHOULD_NOT_HAVE_BEEN_CALLED);
+
+    /* Should be able to "clear" past 2 errors */
+    if (!TEST_true(ERR_clear_last_mark())
+            || !TEST_ulong_eq(shouldnot, ERR_peek_last_error()))
+        return 0;
+
+    /* Clear remaining errors from last test */
+    ERR_clear_error();
+
+    return 1;
+}
+
 int setup_tests(void)
 {
     ADD_TEST(preserves_system_error);
@@ -156,5 +274,6 @@ int setup_tests(void)
 #ifndef OPENSSL_NO_DEPRECATED_3_0
     ADD_TEST(test_print_error_format);
 #endif
+    ADD_TEST(test_marks);
     return 1;
 }


More information about the openssl-commits mailing list