Fix ex_data and session_dup issues (1.1.0) #3625
Conversation
| typedef int CRYPTO_EX_new(void *parent, void *ptr, CRYPTO_EX_DATA *ad, | ||
| int idx, long argl, void *argp); | ||
| typedef void CRYPTO_EX_new(void *parent, void *ptr, CRYPTO_EX_DATA *ad, | ||
| int idx, long argl, void *argp); |
There was a problem hiding this comment.
Side note: second time this makes me jump before I realise I'm looking at docs... ;-)
There was a problem hiding this comment.
This will be the third and final time!
|
Hmmm, was this a clean cherry pick of #3323? In that case, a separate PR shouldn't have been needed... |
No, I had significant updates to the unit test. |
test/exdatatest.c
Outdated
| MYOBJ_EX_DATA *ex_data; | ||
|
|
||
| OPENSSL_assert(idx == saved_idx2); | ||
| if (idx != saved_idx2) { |
There was a problem hiding this comment.
OPENSSL_assert() kills the process on assertion failure even in non-debug mode, so this "if" will never fail.
There was a problem hiding this comment.
I thought I was following a paradigm used elsewhere, but now, I'm not so sure. I will correct.
There was a problem hiding this comment.
We have used that paradigm but with normal assert() not OPENSSL_assert().
There was a problem hiding this comment.
master uses the TEST macros, so this was just a backporting issue.
There was a problem hiding this comment.
It was in this file... so that's been corrected in the latest update.
test/exdatatest.c
Outdated
| cp = MYOBJ_gethello2(t3); | ||
| OPENSSL_assert(cp == p); | ||
| if (cp != p) | ||
| return 1; | ||
| MYOBJ_free(t1); | ||
| MYOBJ_free(t2); | ||
| MYOBJ_free(t3); | ||
| free(saved_argp); | ||
| free(p); |
Code was added in commit b3c31a6 that overwrote the last ex_data value using CRYPTO_dup_ex_data() causing a memory leak, and potentially confusing the ex_data dup() callback. In ssl_session_dup(), fix error handling (properly reference and up-ref shared data) and new-up the ex_data before calling CRYPTO_dup_ex_data(); all other structures that dup ex_data have the destination ex_data new'd before the dup. Fix up some of the ex_data documentation. Reviewed-by: Matt Caswell <matt@openssl.org> Reviewed-by: Rich Salz <rsalz@openssl.org> (Merged from openssl#3323) (cherry picked from commit 1ee2125)
|
Looking for one more? @levitte ? |
|
ping @openssl/omc? This has already been committed into 1.0.2 and master; it's just missing in 1.1.0 |
|
@mattcaswell can you merge? |
Code was added in commit b3c31a6 that overwrote the last ex_data value using CRYPTO_dup_ex_data() causing a memory leak, and potentially confusing the ex_data dup() callback. In ssl_session_dup(), fix error handling (properly reference and up-ref shared data) and new-up the ex_data before calling CRYPTO_dup_ex_data(); all other structures that dup ex_data have the destination ex_data new'd before the dup. Fix up some of the ex_data documentation. Reviewed-by: Matt Caswell <matt@openssl.org> Reviewed-by: Rich Salz <rsalz@openssl.org> (Merged from #3625)
|
Pushed. Thanks. |
Code was added in commit b3c31a6 that overwrote the last ex_data value using CRYPTO_dup_ex_data() causing a memory leak, and potentially confusing the ex_data dup() callback. In ssl_session_dup(), fix error handling (properly reference and up-ref shared data) and new-up the ex_data before calling CRYPTO_dup_ex_data(); all other structures that dup ex_data have the destination ex_data new'd before the dup. Fix up some of the ex_data documentation. Reviewed-by: Matt Caswell <matt@openssl.org> Reviewed-by: Rich Salz <rsalz@openssl.org> (Merged from openssl#3625)
Backport of PR #3323
Code was added in commit b3c31a6 that overwrote the last ex_data value
using CRYPTO_dup_ex_data() causing a memory leak, and potentially
confusing the ex_data dup() callback.
In ssl_session_dup(), fix error handling (properly reference and up-ref
shared data) and new-up the ex_data before calling CRYPTO_dup_ex_data();
all other structures that dup ex_data have the destination ex_data new'd
before the dup.
Fix up some of the ex_data documentation.
Reviewed-by: Matt Caswell matt@openssl.org
Reviewed-by: Rich Salz rsalz@openssl.org
(Merged from #3323)
(cherry picked from commit 1ee2125)
Checklist