On Nov 20, 10:36pm, Jason Thorpe wrote:
} Subject: Re: Fix for "land" bug committed
} On Thu, 20 Nov 1997 22:26:59 -0800 
}  Jason Thorpe <thorpej@nas.nasa.gov> wrote:
} 
}  > I have just committed a fix for the "land" bug.  Here is the relevant
}  > commit message:
}  > 
}  > thorpej
}  > Thu Nov 20 22:18:32 PST 1997
}  > Update of /cvsroot/src/sys/netinet
}  > In directory netbsd1:/var/slash-tmp/cvs-serv14385
}  > 
}  > Modified Files:
}  >         tcp_input.c 
}  > Log Message:
}  > In tcp_input(), if the PCB we lookup for an incoming packet is a listen
}  > socket:
}  > - If we received a SYN,ACK, send an RST.

Shouldn't that already be happening:

        case TCPS_LISTEN: {
                struct mbuf *am;
                register struct sockaddr_in *sin;

                if (tiflags & TH_RST)
                        goto drop;
                if (tiflags & TH_ACK)
                        goto dropwithreset;


}  > - If we received a SYN, and the connection attempt appears to come from
}  >   itself, send an RST, since it cannot possibly be valid.
} 
} ...and I have changed it to just just drop the packet in the latter
} case.  Jason really needs to go home.

Dropping the packet is probably a better choice, but I don't think
this is a complete fix.  I *think* it might be possible to provoke this
between two different ports on the same machine by sending a SYN to
each in quick succession.

One would think that the initial SYN would cause the socket to
send a SYN,ACK and go into the SYN_RCVD state.  Then when it
received the SYN,ACK, it should see the SYN flag and:

        /*
         * If a SYN is in the window, then this is an
         * error and we send an RST and drop the connection.
         */
        if (tiflags & TH_SYN) {
                tp = tcp_drop(tp, ECONNRESET);
                goto dropwithreset;
        }

which should cause it to nuke the socket and then send RST to itself,
which should end the problem.

Depending on how the sequence numbers are initialized, it appears to me
that it is possible that the SYN flag in the SYN,ACK is being cleared
before this test by this code:

        todrop = tp->rcv_nxt - ti->ti_seq;
        if (todrop > 0) {
                if (tiflags & TH_SYN) {
                        tiflags &= ~TH_SYN;
                        ti->ti_seq++;
                        if (ti->ti_urp > 1)
                                ti->ti_urp--;
                        else
                                tiflags &= ~TH_URG;
                        todrop--;
                }

but that should be caught by the sequence number check in the SYN_RCVD
case of the ACK processing.

Another possibility, depending on the sequence numbers is:

        /*
         * If segment ends after window, drop trailing data
         * (and PUSH and FIN); if nothing left, just ACK.
         */
        todrop = (ti->ti_seq+ti->ti_len) - (tp->rcv_nxt+tp->rcv_wnd);
        if (todrop > 0) {
                tcpstat.tcps_rcvpackafterwin++;
                if (todrop >= ti->ti_len) {

			.
			.
			.

                        /*
                         * If window is closed can only take segments at
                         * window edge, and have to drop data and PUSH from
                         * incoming segments.  Continue processing, but
                         * remember to ack.  Otherwise, drop segment
                         * and ack.
                         */
                        if (tp->rcv_wnd == 0 && ti->ti_seq == tp->rcv_nxt) {
                                tp->t_flags |= TF_ACKNOW;
                                tcpstat.tcps_rcvwinprobe++;
                        } else
                                goto dropafterack;


which could result in an ACK contest ...

Since the check to move from the SYN_RCVD state to the ESTABLISHED state
is picky about sequence numbers:

        case TCPS_SYN_RECEIVED:
                if (SEQ_GT(tp->snd_una, ti->ti_ack) ||
                    SEQ_GT(ti->ti_ack, tp->snd_max))
                        goto dropwithreset;

I think this test should be done before the data is trimmed to fit the
window (this is already done in the SYN_SENT state).