[openssl-dev] [openssl.org #3955] [PATCH] Reduce stack usage in PKCS7_verify()

2015-09-05 Thread Rich Salz via RT
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()

2015-07-24 Thread David Woodhouse via RT
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()

2015-07-24 Thread David Woodhouse
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()

2015-07-23 Thread Salz, Rich via RT
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()

2015-07-23 Thread Peter Waltenberg

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()

2015-07-23 Thread Peter Waltenberg via RT

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()

2015-07-23 Thread David Woodhouse via RT
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