On 25/11/14 23:20, Praveen Kariyanahalli wrote:
> Hi Matt
> 
> Trying out your patch. Will keep you posted. In meanwhile we ran into
> more valgrind issues .. on the server end. Can you please comment on them?
> 
> ==621== 8,680 (1,488 direct, 7,192 indirect) bytes in 62 blocks are
> definitely lost in loss record 899 of 952  
> ==621==    at 0x4A05F80: malloc (vg_replace_malloc.c:296)              
>                                        
> ==621==    by 0x5BFCC86: default_malloc_ex (mem.c:79)                  
>                                        
> ==621==    by 0x5BFD315: CRYPTO_malloc (mem.c:308)                      
>                                       
> ==621==    by 0x5D2414D: pitem_new (pqueue.c:73)                        
>                                       
> ==621==    by 0x5958F74: dtls1_buffer_message (d1_both.c:1233)          
>                                       
> ==621==    by 0x594E3B2: dtls1_send_server_done (d1_srvr.c:1032)

This doesn't look right. The code is sending a ServerHelloDone message
here, but...


>                                       
> ==621==    by 0x594D696: dtls1_accept (d1_srvr.c:564)                  
>                                        
> ==621==    by 0x595C555: SSL_accept (ssl_lib.c:940)                    
>                                        
> ==621==    by 0x59539F7: dtls1_listen (d1_lib.c:491)                    
>
...here you have entered dtls1_listen.

This internal function gets called when you call the public API function
DTLSv1_listen - which is actually implemented as a macro that calls
SSL_ctrl - which explains the rest of the stack trace below:



> ==621==    by 0x59533BF: dtls1_ctrl (d1_lib.c:267)                      
>                                       
> ==621==    by 0x595CAF2: SSL_ctrl (ssl_lib.c:1106)                      
>                                       
> ==621==    by 0x416229: server_ssl_event_cb (server.c:3823)


The purpose of DTLSv1_listen is to listen for incoming datagrams from
anyone. If it receives a ClientHello without a cookie it immediately
responds with a HelloVerifyRequest containing a cookie. The client is
expected to respond with a second ClientHello containing the cookie. The
idea is that this is a DoS protection mechanism.

If DTLSv1_listen receives a ClientHello *with* a cookie then it will
return with a positive result. The server is then expected to continue
the handshake with a call to SSL_accept. This is often done in a
separate thread just for that SSL_accept call.

So something like this:
while(1)
{
    ssl = SSL_new(ctx);
    while(DTLSv1_listen(ssl, &client_addr) <= 0);
    /* client_addr will contain ip address of the client */
    Create_a_thread(ssl);
}

In new thread:
SSL_accept(ssl);

Now under the covers DTLSv1_listen calls dlts1_listen which calls
SSL_accept. The difference being that it has set various flags etc to
put it into the "listen" state.

In your stack trace you are sending a ServerHelloDone, which suggests to
me you are attempting to *complete* a handshake whilst in DTLSv1_listen.
So, in other words, in the above example once you have completed the
initial part of the handshake, you are completing it by calling
DTLSv1_listen again instead of calling SSL_accept (this will at least
look like its working because DTLSv1_listen does eventually call
SSL_accept).

Its not clear to me what will happen in that scenario...at best the
behaviour is "undefined" (we should probably put a check in to prevent
this from happening...if that is indeed what is going on). I'm not sure
whether that is the cause of this memory leak or not, but it certainly
won't help track it down.

Can you confirm whether you are using DTLSv1_listen like this or not? If
not something really weird is going on.

Matt

______________________________________________________________________
OpenSSL Project                                 http://www.openssl.org
Development Mailing List                       openssl-dev@openssl.org
Automated List Manager                           majord...@openssl.org

Reply via email to