[openssl-dev] [openssl.org #3955] [PATCH] Reduce stack usage in PKCS7_verify()
fixed in master; allocate space dynamically. we're not going to do a "stack use audit" platforms that need this are better equipped to do that, and can share the results with us. -- Rich Salz, OpenSSL dev team; rs...@openssl.org ___ openssl-dev mailing list To unsubscribe: https://mta.openssl.org/mailman/listinfo/openssl-dev
Re: [openssl-dev] [openssl.org #3955] [PATCH] Reduce stack usage in PKCS7_verify()
On Thu, 2015-07-23 at 20:33 +, Salz, Rich via RT wrote: How about 256 on the stack? Sure. -- David WoodhouseOpen Source Technology Centre david.woodho...@intel.com Intel Corporation From 57aa658b429b1962e2811c30e2b77edb85d134d3 Mon Sep 17 00:00:00 2001 From: David Woodhouse david.woodho...@intel.com Date: Fri, 24 Jul 2015 10:15:04 +0100 Subject: [PATCH] RT3955: Reduce stack usage in PKCS7_verify() and PKCS7_decrypt() Some environments, such as 32-bit UEFI, have strict limits on stack size. Using a 4KiB buffer on the stack for reading from the bio is somewhat excessive, so reduce it to 256 bytes. This might incur a slight performance penalty but it should be negligible. --- crypto/pkcs7/pk7_smime.c | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/crypto/pkcs7/pk7_smime.c b/crypto/pkcs7/pk7_smime.c index e52e746..c3afc96 100644 --- a/crypto/pkcs7/pk7_smime.c +++ b/crypto/pkcs7/pk7_smime.c @@ -253,7 +253,7 @@ int PKCS7_verify(PKCS7 *p7, STACK_OF(X509) *certs, X509_STORE *store, STACK_OF(PKCS7_SIGNER_INFO) *sinfos; PKCS7_SIGNER_INFO *si; X509_STORE_CTX cert_ctx; -char buf[4096]; +char buf[256]; int i, j = 0, k, ret = 0; BIO *p7bio; BIO *tmpin, *tmpout; @@ -519,7 +519,7 @@ int PKCS7_decrypt(PKCS7 *p7, EVP_PKEY *pkey, X509 *cert, BIO *data, int flags) { BIO *tmpmem; int ret, i; -char buf[4096]; +char buf[256]; if (!p7) { PKCS7err(PKCS7_F_PKCS7_DECRYPT, PKCS7_R_INVALID_NULL_POINTER); -- 2.4.3 smime.p7s Description: S/MIME cryptographic signature ___ openssl-dev mailing list To unsubscribe: https://mta.openssl.org/mailman/listinfo/openssl-dev
Re: [openssl-dev] [openssl.org #3955] [PATCH] Reduce stack usage in PKCS7_verify()
On Thu, 2015-07-23 at 20:33 +, Salz, Rich via RT wrote: How about 256 on the stack? Sure. -- David WoodhouseOpen Source Technology Centre david.woodho...@intel.com Intel Corporation From 57aa658b429b1962e2811c30e2b77edb85d134d3 Mon Sep 17 00:00:00 2001 From: David Woodhouse david.woodho...@intel.com Date: Fri, 24 Jul 2015 10:15:04 +0100 Subject: [PATCH] RT3955: Reduce stack usage in PKCS7_verify() and PKCS7_decrypt() Some environments, such as 32-bit UEFI, have strict limits on stack size. Using a 4KiB buffer on the stack for reading from the bio is somewhat excessive, so reduce it to 256 bytes. This might incur a slight performance penalty but it should be negligible. --- crypto/pkcs7/pk7_smime.c | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/crypto/pkcs7/pk7_smime.c b/crypto/pkcs7/pk7_smime.c index e52e746..c3afc96 100644 --- a/crypto/pkcs7/pk7_smime.c +++ b/crypto/pkcs7/pk7_smime.c @@ -253,7 +253,7 @@ int PKCS7_verify(PKCS7 *p7, STACK_OF(X509) *certs, X509_STORE *store, STACK_OF(PKCS7_SIGNER_INFO) *sinfos; PKCS7_SIGNER_INFO *si; X509_STORE_CTX cert_ctx; -char buf[4096]; +char buf[256]; int i, j = 0, k, ret = 0; BIO *p7bio; BIO *tmpin, *tmpout; @@ -519,7 +519,7 @@ int PKCS7_decrypt(PKCS7 *p7, EVP_PKEY *pkey, X509 *cert, BIO *data, int flags) { BIO *tmpmem; int ret, i; -char buf[4096]; +char buf[256]; if (!p7) { PKCS7err(PKCS7_F_PKCS7_DECRYPT, PKCS7_R_INVALID_NULL_POINTER); -- 2.4.3 smime.p7s Description: S/MIME cryptographic signature ___ openssl-dev mailing list To unsubscribe: https://mta.openssl.org/mailman/listinfo/openssl-dev
Re: [openssl-dev] [openssl.org #3955] [PATCH] Reduce stack usage in PKCS7_verify()
How about 256 on the stack? ___ openssl-dev mailing list To unsubscribe: https://mta.openssl.org/mailman/listinfo/openssl-dev
Re: [openssl-dev] [openssl.org #3955] [PATCH] Reduce stack usage in PKCS7_verify()
bn/bn_prime.c static int probable_prime(BIGNUM *rnd, int bits) { int i; prime_t mods[NUMPRIMES]; == BN_ULONG delta, maxdelta; This one is also excessive. The problem is that even on OS's with dynamic thread stack if you do cause a stack overrun, the entire process gets frozen, a new stack for that thread is allocated, stack copied, process restarted. Sounds O.K., but if you have a 1000 threads and they all sequentially hit their guard pages performance suffers rather badly with the entire process being stalled for each thread. OS's without dynamic thread stacks just crash. And yes, 256 bytes is usually O.K., but it's overall thread stack use for the component that really needs to be audited and kept within some fixed budget. Any single stack allocation 4k is generally bad news as that's large enough to reach past the (typical) 4k guard pages. Peter From: Salz, Rich via RT r...@openssl.org To: dw...@infradead.org Cc: openssl-dev@openssl.org Date: 24/07/2015 06:35 AM Subject:Re: [openssl-dev] [openssl.org #3955] [PATCH] Reduce stack usage inPKCS7_verify() Sent by:openssl-dev openssl-dev-boun...@openssl.org How about 256 on the stack? ___ openssl-dev mailing list To unsubscribe: https://mta.openssl.org/mailman/listinfo/openssl-dev ___ openssl-dev mailing list To unsubscribe: https://mta.openssl.org/mailman/listinfo/openssl-dev
Re: [openssl-dev] [openssl.org #3955] [PATCH] Reduce stack usage in PKCS7_verify()
bn/bn_prime.c static int probable_prime(BIGNUM *rnd, int bits) { int i; prime_t mods[NUMPRIMES]; == BN_ULONG delta, maxdelta; This one is also excessive. The problem is that even on OS's with dynamic thread stack if you do cause a stack overrun, the entire process gets frozen, a new stack for that thread is allocated, stack copied, process restarted. Sounds O.K., but if you have a 1000 threads and they all sequentially hit their guard pages performance suffers rather badly with the entire process being stalled for each thread. OS's without dynamic thread stacks just crash. And yes, 256 bytes is usually O.K., but it's overall thread stack use for the component that really needs to be audited and kept within some fixed budget. Any single stack allocation 4k is generally bad news as that's large enough to reach past the (typical) 4k guard pages. Peter From: Salz, Rich via RT r...@openssl.org To: dw...@infradead.org Cc: openssl-dev@openssl.org Date: 24/07/2015 06:35 AM Subject:Re: [openssl-dev] [openssl.org #3955] [PATCH] Reduce stack usage inPKCS7_verify() Sent by:openssl-dev openssl-dev-boun...@openssl.org How about 256 on the stack? ___ openssl-dev mailing list To unsubscribe: https://mta.openssl.org/mailman/listinfo/openssl-dev ___ openssl-dev mailing list To unsubscribe: https://mta.openssl.org/mailman/listinfo/openssl-dev
[openssl-dev] [openssl.org #3955] [PATCH] Reduce stack usage in PKCS7_verify()
From: Long, Qin qin.l...@intel.com Some environments, such as 32-bit UEFI, have strict limits on stack size. Using a 4KiB buffer on the stack for reading from p7bio is somewhat excessive, so allocate it on the heap instead. --- Alternatively, we could leave it on the stack and reduce it to 256 bytes or something like that. It's not as if performance is really an issue here if we do that, right? crypto/pkcs7/pk7_smime.c | 14 -- 1 file changed, 12 insertions(+), 2 deletions(-) diff --git a/crypto/pkcs7/pk7_smime.c b/crypto/pkcs7/pk7_smime.c index e52e746..077b06d 100644 --- a/crypto/pkcs7/pk7_smime.c +++ b/crypto/pkcs7/pk7_smime.c @@ -253,7 +253,8 @@ int PKCS7_verify(PKCS7 *p7, STACK_OF(X509) *certs, X509_STORE *store, STACK_OF(PKCS7_SIGNER_INFO) *sinfos; PKCS7_SIGNER_INFO *si; X509_STORE_CTX cert_ctx; -char buf[4096]; +char *buf = NULL; +int bufsiz; int i, j = 0, k, ret = 0; BIO *p7bio; BIO *tmpin, *tmpout; @@ -365,9 +366,14 @@ int PKCS7_verify(PKCS7 *p7, STACK_OF(X509) *certs, X509_STORE *store, } else tmpout = out; +bufsiz = 4096; +buf = OPENSSL_malloc(bufsiz); +if (buf == NULL) { +goto err; +} /* We now have to 'read' from p7bio to calculate digests etc. */ for (;;) { -i = BIO_read(p7bio, buf, sizeof(buf)); +i = BIO_read(p7bio, buf, bufsiz); if (i = 0) break; if (tmpout) @@ -407,6 +413,10 @@ int PKCS7_verify(PKCS7 *p7, STACK_OF(X509) *certs, X509_STORE *store, sk_X509_free(signers); +if (buf != NULL) { + OPENSSL_free(buf); +} + return ret; } -- 2.4.3 -- David WoodhouseOpen Source Technology Centre david.woodho...@intel.com Intel Corporation smime.p7s Description: S/MIME cryptographic signature ___ openssl-bugs-mod mailing list openssl-bugs-...@openssl.org https://mta.openssl.org/mailman/listinfo/openssl-bugs-mod___ openssl-dev mailing list To unsubscribe: https://mta.openssl.org/mailman/listinfo/openssl-dev