[Click] a bug in tcprewriter.cc:apply_sack() and/or tcprewriter.cc:apply()?

[Weidong Cui]

Hi There,

I think there is a bug in apply_sack() in tcprewriter.cc though I 
couldn't figure out the fix.  Here is the story.

1. I used a downstream tcprewriter to manipulate TCP sequence numbers 
for a TCP proxy module.  I manually assign the seqno_delta for the 
reverse mapping in the TCP proxy code:

         // update seqno_delta in tcprewriter
         if (TCPRewriter::TCPMapping *p_mapping = 
_tcp_rewriter->get_mapping (IP_PROTO_TCP, tcp_flow)) {
             tcp_state->_server_syn_seqno = ntohl (tcph->th_seq);
             int32_t delta;
             if (tcp_state->_filter_syn_seqno >= 
tcp_state->_server_syn_seqno)
                 delta = tcp_state->_filter_syn_seqno - 
tcp_state->_server_syn_seqno;
             else
                 delta = -1 * (tcp_state->_server_syn_seqno - 
tcp_state->_filter_syn_seqno);
             p_mapping->update_seqno_delta 
(tcp_state->_server_syn_seqno, delta);
         }

2. Randomly (in the sense of running time, but in all three runs) Click 
fell into an infinite loop (used 99% CPU while normally it was around 
10%).  After stopping it in gdb, I found Click stopped at the line of 
"break" in tcprewriter.cc:apply_sack():

     // finish off csum_delta calculation
     for (uint16_t *csum = csum_begin; reinterpret_cast<uint8_t *>(csum) 
< end_sack; csum++)
         csum_delta += *csum;
     break;

3. I found that there is condition check on if there is a 
reverse_seqno_delta in tcprewriter.cc:apply() before calling 
tpcrewriter.cc:apply_sack():

     // update SACK sequence numbers
     if ((tcph->th_off > 8
      || (tcph->th_off == 8
          && *(reinterpret_cast<const uint32_t *>(tcph + 1)) != 
htonl(0x0101080A)))
     && reverse()->have_seqno_delta())
     csum_delta += reverse()->apply_sack(tcph, p->transport_length());

This made me think that it's because I manually added 
reverse_seqno_delta.  Also there shouldn't be no SACK in all the TCP 
connections seen by the tcprewriter because the TCP proxy refuses all 
TCP options during three-way handshakes before anything reaches the 
tcprewriter.

4. I commented off the above lines in tcprewriter.cc:apply() and the 
problem was gone.

Please let me know if you have any questions.

Thanks,
Weidong

-- 
_____________________________________________________
Weidong Cui
Ph.D. Candidate      wdc at EECS.Berkeley.EDU
EECS, UC Berkeley    http://www.cs.berkeley.edu/~wdc/