mirror/openssl
2
0
Fork 0
mirror of https://github.com/openssl/openssl.git synced 2025-02-23 14:42:15 +08:00
Code Issues Packages Projects Releases Wiki Activity

Improve documentation for BIO_s_mem

Browse Source 
Recent leak discovered by valgrind:
==1007580== at 0x483C815: malloc (vg_replace_malloc.c:431)
==1007580== by 0x2C2689: CRYPTO_zalloc (in /home/vien/microedge-c/test)
==1007580== by 0x295A17: BUF_MEM_new (in /home/vien/microedge-c/test)
==1007580== by 0x295A78: BUF_MEM_new_ex (in /home/vien/microedge-c/test)
==1007580== by 0x28CACE: mem_new (in /home/vien/microedge-c/test)
==1007580== by 0x285EA8: BIO_new_ex (in /home/vien/microedge-c/test)
==1007580== by 0x231894: convert_pubkey_ECC (tpm2_driver.c:221)
==1007580== by 0x232B73: create_ephemeral_key (tpm2_driver.c:641)
==1007580== by 0x232E1F: tpm_gen_keypair (tpm2_driver.c:695)
==1007580== by 0x22D60A: gen_keypair (se_driver_api.c:275)
==1007580== by 0x21FF35: generate_keypair (dhkey.c:142)
==1007580== by 0x24D4C8: __test_dhkey (dhkey_test.c:55)

led me to find that BIO_get_mem_data is informative only, it does not
transer ownership of a BIO_s_mems data structure to the caller.
Additionally treating it as such leads to the above leak, or possibly
data corruption in the event that BIO_set_close(bio, BIO_NOCLOSE) is not
set properly prior to calling BIO_free.

Made an attempt to fix it in a minimally invasive manner in the 3.1
branch, but based on discussion, its just not safe to do in an API
compatible way, so just document the sematics a little more clearly
here, and fix it properly in a future release

Reviewed-by: Hugo Landau <hlandau@openssl.org>
Reviewed-by: Tomas Mraz <tomas@openssl.org>
(Merged from https://github.com/openssl/openssl/pull/21724)
This commit is contained in:
Neil Horman 2023-08-14 12:17:11 -04:00 committed by Tomas Mraz
parent 4de23ee3dc
commit 66d1658b4d
1 changed files with 20 additions and 0 deletions
Show Stats Download Patch File Download Diff File Expand all files Collapse all files

20
doc/man3/BIO_s_mem.pod
View File

@ -59,6 +59,8 @@ positive return value B<v> should be set to a negative value, typically -1.
BIO_get_mem_data() sets *B<pp> to a pointer to the start of the memory BIOs data
and returns the total amount of data available. It is implemented as a macro.
Note the pointer returned by this call is informative, no transfer of ownership
of this memory is implied. See notes on BIO_set_close().
BIO_set_mem_buf() sets the internal BUF_MEM structure to B<bm> and sets the
close flag to B<c>, that is B<c> should be either BIO_CLOSE or BIO_NOCLOSE.
@ -114,6 +116,10 @@ preceding 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.
Calling BIO_set_close() with BIO_NOCLOSE orphans the BUF_MEM internal to the
BIO, _not_ its actual data buffer. See the examples section for the proper
method for claiming ownership of the data pointer for a deferred free operation.
=head1 BUGS
There should be an option to set the maximum size of a memory BIO.
@ -151,6 +157,20 @@ Extract the BUF_MEM structure from a memory BIO and then free up the BIO:
BIO_set_close(mem, BIO_NOCLOSE); /* So BIO_free() leaves BUF_MEM alone */
BIO_free(mem);
Extract the BUF_MEM ptr, claim ownership of the internal data and free the BIO
and BUF_MEM structure:
BUF_MEM *bptr;
char *data;
BIO_get_mem_data(bio, &data);
BIO_get_mem_ptr(bio, &bptr);
BIO_set_close(mem, BIO_NOCLOSE); /* So BIO_free orphans BUF_MEM */
BIO_free(bio);
bptr->data = NULL; /* Tell BUF_MEM to orphan data */
BUF_MEM_free(bptr);
...
free(data);
=head1 COPYRIGHT