Your analysis is correct. However the library is still correct in regards to refcounting even for an SSL BIO in the chain. The reason is that the decrement of refcount of the BIOs underlying the SSL BIO is handled through the actual freeing of the SSL BIO. If the refcount for the SSL BIO in the chain freed by BIO_free_all() drops to zero, it will, in ssl_free() call in bio_ssl.c, free the underlying SSL * object. And if the refcount of that SSL object drops to zero it will call BIO_free_all() on the underlying wbio and rbio. Which means also the chained BIOs underlying the SSL BIO will have their refcount dropped and they will be properly freed. Tomas Mraz, OpenSSL On Mon, 2022-10-03 at 09:35 -0700, Jay Foster wrote: > Your response makes sense. I am a bit puzzled by the BIO reference > counting. For example > > BIO_new() (or BIO_new_socket() which calls BIO_new()) produces a > BIO with a reference count of 1. > BIO_free() drops 1 reference and if the reference count is 0, > frees > the BIO. > > BIO_push() connects the BIO to the BIO chain. No references are > directly changed. However, the BIO_CTRL_PUSH command is sent to the > BIO. For the SSL BIO, this results in a call to SSL_set_bio() and > adding 1 to the next_bio reference (socket BIO). > BIO_pop() calls BIO_CTRL_POP and disconnects the BIO from the > BIO > chain. For the SSL BIO, the BIO_CTRL_POP subtracts one from the > next_bio reference (undoing BIO_push()). > > The problem case is calling BIO_free_all() on the BIO chain. > BIO_free_all() does not use BIO_pop(). It simply detaches each BIO > from > the chain, and calls BIO_free() for it. If the next_bio reference > count > is > 1 (which it will be for the SSL BIO next_bio), it stops > unlinking > and freeing BIOs. This seems asymmetrical with respect to the BIO > reference counting, and leaks the next_bio (socket BIO in this case). > > What am I missing here? > Jay > > > On 9/29/2022 11:50 PM, Tomas Mraz wrote: > > The SSL BIO should have the rbio from the SSL object as the next > > BIO. > > If you create the SSL BIO and then BIO_push() the TCP socket BIO > > into > > the SSL BIO, it will work correctly. > > > > Otherwise, you can just fix the next BIO of the SSL BIO by using > > > > BIO_up_ref(socketbio); > > BIO_set_next(sslbio, socketbio); > > > > The SSL BIO should always have a next BIO if properly initialized. > > > > Tomas Mraz, OpenSSL > > > > On Thu, 2022-09-29 at 13:02 -0700, Jay Foster wrote: > > > I have an application that constructs a chain of BIOs. Sometimes > > > this > > > chain also includes an SSL BIO. Years ago, I ran into a problem > > > that > > > caused BIO_flush() to segfault on the SSL BIO. This turned out > > > to > > > happen because the SSL BIO is added using SSL_set_bio() instead > > > of > > > BIO_push(). SSL_set_bio() results in the SSL BIO always having a > > > NULL > > > bio_next value, so BIO_flush then crashes dereferencing this NULL > > > pointer when it calls BIO_copy_next_retry() on the SSL BIO (see > > > BIO_CTRL_FLUSH in ssl/bio_ssl.c). > > > > > > This was reported as ticket 2615 years ago. > > > > > > My question is, how could calling BIO_flush() on a BIO chain with > > > an > > > SSL > > > BIO ever work? Is there a way to add the SSL BIO using > > > BIO_push() > > > instead of SSL_set_bio()? > > > > > > Jay > > > > -- Tomáš Mráz, OpenSSL
