Bug in tcp_output?

Chris Harrer cjharrer at comcast.net
Thu Mar 18 20:33:15 UTC 2010


Hi All,

 

In the following block of code, running on a x86_64 platform, I believe that
cwin should be declared as an int:

        /*

         * If snd_nxt == snd_max and we have transmitted a FIN, the

         * offset will be > 0 even if so_snd.sb_cc is 0, resulting in

         * a negative length.  This can also occur when TCP opens up

         * its congestion window while receiving additional duplicate

         * acks after fast-retransmit because TCP will reset snd_nxt

         * to snd_max after the fast-retransmit.

         *

         * In the normal retransmit-FIN-only case, however, snd_nxt will

         * be set to snd_una, the offset will be 0, and the length may

         * wind up 0.

         *

         * If sack_rxmit is true we are retransmitting from the scoreboard

         * in which case len is already set.

         */

        if (sack_rxmit == 0) {

               if (sack_bytes_rxmt == 0)

                       len = ((long)ulmin(so->so_snd.sb_cc, sendwin) - off);

               else {

                       long cwin;  ß-- Should be an int

 

                        /*

                        * We are inside of a SACK recovery episode and are

                        * sending new data, having retransmitted all the

                        * data possible in the scoreboard.

                        */

                       len = ((long)ulmin(so->so_snd.sb_cc, tp->snd_wnd) 

                              - off);

                       /*

                        * Don't remove this (len > 0) check !

                        * We explicitly check for len > 0 here (although it 

                        * isn't really necessary), to work around a gcc 

                        * optimization issue - to force gcc to compute

                        * len above. Without this check, the computation

                        * of len is bungled by the optimizer.

                        */

                       if (len > 0) {

                               cwin = tp->snd_cwnd - 

                                      (tp->snd_nxt - tp->sack_newdata) -

                                      sack_bytes_rxmt;

                               if (cwin < 0)

                                      cwin = 0;

                               len = lmin(len, cwin);

                       }

               }

        }

 

Consider the case where:

sack_rxmit = 0

sack_bytes_rxmt = 0x2238

off = 0

len =0xa19c

tp->snd_cwnd = 0x2238

tp->snd_nxt = 0xdd6d7974

tp->sack_newdata = 0xdd6d6858

In this case cwin evaluates to 0x00000000ffffe37c, which is not <0, but
instead huge.  This causes the remaining data on the socket’s so->so_snd
buffer to be sent to the network causing more problems at the receiver which
is already dropping frames.

Thanks,

 

Chris



More information about the freebsd-net mailing list