1 Topic by SheldonCooper (edited by SheldonCooper 2013-05-09 02:02:14)

  • Registered: 2011-05-31
  • Posts: 24

Topic: multithreading with sniffer application - [suggested bug and fix]

Hey all,
I monitor my own server and use the wolfSSL embedded ssl sniffer application using my server's private key. I register my server using ssl_setPrivateKey and do this only once (since i have only one server i monior). then, I call ssl_decodePacket for each packet. everything works just fine. recently, I tried to multithread the sniffer - I spawn some threads and call ssl_decodePacket from multiple threads. as far as i can see, the session table is thread safe, and it seems the application as a whole is built for it. however, I seem to crush when I ssl_decodePacket in a multithreaded fashion. I suspect it has to do with the fact that all the sessions to my server are using the same SnifferServer object, although, as I imagine, that has to be the case since I have only one server, accepting traffic on port 443.

I know SnifferServer is not being freed, this isn't the problem - I know it's quite general, but, any ideas?

sure, I can use the program with one thread, but I find the multithreaded version much faster to go over large data sets.

thanks,
Dan

Share

  • Registered: 2011-05-31
  • Posts: 24

Re: multithreading with sniffer application - [suggested bug and fix]

just a note:
I know I am not using solely the sniffer application, and it's most possible the error is in my addition of the multithreading, I have looked at it hard before posting. If you feel this question is out of place in the forum I completely understand. just a reply of whether my understanding of "snifferServer" is correct would suffice if you are not able to elaborate...


thanks,
Dan

Share

  • todd
  • Moderator
  • Offline
  • Registered: 2010-10-21
  • Posts: 98

Re: multithreading with sniffer application - [suggested bug and fix]

Hi Dan,

Access to the sniffer session table is thread safe.  What isn't thread safe, is using the same sniffer session from multiple threads.  For example, say sniffer session A is created by thread X. If 3 new packets come in for session A and threads X, Y, and Z all try to handle those packets concurrently that's a problem.  Ideally, you're main thread would associate an ssl sniffer session (client ip/client port <-> server ip/server port) with a particular thread and use that same thread for the lifetime of the session.  Short of that, the sniffer session would need a lock which isn't ideal in your scenario because once thread X locks the first packet from session A threads Y and Z would be blocked until thread X is done.  That defeats the whole purpose of what you're trying to accomplish.  Does that information help?

-Todd

Share

4 Reply by SheldonCooper (edited by SheldonCooper 2013-05-09 06:02:06)

  • Registered: 2011-05-31
  • Posts: 24

Re: multithreading with sniffer application - [suggested bug and fix]

thanks for your reply - however, I already solved the problem and suggest you patch it if possible:
while the session table is thread safe, the usage of ssl_decodePacket isn't thread safe, even if the the same session is always treated by the same thread:
note that whenever ssl_decodePacket is called, if syn is encountered the "CreateSession" method is invoked, which in turn calls RemoveStaleSessions. there, "old" sessions are removed, but no one makes sure that another thread is not currently using these "old" sessions. the bug is that currently used sessions are considered "old". this can be easily fixed by adding the following (one) line to the "GetSnifferSession" method in "sniffer.c" after the session is pointer is acquired and verified as non-null:

static SnifferSession* GetSnifferSession(...){
    ...
    while(session){ //obtaining the session according to ipInfo tcpInfo passed to the function
    }
    unlock(&SessionMutex);
    if(session){ //checking if a session was actually found
       .... 
       session->bornOn = time(NULL); ///******* new code ! *********// <--------------------
    }
}

this patch allows multithreaded use of ssl_decodePacket (assuming of course session-wise load balancing between the threads), and will not cause thread X to free a session thread Y is currently working on.
in addition, this also fixes a functionality problem (as I see it...), you don't really want to throw away a session SNIFFER_TIMEOUT seconds from arrival time, but only if the session was silent for SNIFFER_TIMEOUT.

what do you think?

regards,
Dan

Share

  • todd
  • Moderator
  • Offline
  • Registered: 2010-10-21
  • Posts: 98

Re: multithreading with sniffer application - [suggested bug and fix]

Thanks for the patch.  I agree with your analysis.  The only change I did was to move the bornOn reset into the locked region to prevent an unlikely race condition.  Here's the commit for your reference: https://github.com/cyassl/cyassl/commit … d0b6716afd

Thanks again.

Share

  • Registered: 2011-05-31
  • Posts: 24

Re: multithreading with sniffer application - [suggested bug and fix]

you are right, this patch should be under the locked region. thanks for patching!

Dan

Share