[openssl] master update
Richard Levitte
levitte at openssl.org
Tue Jul 30 05:08:43 UTC 2019
The branch master has been updated
via 10f8b36874fca928c3f41834babac8ee94dd3f09 (commit)
from e9a5932d04f6b7dd25b39a8ff9dc162d64a78c22 (commit)
- Log -----------------------------------------------------------------
commit 10f8b36874fca928c3f41834babac8ee94dd3f09
Author: Richard Levitte <levitte at openssl.org>
Date: Thu Jul 25 17:51:30 2019 +0200
ERR: re-use the err_data field when possible
To deallocate the err_data field and then allocating it again might be
a waste of processing, but may also be a source of errors when memory
is scarce. While we normally tolerate that, the ERR sub-system is an
exception and we need to pay closer attention to how we handle memory.
This adds a new err_data flag, ERR_TXT_IGNORE, which means that even
if there is err_data memory allocated, its contents should be ignored.
Deallocation of the err_data field is much more selective, aand should
only happen when ERR_free_state() is called.
Fixes #9458
Reviewed-by: Paul Dale <paul.dale at oracle.com>
(Merged from https://github.com/openssl/openssl/pull/9459)
-----------------------------------------------------------------------
Summary of changes:
crypto/err/err.c | 106 +++++++++++++++++++++++++++++++++++---------------
include/openssl/err.h | 1 +
2 files changed, 75 insertions(+), 32 deletions(-)
diff --git a/crypto/err/err.c b/crypto/err/err.c
index 71b1049a5d..7a35512f87 100644
--- a/crypto/err/err.c
+++ b/crypto/err/err.c
@@ -235,18 +235,34 @@ static void build_SYS_str_reasons(void)
}
#endif
-#define err_clear_data(p, i) \
- do { \
- if ((p)->err_data_flags[i] & ERR_TXT_MALLOCED) {\
- OPENSSL_free((p)->err_data[i]); \
- (p)->err_data[i] = NULL; \
- } \
- (p)->err_data_flags[i] = 0; \
+#define err_get_slot(p) \
+ do { \
+ (p)->top = ((p)->top + 1) % ERR_NUM_ERRORS; \
+ if ((p)->top == (p)->bottom) \
+ (p)->bottom = ((p)->bottom + 1) % ERR_NUM_ERRORS; \
+ } while (0)
+
+#define err_clear_data(p, i, deall) \
+ do { \
+ if ((p)->err_data_flags[i] & ERR_TXT_MALLOCED) { \
+ if (deall) { \
+ OPENSSL_free((p)->err_data[i]); \
+ (p)->err_data[i] = NULL; \
+ (p)->err_data_size[i] = 0; \
+ (p)->err_data_flags[i] = 0; \
+ } else if ((p)->err_data[i] != NULL) { \
+ (p)->err_data[i][0] = '\0'; \
+ } \
+ } else { \
+ (p)->err_data[i] = NULL; \
+ (p)->err_data_size[i] = 0; \
+ (p)->err_data_flags[i] = 0; \
+ } \
} while (0)
-#define err_clear(p, i) \
+#define err_clear(p, i, deall) \
do { \
- err_clear_data(p, i); \
+ err_clear_data((p), (i), (deall)); \
(p)->err_flags[i] = 0; \
(p)->err_buffer[i] = 0; \
(p)->err_file[i] = NULL; \
@@ -260,7 +276,7 @@ static void ERR_STATE_free(ERR_STATE *s)
if (s == NULL)
return;
for (i = 0; i < ERR_NUM_ERRORS; i++) {
- err_clear_data(s, i);
+ err_clear_data(s, i, 1);
}
OPENSSL_free(s);
}
@@ -406,14 +422,11 @@ void ERR_put_error(int lib, int func, int reason, const char *file, int line)
if (es == NULL)
return;
- es->top = (es->top + 1) % ERR_NUM_ERRORS;
- if (es->top == es->bottom)
- es->bottom = (es->bottom + 1) % ERR_NUM_ERRORS;
- es->err_flags[es->top] = 0;
+ err_get_slot(es);
+ err_clear(es, es->top, 0);
es->err_buffer[es->top] = ERR_PACK(lib, func, reason);
es->err_file[es->top] = file;
es->err_line[es->top] = line;
- err_clear_data(es, es->top);
}
void ERR_clear_error(void)
@@ -426,7 +439,7 @@ void ERR_clear_error(void)
return;
for (i = 0; i < ERR_NUM_ERRORS; i++) {
- err_clear(es, i);
+ err_clear(es, i, 0);
}
es->top = es->bottom = 0;
}
@@ -506,14 +519,14 @@ static unsigned long get_error_values(int inc, int top, const char **file,
while (es->bottom != es->top) {
if (es->err_flags[es->top] & ERR_FLAG_CLEAR) {
- err_clear(es, es->top);
+ err_clear(es, es->top, 0);
es->top = es->top > 0 ? es->top - 1 : ERR_NUM_ERRORS - 1;
continue;
}
i = (es->bottom + 1) % ERR_NUM_ERRORS;
if (es->err_flags[i] & ERR_FLAG_CLEAR) {
es->bottom = i;
- err_clear(es, es->bottom);
+ err_clear(es, es->bottom, 0);
continue;
}
break;
@@ -545,7 +558,7 @@ static unsigned long get_error_values(int inc, int top, const char **file,
if (data == NULL) {
if (inc) {
- err_clear_data(es, i);
+ err_clear_data(es, i, 0);
}
} else {
if (es->err_data[i] == NULL) {
@@ -772,7 +785,8 @@ int ERR_get_next_error_library(void)
return ret;
}
-static int err_set_error_data_int(char *data, int flags)
+static int err_set_error_data_int(char *data, size_t size, int flags,
+ int deallocate)
{
ERR_STATE *es;
int i;
@@ -783,8 +797,9 @@ static int err_set_error_data_int(char *data, int flags)
i = es->top;
- err_clear_data(es, i);
+ err_clear_data(es, es->top, deallocate);
es->err_data[i] = data;
+ es->err_data_size[i] = size;
es->err_data_flags[i] = flags;
return 1;
@@ -795,8 +810,18 @@ void ERR_set_error_data(char *data, int flags)
/*
* This function is void so we cannot propagate the error return. Since it
* is also in the public API we can't change the return type.
+ *
+ * We estimate the size of the data. If it's not flagged as allocated,
+ * then this is safe, and if it is flagged as allocated, then our size
+ * may be smaller than the actual allocation, but that doesn't matter
+ * too much, the buffer will remain untouched or will eventually be
+ * reallocated to a new size.
+ *
+ * callers should be advised that this function takes over ownership of
+ * the allocated memory, i.e. they can't count on the pointer to remain
+ * valid.
*/
- err_set_error_data_int(data, flags);
+ err_set_error_data_int(data, strlen(data) + 1, flags, 1);
}
void ERR_add_error_data(int num, ...)
@@ -810,7 +835,8 @@ void ERR_add_error_data(int num, ...)
void ERR_add_error_vdata(int num, va_list args)
{
int i, len, size;
- char *str, *p, *arg;
+ int flags = ERR_TXT_MALLOCED | ERR_TXT_STRING;
+ char *str, *arg;
ERR_STATE *es;
/* Get the current error data; if an allocated string get it. */
@@ -818,16 +844,30 @@ void ERR_add_error_vdata(int num, va_list args)
if (es == NULL)
return;
i = es->top;
- p = es->err_data_flags[i] == (ERR_TXT_MALLOCED | ERR_TXT_STRING)
- ? es->err_data[i] : "";
- /* Start with initial (or empty) string and allocate a new buffer */
- size = 80 + strlen(p);
- if ((str = OPENSSL_malloc(size + 1)) == NULL) {
- /* ERRerr(ERR_F_ERR_ADD_ERROR_VDATA, ERR_R_MALLOC_FAILURE); */
+ /*
+ * If err_data is allocated already, re-use the space.
+ * Otherwise, allocate a small new buffer.
+ */
+ if ((es->err_data_flags[i] & flags) == flags) {
+ str = es->err_data[i];
+ size = es->err_data_size[i];
+
+ /*
+ * To protect the string we just grabbed from tampering by other
+ * functions we may call, or to protect them from freeing a pointer
+ * that may no longer be valid at that point, we clear away the
+ * data pointer and the flags. We will set them again at the end
+ * of this function.
+ */
+ es->err_data[i] = NULL;
+ es->err_data_flags[i] = 0;
+ } else if ((str = OPENSSL_malloc(size = 81)) == NULL) {
return;
+ } else {
+ str[0] = '\0';
}
- strcpy(str, p);
+ len = strlen(str);
for (len = 0; --num >= 0; ) {
arg = va_arg(args, char *);
@@ -835,6 +875,8 @@ void ERR_add_error_vdata(int num, va_list args)
arg = "<NULL>";
len += strlen(arg);
if (len > size) {
+ char *p;
+
size = len + 20;
p = OPENSSL_realloc(str, size + 1);
if (p == NULL) {
@@ -845,7 +887,7 @@ void ERR_add_error_vdata(int num, va_list args)
}
OPENSSL_strlcat(str, arg, (size_t)size + 1);
}
- if (!err_set_error_data_int(str, ERR_TXT_MALLOCED | ERR_TXT_STRING))
+ if (!err_set_error_data_int(str, size, flags, 0))
OPENSSL_free(str);
}
@@ -873,7 +915,7 @@ int ERR_pop_to_mark(void)
while (es->bottom != es->top
&& (es->err_flags[es->top] & ERR_FLAG_MARK) == 0) {
- err_clear(es, es->top);
+ err_clear(es, es->top, 0);
es->top = es->top > 0 ? es->top - 1 : ERR_NUM_ERRORS - 1;
}
diff --git a/include/openssl/err.h b/include/openssl/err.h
index 3fa30ab2c1..e84bc68a4e 100644
--- a/include/openssl/err.h
+++ b/include/openssl/err.h
@@ -46,6 +46,7 @@ typedef struct err_state_st {
int err_flags[ERR_NUM_ERRORS];
unsigned long err_buffer[ERR_NUM_ERRORS];
char *err_data[ERR_NUM_ERRORS];
+ size_t err_data_size[ERR_NUM_ERRORS];
int err_data_flags[ERR_NUM_ERRORS];
const char *err_file[ERR_NUM_ERRORS];
int err_line[ERR_NUM_ERRORS];
More information about the openssl-commits
mailing list