ClearFoundation

Project/Bug Tracker


Viewing Issue Simple Details Jump to Notes ] View Advanced ] Issue History ] Print ]
ID Category Severity Reproducibility Date Submitted Last Update
0000003 [Userspace] Source Code crash have not tried 2010-05-05 00:22 2010-07-22 16:36
Reporter james.king View Status public  
Assigned To dsokoloski
Priority normal Resolution fixed  
Status resolved   Product Version 0.11
Summary 0000003: Make l7_connections map access thread-safe
Description As originally reported on the l7-filter-users ML (http://sourceforge.net/mailarchive/message.php?msg_name=185cb23.2196.1272c1eb3e1.Coremail.lzqsist%40163.com), [^] the following crash occurred with v0.7:

#0 0x00be775d in std::_Rb_tree_rotate_left () from /usr/lib/libstdc++.so.6
(gdb) bt
#0 0x00be775d in std::_Rb_tree_rotate_left () from /usr/lib/libstdc++.so.6
0000001 0x00be78ca in std::_Rb_tree_insert_and_rebalance ()
   from /usr/lib/libstdc++.so.6
0000002 0x0804f9e8 in std::_Rb_tree<std::string, std::pair<std::string const, l7_connection*>, std::_Select1st<std::pair<std::string const, l7_connection*> >, std::less<std::string>, std::allocator<std::pair<std::string const, l7_connection*> > >::_M_insert (this=0x855e868, __x=0x0, __p=0x88d1858, __v=@0x9ff9f218)
    at /usr/lib/gcc/i386-redhat-linux/4.1.2/../../../../include/c++/4.1.2/bits/stl_tree.h:821
0000003 0x08050003 in std::_Rb_tree<std::string, std::pair<std::string const, l7_connection*>, std::_Select1st<std::pair<std::string const, l7_connection*> >, std::less<std::string>, std::allocator<std::pair<std::string const, l7_connection*> > >::insert_unique (this=0x855e868, __position={_M_node = 0x8fe409a8},
    __v=@0x9ff9f218)
    at /usr/lib/gcc/i386-redhat-linux/4.1.2/../../../../include/c++/4.1.2/bits/stl_tree.h:962
0000004 0x0804edda in l7_conntrack::get_l7_connection (this=0x855e868, key=
        {static npos = 4294967295, _M_dataplus = {<std::allocator<char>> = {<__gnu_cxx::new_allocator<char>> = {<No data fields>}, <No data fields>}, _M_p = 0x9ff9f284 "\004\034\215\bT\033\215\bd\003\215\b"}})
    at /usr/lib/gcc/i386-redhat-linux/4.1.2/../../../../include/c++/4.1.2/bits/stl_map.h:420
0000005 0x0804dfa2 in l7_queue::handle_packet (this=0x85413a8, tb=0x9ff9f2d0,
---Type <return> to continue, or q <return> to quit---return
    qh=0x855d688) at l7-queue.cpp:216
0000006 0x00178df6 in __nfq_rcv_pkt (nlh=0x9ff9f39c, nfa=0x9ff9f2f0,
    data=0x85608a0) at libnetfilter_queue.c:134
0000007 0x002f0943 in __nfnl_handle_msg (h=<value optimized out>, nlh=0x9ff9f39c,
    len=<value optimized out>) at libnfnetlink.c:1185
0000008 0x002f098f in nfnl_handle_packet (h=0x8560758, buf=0x9ff9f39c "\220",
    len=<value optimized out>) at libnfnetlink.c:1205
0000009 0x0017932d in nfq_handle_packet (h=0x85608a0, buf=0x9ff9f39c "\220",
    len=144) at libnetfilter_queue.c:271
0000010 0x0804d69d in l7_queue::start (this=0x85413a8,
    queuenum=<value optimized out>) at l7-queue.cpp:144
0000011 0x0805033c in start_queue_thread (qnum=0xbff4fc4c) at l7-filter.cpp:114
0000012 0x008675ab in start_thread () from /lib/libpthread.so.0
#13 0x0024ccfe in clone () from /lib/libc.so.6

Looks like this is because l7_handle_conntrack_event() callback may try to add to or remove from the map while l7_queue::handle_packet() is attempting a read.

The reporter tested the following patch I generated against v0.11 and confirmed it resolved their problem:

--- a/l7-conntrack.cpp 2009-02-26 13:40:28.000000000 -0800
+++ b/l7-conntrack.cpp 2010-03-10 10:08:43.000000000 -0800
@@ -195,10 +195,12 @@
 {
  nfct_conntrack_free(ct);
  nfct_close(cth);
+ pthread_mutex_destroy(&map_mutex);
 }

 l7_conntrack::l7_conntrack(void* l7_classifier_in)
 {
+ pthread_mutex_init(&map_mutex, NULL);
  l7_classifier = (l7_classify *)l7_classifier_in;

  // Now open a handler that is subscribed to all possible events
@@ -211,19 +213,27 @@

 l7_connection *l7_conntrack::get_l7_connection(const string key)
 {
- return l7_connections[key];
+ l7_connection *conn;
+ pthread_mutex_lock(&map_mutex);
+ conn = l7_connections[key];
+ pthread_mutex_unlock(&map_mutex);
+ return conn;
 }

 void l7_conntrack::add_l7_connection(l7_connection* connection,
                                       const string key)
 {
+ pthread_mutex_lock(&map_mutex);
  l7_connections[key] = connection;
+ pthread_mutex_unlock(&map_mutex);
 }

 void l7_conntrack::remove_l7_connection(const string key)
 {
+ pthread_mutex_lock(&map_mutex);
  delete l7_connections[key];
  l7_connections.erase(l7_connections.find(key));
+ pthread_mutex_unlock(&map_mutex);
 }

 void l7_conntrack::start()
diff -urN a/l7-conntrack.h b/l7-conntrack.h
--- a/l7-conntrack.h 2007-05-10 18:42:50.000000000 -0700
+++ b/l7-conntrack.h 2010-03-10 09:51:29.000000000 -0800
@@ -52,6 +52,7 @@
  l7_map l7_connections;
  struct nfct_conntrack *ct;
  struct nfct_handle *cth; // the callback
+ pthread_mutex_t map_mutex;

 public:
  l7_conntrack(void * foo);
Additional Information
Tags No tags attached.
Attached Files

- Relationships

-  Notes
(0000003)
dsokoloski (administrator)
2010-05-05 11:40

Yes I've experienced this crash as well. Thanks for the patch, I will apply and test.
(0000004)
dsokoloski (administrator)
2010-05-05 16:33

Source Code Changelog
---------------------------------------------------
- Applied l7_conections map mutex patch from James King. Resolves crash resulting from multi-threaded modification of the l7_connections map. [fixed tracker 0000003]

File Changes
---------------------------------------------------
Details: http://code.clearfoundation.com/svn/revision.php?repname=l7-filter&rev=325 [^]
U l7-filter-userspace/trunk/THANKS
U l7-filter-userspace/trunk/l7-conntrack.cpp
U l7-filter-userspace/trunk/l7-conntrack.h

- Issue History
Date Modified Username Field Change
2010-05-05 00:22 james.king New Issue
2010-05-05 11:40 dsokoloski Note Added: 0000003
2010-05-05 11:40 dsokoloski Assigned To => dsokoloski
2010-05-05 11:40 dsokoloski Status new => acknowledged
2010-05-05 11:40 dsokoloski Projection none => minor fix
2010-05-05 16:33 dsokoloski Checkin
2010-05-05 16:33 dsokoloski Note Added: 0000004
2010-05-05 16:33 dsokoloski Status acknowledged => resolved
2010-05-05 16:33 dsokoloski Resolution open => fixed
2010-05-10 15:12 pbaldwin Fixed in Version => 0.12
2010-07-22 16:36 pbaldwin Target Version => 0.12