Optimized BIO mem read - without reallocation

Currently on every BIO mem read operation the remaining data is reallocated.
This commit solves the issue.
BIO mem structure includes additional pointer to the read position.
On every read the pointer moves instead of reallocating the memory for the remaining data.
Reallocation accures before write and some ioctl operations, if the read pointer doesn't point on the beginning of the buffer.
Also the flag is added to rewind the read pointer without losing the data.

Reviewed-by: Richard Levitte <levitte@openssl.org>
Reviewed-by: Rich Salz <rsalz@openssl.org>

diff --git a/crypto/bio/bio_lcl.h b/crypto/bio/bio_lcl.h
index 7f3b22268a1e530c6f610bfc6f8e0eacc7f3381c..ffd9e89ccd0b3c79fc0e7c2eab3fdca4263bd75a 100644 (file)
--- a/crypto/bio/bio_lcl.h
+++ b/crypto/bio/bio_lcl.h
@@ -76,6 +76,12 @@ union bio_addr_st {
 
 /* END BIO_ADDRINFO/BIO_ADDR stuff. */
 
+/* BIO_BUF_MEM  */
+struct bio_buf_mem_st {
+    struct buf_mem_st *buf;   /* allocated buffer */
+    struct buf_mem_st *readp; /* read pointer */
+};
+
 #include "internal/cryptlib.h"
 #include <internal/bio.h>
 

diff --git a/crypto/bio/bss_mem.c b/crypto/bio/bss_mem.c
index 460e070a7d957937008c969bd72604e5b092a1d0..cc7b6d908edc99bfce6a72721e1372cb3c2c2388 100644 (file)
--- a/crypto/bio/bss_mem.c
+++ b/crypto/bio/bss_mem.c
@@ -68,6 +68,8 @@ static long mem_ctrl(BIO *h, int cmd, long arg1, void *arg2);
 static int mem_new(BIO *h);
 static int secmem_new(BIO *h);
 static int mem_free(BIO *data);
+static int mem_buf_free(BIO *data, int free_all);
+static int mem_buf_sync(BIO *h);
 static const BIO_METHOD mem_method = {
     BIO_TYPE_MEM,
     "memory buffer",
@@ -112,6 +114,7 @@ BIO *BIO_new_mem_buf(const void *buf, int len)
 {
     BIO *ret;
     BUF_MEM *b;
+    BIO_BUF_MEM *bb;
     size_t sz;
 
     if (buf == NULL) {
@@ -121,11 +124,13 @@ BIO *BIO_new_mem_buf(const void *buf, int len)
     sz = (len < 0) ? strlen(buf) : (size_t)len;
     if ((ret = BIO_new(BIO_s_mem())) == NULL)
         return NULL;
-    b = (BUF_MEM *)ret->ptr;
+    bb = (BIO_BUF_MEM *)ret->ptr;
+    b = bb->buf;
     /* Cast away const and trust in the MEM_RDONLY flag. */
     b->data = (void *)buf;
     b->length = sz;
     b->max = sz;
+    *bb->readp = *bb->buf;
     ret->flags |= BIO_FLAGS_MEM_RDONLY;
     /* Since this is static data retrying wont help */
     ret->num = 0;
@@ -134,14 +139,19 @@ BIO *BIO_new_mem_buf(const void *buf, int len)
 
 static int mem_init(BIO *bi, unsigned long flags)
 {
-    BUF_MEM *b;
+    BIO_BUF_MEM *bb = OPENSSL_zalloc(sizeof(BIO_BUF_MEM));
 
-    if ((b = BUF_MEM_new_ex(flags)) == NULL)
+    if (bb == NULL)
+        return(0);
+    if ((bb->buf = BUF_MEM_new_ex(flags)) == NULL)
+        return(0);
+    if ((bb->readp = OPENSSL_zalloc(sizeof(BUF_MEM))) == NULL)
         return(0);
+    *bb->readp = *bb->buf;
     bi->shutdown = 1;
     bi->init = 1;
     bi->num = -1;
-    bi->ptr = (char *)b;
+    bi->ptr = (char *)bb;
     return(1);
 }
 
@@ -156,38 +166,64 @@ static int secmem_new(BIO *bi)
 }
 
 static int mem_free(BIO *a)
+{
+    return (mem_buf_free(a, 1));
+}
+
+static int mem_buf_free(BIO *a, int free_all)
 {
     if (a == NULL)
         return (0);
     if (a->shutdown) {
         if ((a->init) && (a->ptr != NULL)) {
             BUF_MEM *b;
-            b = (BUF_MEM *)a->ptr;
-            if (a->flags & BIO_FLAGS_MEM_RDONLY)
-                b->data = NULL;
-            BUF_MEM_free(b);
+            BIO_BUF_MEM *bb = (BIO_BUF_MEM *)a->ptr;
+
+            if(bb != NULL) {
+                b = bb->buf;
+                if (a->flags & BIO_FLAGS_MEM_RDONLY)
+                    b->data = NULL;
+                BUF_MEM_free(b);
+                if(free_all) {
+                    OPENSSL_free(bb->readp);
+                    OPENSSL_free(bb);
+                }
+            }
             a->ptr = NULL;
         }
     }
     return (1);
 }
 
+/*
+ * Reallocate memory buffer if read pointer differs
+ */
+static int mem_buf_sync(BIO *b)
+{
+    if((b != NULL) && (b->init) && (b->ptr != NULL)) {
+        BIO_BUF_MEM *bbm = (BIO_BUF_MEM *)b->ptr;
+
+        if(bbm->readp->data != bbm->buf->data) {
+            memmove(bbm->buf->data, bbm->readp->data, bbm->readp->length);
+            bbm->buf->length = bbm->readp->length;
+            bbm->readp->data = bbm->buf->data;
+        }
+    }
+    return (0);
+}
+
 static int mem_read(BIO *b, char *out, int outl)
 {
     int ret = -1;
-    BUF_MEM *bm;
+    BIO_BUF_MEM *bbm = (BIO_BUF_MEM *)b->ptr;
+    BUF_MEM *bm = bbm->readp;
 
-    bm = (BUF_MEM *)b->ptr;
     BIO_clear_retry_flags(b);
     ret = (outl >= 0 && (size_t)outl > bm->length) ? (int)bm->length : outl;
     if ((out != NULL) && (ret > 0)) {
         memcpy(out, bm->data, ret);
         bm->length -= ret;
-        if (b->flags & BIO_FLAGS_MEM_RDONLY)
-            bm->data += ret;
-        else {
-            memmove(&(bm->data[0]), &(bm->data[ret]), bm->length);
-        }
+        bm->data += ret;
     } else if (bm->length == 0) {
         ret = b->num;
         if (ret != 0)
@@ -200,24 +236,23 @@ static int mem_write(BIO *b, const char *in, int inl)
 {
     int ret = -1;
     int blen;
-    BUF_MEM *bm;
+    BIO_BUF_MEM *bbm = (BIO_BUF_MEM *)b->ptr;
 
-    bm = (BUF_MEM *)b->ptr;
     if (in == NULL) {
         BIOerr(BIO_F_MEM_WRITE, BIO_R_NULL_PARAMETER);
         goto end;
     }
-
     if (b->flags & BIO_FLAGS_MEM_RDONLY) {
         BIOerr(BIO_F_MEM_WRITE, BIO_R_WRITE_TO_READ_ONLY_BIO);
         goto end;
     }
-
     BIO_clear_retry_flags(b);
-    blen = bm->length;
-    if (BUF_MEM_grow_clean(bm, blen + inl) == 0)
+    blen = bbm->readp->length;
+    mem_buf_sync(b);
+    if (BUF_MEM_grow_clean(bbm->buf, blen + inl) == 0)
         goto end;
-    memcpy(&(bm->data[blen]), in, inl);
+    memcpy(bbm->buf->data + blen, in, inl);
+    *bbm->readp = *bbm->buf;
     ret = inl;
  end:
     return (ret);
@@ -227,29 +262,32 @@ static long mem_ctrl(BIO *b, int cmd, long num, void *ptr)
 {
     long ret = 1;
     char **pptr;
-
-    BUF_MEM *bm = (BUF_MEM *)b->ptr;
+    BIO_BUF_MEM *bbm = (BIO_BUF_MEM *)b->ptr;
+    BUF_MEM *bm;
 
     switch (cmd) {
     case BIO_CTRL_RESET:
+        bm = bbm->buf;
         if (bm->data != NULL) {
             /* For read only case reset to the start again */
-            if (b->flags & BIO_FLAGS_MEM_RDONLY) {
-                bm->data -= bm->max - bm->length;
+            if ((b->flags & BIO_FLAGS_MEM_RDONLY) || (b->flags & BIO_FLAGS_NONCLEAR_RST)) {
                 bm->length = bm->max;
             } else {
                 memset(bm->data, 0, bm->max);
                 bm->length = 0;
             }
+            *bbm->readp = *bbm->buf;
         }
         break;
     case BIO_CTRL_EOF:
+        bm = bbm->readp;
         ret = (long)(bm->length == 0);
         break;
     case BIO_C_SET_BUF_MEM_EOF_RETURN:
         b->num = (int)num;
         break;
     case BIO_CTRL_INFO:
+        bm = bbm->readp;
         ret = (long)bm->length;
         if (ptr != NULL) {
             pptr = (char **)ptr;
@@ -257,12 +295,16 @@ static long mem_ctrl(BIO *b, int cmd, long num, void *ptr)
         }
         break;
     case BIO_C_SET_BUF_MEM:
-        mem_free(b);
+        mem_buf_free(b, 0);
         b->shutdown = (int)num;
-        b->ptr = ptr;
+        bbm->buf = ptr;
+        *bbm->readp = *bbm->buf;
+        b->ptr = bbm;
         break;
     case BIO_C_GET_BUF_MEM_PTR:
         if (ptr != NULL) {
+            mem_buf_sync(b);
+            bm = bbm->readp;
             pptr = (char **)ptr;
             *pptr = (char *)bm;
         }
@@ -273,11 +315,11 @@ static long mem_ctrl(BIO *b, int cmd, long num, void *ptr)
     case BIO_CTRL_SET_CLOSE:
         b->shutdown = (int)num;
         break;
-
     case BIO_CTRL_WPENDING:
         ret = 0L;
         break;
     case BIO_CTRL_PENDING:
+        bm = bbm->readp;
         ret = (long)bm->length;
         break;
     case BIO_CTRL_DUP:
@@ -298,7 +340,8 @@ static int mem_gets(BIO *bp, char *buf, int size)
     int i, j;
     int ret = -1;
     char *p;
-    BUF_MEM *bm = (BUF_MEM *)bp->ptr;
+    BIO_BUF_MEM *bbm = (BIO_BUF_MEM *)bp->ptr;
+    BUF_MEM *bm = bbm->readp;
 
     BIO_clear_retry_flags(bp);
     j = bm->length;

diff --git a/doc/crypto/BIO_s_mem.pod b/doc/crypto/BIO_s_mem.pod
index 03e291dce303ef036643a66eb09ff536052ebae9..84abb29823ca161a7c4625789b29e5eb66eeb4c4 100644 (file)
--- a/doc/crypto/BIO_s_mem.pod
+++ b/doc/crypto/BIO_s_mem.pod
@@ -39,9 +39,10 @@ Memory BIOs support BIO_gets() and BIO_puts().
 If the BIO_CLOSE flag is set when a memory BIO is freed then the underlying
 BUF_MEM structure is also freed.
 
-Calling BIO_reset() on a read write memory BIO clears any data in it. On a
-read only BIO it restores the BIO to its original state and the read only
-data can be read again.
+Calling BIO_reset() on a read write memory BIO clears any data in it if the
+flag BIO_FLAGS_NONCLEAR_RST is not set. On a read only BIO or if the flag
+BIO_FLAGS_NONCLEAR_RST is set it restores the BIO to its original state and 
+the data can be read again.
 
 BIO_eof() is true if no data is in the BIO.
 
@@ -90,12 +91,6 @@ give undefined results, including perhaps a program crash.
 
 There should be an option to set the maximum size of a memory BIO.
 
-There should be a way to "rewind" a read write BIO without destroying
-its contents.
-
-The copying operation should not occur after every small read of a large BIO
-to improve efficiency.
-
 =head1 EXAMPLE
 
 Create a memory BIO and write some data to it:

diff --git a/include/openssl/bio.h b/include/openssl/bio.h
index a32c2d38862f59db584c99699cfef8ade649657d..e0c6380883cee0b971c07e848157395bdefa87fe 100644 (file)
--- a/include/openssl/bio.h
+++ b/include/openssl/bio.h
@@ -216,10 +216,12 @@ extern "C" {
 # define BIO_FLAGS_BASE64_NO_NL  0x100
 
 /*
- * This is used with memory BIOs: it means we shouldn't free up or change the
- * data in any way.
+ * This is used with memory BIOs:
+ * BIO_FLAGS_MEM_RDONLY means we shouldn't free up or change the data in any way;
+ * BIO_FLAGS_NONCLEAR_RST means we should't clear data on reset.
  */
 # define BIO_FLAGS_MEM_RDONLY    0x200
+# define BIO_FLAGS_NONCLEAR_RST  0x400
 
 typedef union bio_addr_st BIO_ADDR;
 typedef struct bio_addrinfo_st BIO_ADDRINFO;

diff --git a/include/openssl/ossl_typ.h b/include/openssl/ossl_typ.h
index 7a8d319989e6269db09ae6760afee1bebe2ae9b5..ee2f70af137f551fa097775172908021af4a4dd3 100644 (file)
--- a/include/openssl/ossl_typ.h
+++ b/include/openssl/ossl_typ.h
@@ -128,6 +128,7 @@ typedef struct bn_recp_ctx_st BN_RECP_CTX;
 typedef struct bn_gencb_st BN_GENCB;
 
 typedef struct buf_mem_st BUF_MEM;
+typedef struct bio_buf_mem_st BIO_BUF_MEM;
 
 typedef struct evp_cipher_st EVP_CIPHER;
 typedef struct evp_cipher_ctx_st EVP_CIPHER_CTX;