Part of the problem reported here was resolved, namely the reference
count increment/decrement.
However, there is still a problem but I have a simple patch that fixes
it.
The problem is that the SSL may have the bbio in place when the pop
happens. If that is the case, then rbio != wbio and the
BIO_free_all(ssl->wbio) leads to two double frees later (the bbio and
the rbio). The fix is to remove the bbio is it in place. It doesn't need
to be freed, SSL_free will do that.
>From openssl-1.0.1e/ssl/bio_ssl.c:
case BIO_CTRL_POP:
/* Only detach if we are the BIO explicitly being popped
*/
if (b == ptr)
{
>> if (ssl->bbio && ssl_p->bbio == ssl_p->wbio)
>> ssl_p->wbio = BIO_POP(ssl_p->wbio);
/* Shouldn't happen in practice because the
* rbio and wbio are the same when pushed.
*/
if (ssl->rbio != ssl->wbio)
BIO_free_all(ssl->wbio);
if (b->next_bio != NULL)
CRYPTO_add(&b->next_bio->references,-1,CRYPTO_LOCK_BIO);
ssl->wbio=NULL;
ssl->rbio=NULL;
}
break;
Regards,
Tom Maher
-----Original Message-----
From: Tom Maher
Sent: 19 May 2006 15:15
To: [email protected]
Subject: Possible bug/leak in OpenSSL
ssl/bio_ssl.c:ssl_ctrl(BIO_CTRL_POP)
I suspect that there may be a bug in ssl/bio_ssl.c (OpenSSL 0.9.7j - and
earlier versions).
In a BIO_CTRL_PUSH, the next_bio->references is incremented.
In a BIO_CTRL_POP, the next_bio->references is also incremented.
Shouldn't it be decremented.
To worked around it I am using a BIO_free_all() instead of a BIO_pop(),
which is probably the recommened way, but I thought I should report the
possibility of a bug that could lead to memory leaks (in my case I was
leaking the BIO's under my SSL's).
static long ssl_ctrl(BIO *b, int cmd, long num, void *ptr)
{
...
case BIO_CTRL_PUSH:
if ((b->next_bio != NULL) && (b->next_bio != ssl->rbio))
{
SSL_set_bio(ssl,b->next_bio,b->next_bio);
CRYPTO_add(&b->next_bio->references,1,CRYPTO_LOCK_BIO);
}
break;
case BIO_CTRL_POP:
/* ugly bit of a hack */
if (ssl->rbio != ssl->wbio) /* we are in trouble :-( */
{
BIO_free_all(ssl->wbio);
}
if (b->next_bio != NULL)
{
<<
CRYPTO_add(&b->next_bio->references,1,CRYPTO_LOCK_BIO);
>>
CRYPTO_add(&b->next_bio->references,-1,CRYPTO_LOCK_BIO);
}
ssl->wbio=NULL;
ssl->rbio=NULL;
break;
...
}
Regards,
Tom Maher
______________________________________________________________________
OpenSSL Project http://www.openssl.org
Development Mailing List [email protected]
Automated List Manager [email protected]
