Fix and document BIO_FLAGS_NONCLEAR_RST behavior on memory BIO

The BIO_FLAGS_NONCLEAR_RST flag behavior was not properly documented
and it also caused the length to be incorrectly set after the reset
operation.

Reviewed-by: Bernd Edlinger <bernd.edlinger@hotmail.de>
Reviewed-by: Paul Dale <paul.dale@oracle.com>
(Merged from https://github.com/openssl/openssl/pull/9179)

(cherry picked from commit 8b7b32921e63c492fa7233d81b11ee4d7ba266de)

diff --git a/crypto/bio/bss_mem.c b/crypto/bio/bss_mem.c
index 8c621d6c1e880d6bebdd1b991c89ad77bf836245..2d536e9db05f4cfd4560d3d3f0825daf066a7c85 100644 (file)
--- a/crypto/bio/bss_mem.c
+++ b/crypto/bio/bss_mem.c
@@ -259,9 +259,7 @@ static long mem_ctrl(BIO *b, int cmd, long num, void *ptr)
         bm = bbm->buf;
         if (bm->data != NULL) {
             if (!(b->flags & BIO_FLAGS_MEM_RDONLY)) {
-                if (b->flags & BIO_FLAGS_NONCLEAR_RST) {
-                    bm->length = bm->max;
-                } else {
+                if (!(b->flags & BIO_FLAGS_NONCLEAR_RST)) {
                     memset(bm->data, 0, bm->max);
                     bm->length = 0;
                 }

diff --git a/doc/man3/BIO_s_mem.pod b/doc/man3/BIO_s_mem.pod
index 2a5d423648db6cbf1ec1b680f0383fd1e44aee28..9c62955e0f20afbd766fd97bab08d98b9d00ca33 100644 (file)
--- a/doc/man3/BIO_s_mem.pod
+++ b/doc/man3/BIO_s_mem.pod
@@ -41,9 +41,10 @@ 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 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.
+flag BIO_FLAGS_NONCLEAR_RST is not set, otherwise it just restores the read
+pointer to the state it was just after the last write was performed and the
+data can be read again. On a read only BIO it similarly restores the BIO to
+its original state and the read only data can be read again.
 
 BIO_eof() is true if no data is in the BIO.
 
@@ -79,11 +80,11 @@ first, so the supplied area of memory must be unchanged until the BIO is freed.
 Writes to memory BIOs will always succeed if memory is available: that is
 their size can grow indefinitely.
 
-Every read from a read write memory BIO will remove the data just read with
-an internal copy operation, if a BIO contains a lot of data and it is
-read in small chunks the operation can be very slow. The use of a read only
-memory BIO avoids this problem. If the BIO must be read write then adding
-a buffering BIO to the chain will speed up the process.
+Every write after partial read (not all data in the memory buffer was read)
+to a read write memory BIO will have to move the unread data with an internal
+copy operation, if a BIO contains a lot of data and it is read in small
+chunks intertwined with writes the operation can be very slow. Adding
+a buffering BIO to the chain can speed up the process.
 
 Calling BIO_set_mem_buf() on a BIO created with BIO_new_secmem() will
 give undefined results, including perhaps a program crash.
@@ -104,6 +105,15 @@ BIO is set to BIO_NOCLOSE, before freeing the BUF_MEM the data pointer
 in it must be set to NULL as the data pointer does not point to an
 allocated memory.
 
+Calling BIO_reset() on a read write memory BIO with BIO_FLAGS_NONCLEAR_RST
+flag set can have unexpected outcome when the reads and writes to the
+BIO are intertwined. As documented above the BIO will be reset to the
+state after the last completed write operation. The effects of reads
+preceeding that write operation cannot be undone.
+
+Calling BIO_get_mem_ptr() prior to a BIO_reset() call with
+BIO_FLAGS_NONCLEAR_RST set has the same effect as a write operation.
+
 =head1 BUGS
 
 There should be an option to set the maximum size of a memory BIO.

diff --git a/test/bio_memleak_test.c b/test/bio_memleak_test.c
index 9724148fae6e2f52fd5003f22f9d1df63b8b90b0..383c82d156c064a918330ef111134adc97df7baf 100644 (file)
--- a/test/bio_memleak_test.c
+++ b/test/bio_memleak_test.c
@@ -181,6 +181,45 @@ finish:
     return ok;
 }
 
+static int test_bio_nonclear_rst(void)
+{
+    int ok = 0;
+    BIO *bio = NULL;
+    char data[16];
+
+    bio = BIO_new(BIO_s_mem());
+    if (!TEST_ptr(bio))
+        goto finish;
+    if (!TEST_int_eq(BIO_puts(bio, "Hello World\n"), 12))
+        goto finish;
+
+    BIO_set_flags(bio, BIO_FLAGS_NONCLEAR_RST);
+
+    if (!TEST_int_eq(BIO_read(bio, data, 16), 12))
+        goto finish;
+    if (!TEST_mem_eq(data, 12, "Hello World\n", 12))
+        goto finish;
+    if (!TEST_int_gt(BIO_reset(bio), 0))
+        goto finish;
+
+    if (!TEST_int_eq(BIO_read(bio, data, 16), 12))
+        goto finish;
+    if (!TEST_mem_eq(data, 12, "Hello World\n", 12))
+        goto finish;
+
+    BIO_clear_flags(bio, BIO_FLAGS_NONCLEAR_RST);
+    if (!TEST_int_gt(BIO_reset(bio), 0))
+        goto finish;
+
+    if (!TEST_int_lt(BIO_read(bio, data, 16), 1))
+        goto finish;
+
+    ok = 1;
+
+finish:
+    BIO_free(bio);
+    return ok;
+}
 
 int global_init(void)
 {
@@ -196,5 +235,6 @@ int setup_tests(void)
     ADD_TEST(test_bio_new_mem_buf);
     ADD_TEST(test_bio_rdonly_mem_buf);
     ADD_TEST(test_bio_rdwr_rdonly);
+    ADD_TEST(test_bio_nonclear_rst);
     return 1;
 }