| Message ID | 20230724165832.15797-2-jo.vanbulck@cs.kuleuven.be (mailing list archive) |
|---|---|
| State | New, archived |
| Headers | show |
| Series | selftests/sgx: Fix compilation errors. | expand |
On Mon Jul 24, 2023 at 4:58 PM UTC, Jo Van Bulck wrote: > Ensure ctx is zero-initialized, such that the encl_measure function will > not call EVP_MD_CTX_destroy with an uninitialized ctx pointer in case of an > early error during key generation. > > Signed-off-by: Jo Van Bulck <jo.vanbulck@cs.kuleuven.be> > --- > tools/testing/selftests/sgx/sigstruct.c | 2 +- > 1 file changed, 1 insertion(+), 1 deletion(-) > > diff --git a/tools/testing/selftests/sgx/sigstruct.c b/tools/testing/selftests/sgx/sigstruct.c > index a07896a46364..dd1fdab90e26 100644 > --- a/tools/testing/selftests/sgx/sigstruct.c > +++ b/tools/testing/selftests/sgx/sigstruct.c > @@ -318,9 +318,9 @@ bool encl_measure(struct encl *encl) > struct sgx_sigstruct *sigstruct = &encl->sigstruct; > struct sgx_sigstruct_payload payload; > uint8_t digest[SHA256_DIGEST_LENGTH]; > + EVP_MD_CTX *ctx = NULL; > unsigned int siglen; > RSA *key = NULL; > - EVP_MD_CTX *ctx; > int i; > > memset(sigstruct, 0, sizeof(*sigstruct)); > -- > 2.34.1 Add a fixes tag. In other words, find the commit ID that caused the issue and add the output of the following to this patch before your sob: git --no-pager log --format='Fixes: %h ("%s")' --abbrev=12 -1 <commit ID>; BR, Jarkko
On Mon Jul 24, 2023 at 4:58 PM UTC, Jo Van Bulck wrote: > Ensure ctx is zero-initialized, such that the encl_measure function will > not call EVP_MD_CTX_destroy with an uninitialized ctx pointer in case of an > early error during key generation. > > Signed-off-by: Jo Van Bulck <jo.vanbulck@cs.kuleuven.be> > --- > tools/testing/selftests/sgx/sigstruct.c | 2 +- > 1 file changed, 1 insertion(+), 1 deletion(-) > > diff --git a/tools/testing/selftests/sgx/sigstruct.c b/tools/testing/selftests/sgx/sigstruct.c > index a07896a46364..dd1fdab90e26 100644 > --- a/tools/testing/selftests/sgx/sigstruct.c > +++ b/tools/testing/selftests/sgx/sigstruct.c > @@ -318,9 +318,9 @@ bool encl_measure(struct encl *encl) > struct sgx_sigstruct *sigstruct = &encl->sigstruct; > struct sgx_sigstruct_payload payload; > uint8_t digest[SHA256_DIGEST_LENGTH]; > + EVP_MD_CTX *ctx = NULL; > unsigned int siglen; > RSA *key = NULL; > - EVP_MD_CTX *ctx; > int i; > > memset(sigstruct, 0, sizeof(*sigstruct)); > -- > 2.34.1 Ditto. BR, Jarkko
On Mon, 2023-07-24 at 18:58 +0200, Jo Van Bulck wrote: > Ensure ctx is zero-initialized, such that the encl_measure function will > not call EVP_MD_CTX_destroy with an uninitialized ctx pointer in case of an > early error during key generation. > > Signed-off-by: Jo Van Bulck <jo.vanbulck@cs.kuleuven.be> > --- > tools/testing/selftests/sgx/sigstruct.c | 2 +- > 1 file changed, 1 insertion(+), 1 deletion(-) > > diff --git a/tools/testing/selftests/sgx/sigstruct.c b/tools/testing/selftests/sgx/sigstruct.c > index a07896a46364..dd1fdab90e26 100644 > --- a/tools/testing/selftests/sgx/sigstruct.c > +++ b/tools/testing/selftests/sgx/sigstruct.c > @@ -318,9 +318,9 @@ bool encl_measure(struct encl *encl) > struct sgx_sigstruct *sigstruct = &encl->sigstruct; > struct sgx_sigstruct_payload payload; > uint8_t digest[SHA256_DIGEST_LENGTH]; > + EVP_MD_CTX *ctx = NULL; > unsigned int siglen; > RSA *key = NULL; > - EVP_MD_CTX *ctx; > int i; > > memset(sigstruct, 0, sizeof(*sigstruct)); Is it safe to assume EVP_MD_CTX_destroy() can always handle a NULL ctx? The manpage says: EVP_MD_CTX_destroy() cleans up digest context ctx and frees up the space allocated to it, it should be called only on a context created using EVP_MD_CTX_create().
On 28.07.23 21:03, Jarkko Sakkinen wrote: > Add a fixes tag. In other words, find the commit ID that caused the issue > and add the output of the following to this patch before your sob: > > git --no-pager log --format='Fixes: %h ("%s")' --abbrev=12 -1 <commit ID>; > > BR, Jarkko Thank you, added for the next patch revision.
On 03.08.23 05:51, Huang, Kai wrote: > Is it safe to assume EVP_MD_CTX_destroy() can always handle a NULL ctx? > > The manpage says: > > EVP_MD_CTX_destroy() cleans up digest context ctx and frees up the space > allocated to it, it should be called only on a context created using > EVP_MD_CTX_create(). Thank you for pointing this out. Afais the implementations I've seen can handle NULL, and similar error-handling paths exists where EVP_MD_CTX_destroy() is called with a NULL pointer exist in several places in the openSSL code. That being said, this indeed not explicit in the specification (unlike RSA_free() which is called just after and explicitly specifies that NULL is okay). So you're probably right that it's generally safer to not call EVP_MD_CTX_destroy() with a NULL pointer. I'll include an extra check for this in the next patch revision.
diff --git a/tools/testing/selftests/sgx/sigstruct.c b/tools/testing/selftests/sgx/sigstruct.c index a07896a46364..dd1fdab90e26 100644 --- a/tools/testing/selftests/sgx/sigstruct.c +++ b/tools/testing/selftests/sgx/sigstruct.c @@ -318,9 +318,9 @@ bool encl_measure(struct encl *encl) struct sgx_sigstruct *sigstruct = &encl->sigstruct; struct sgx_sigstruct_payload payload; uint8_t digest[SHA256_DIGEST_LENGTH]; + EVP_MD_CTX *ctx = NULL; unsigned int siglen; RSA *key = NULL; - EVP_MD_CTX *ctx; int i; memset(sigstruct, 0, sizeof(*sigstruct));
Ensure ctx is zero-initialized, such that the encl_measure function will not call EVP_MD_CTX_destroy with an uninitialized ctx pointer in case of an early error during key generation. Signed-off-by: Jo Van Bulck <jo.vanbulck@cs.kuleuven.be> --- tools/testing/selftests/sgx/sigstruct.c | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-)