dccp: Fix the adjustments to AWL and SWL

This fixes a problem and a potential loophole with regard to seqno/ackno
validity: the problem is that the initial adjustments to AWL/SWL were
only performed at the begin of the connection, during the handshake.

Since the Sequence Window feature is always greater than Wmin=32 (7.5.2), 
it is however necessary to perform these adjustments at least for the first
W/W' (variables as per 7.5.1) packets in the lifetime of a connection.

This requirement is complicated by the fact that W/W' can change at any time
during the lifetime of a connection.

Therefore the consequence is to perform this safety check each time SWL/AWL
are updated.

A second problem solved by this patch is that the remote/local Sequence Window
feature values (which set the bounds for AWL/SWL/SWH) are undefined until the
feature negotiation has completed.

During the initial handshake we have more stringent sequence number protection,
the changes added by this patch effect that {A,S}W{L,H} are within the correct
bounds at the instant that feature negotiation completes (since the SeqWin
feature activation handlers call dccp_update_gsr/gss()). 

A detailed rationale is below -- can be removed from the commit message.


1. Server sequence number checks during initial handshake
---------------------------------------------------------
The server can not use the fields of the listening socket for seqno/ackno checks
and thus needs to store all relevant information on a per-connection basis on
the dccp_request socket. This is a size-constrained structure and has currently
only ISS (dreq_iss) and ISR (dreq_isr) defined.
Adding further fields (SW{L,H}, AW{L,H}) would increase the size of the struct
and it is questionable whether this will have any practical gain. The currently
implemented solution is as follows.
 * receiving first Request: dccp_v{4,6}_conn_request sets 
                            ISR := P.seqno, ISS := dccp_v{4,6}_init_sequence()

 * sending first Response:  dccp_v{4,6}_send_response via dccp_make_response()	
                            sets P.seqno := ISS, sets P.ackno := ISR

 * receiving retransmitted Request: dccp_check_req() overrides ISR := P.seqno

 * answering retransmitted Request: dccp_make_response() sets ISS += 1,
                                    otherwise as per first Response

 * completing the handshake: succeeds in dccp_check_req() for the first Ack
                             where P.ackno == ISS (P.seqno is not tested)

 * creating child socket: ISS, ISR are copied from the request_sock

This solution will succeed whenever the server can receive the Request and the
subsequent Ack in succession, without retransmissions. If there is packet loss,
the client needs to retransmit until this condition succeeds; it will otherwise
eventually give up. Adding further fields to the request_sock could increase
the robustness a bit, in that it would make possible to let a reordered Ack
(from a retransmitted Response) pass. The argument against such a solution is
that if the packet loss is not persistent and an Ack gets through, why not
wait for the one answering the original response: if the loss is persistent, it
is probably better to not start the connection in the first place.

Long story short: the present design (by Arnaldo) is simple and will likely work
just as well as a more complicated solution. As a consequence, {A,S}W{L,H} are
not needed until the moment the request_sock is cloned into the accept queue.

At that stage feature negotiation has completed, so that the values for the local
and remote Sequence Window feature (7.5.2) are known, i.e. we are now in a better
position to compute {A,S}W{L,H}.


2. Client sequence number checks during initial handshake
---------------------------------------------------------
Until entering PARTOPEN the client does not need the adjustments, since it 
constrains the Ack window to the packet it sent.

 * sending first Request: dccp_v{4,6}_connect() choose ISS, 
                          dccp_connect() then sets GAR := ISS (as per 8.5),
			  dccp_transmit_skb() (with the previous bug fix) sets
			         GSS := ISS, AWL := ISS, AWH := GSS
 * n-th retransmitted Request (with previous patch):
	                  dccp_retransmit_skb() via timer calls
			  dccp_transmit_skb(), which sets GSS := ISS+n
                          and then AWL := ISS, AWH := ISS+n
	                  
 * receiving any Response: dccp_rcv_request_sent_state_process() 
	                   -- accepts packet if AWL <= P.ackno <= AWH;
			   -- sets GSR = ISR = P.seqno

 * sending the Ack completing the handshake: dccp_send_ack() calls 
                           dccp_transmit_skb(), which sets GSS += 1
			   and AWL := ISS, AWH := GSS
			   

Signed-off-by: Gerrit Renker <gerrit@erg.abdn.ac.uk>

1 files changed, 6 insertions, 12 deletions

diff --git a/net/dccp/input.c b/net/dccp/input.c
index 5eb443f656c1..e3f43d55e3ce 100644
--- a/net/dccp/input.c
+++ b/net/dccp/input.c
@@ -440,20 +440,14 @@ static int dccp_rcv_request_sent_state_process(struct sock *sk,
                 kfree_skb(sk->sk_send_head);
                 sk->sk_send_head = NULL;
 
-                dp->dccps_isr = DCCP_SKB_CB(skb)->dccpd_seq;
-                dccp_update_gsr(sk, dp->dccps_isr);
                 /*
-                 * SWL and AWL are initially adjusted so that they are not less than
-                 * the initial Sequence Numbers received and sent, respectively:
-                 *      SWL := max(GSR + 1 - floor(W/4), ISR),
-                 *      AWL := max(GSS - W' + 1, ISS).
-                 * These adjustments MUST be applied only at the beginning of the
-                 * connection.
-                 *
-                 * AWL was adjusted in dccp_v4_connect -acme
+                 * Set ISR, GSR from packet. ISS was set in dccp_v{4,6}_connect
+                 * and GSS in dccp_transmit_skb(). Setting AWL/AWH and SWL/SWH
+                 * is done as part of activating the feature values below, since
+                 * these settings depend on the local/remote Sequence Window
+                 * features, which were undefined or not confirmed until now.
                  */
-                dccp_set_seqno(&dp->dccps_swl,
-                               max48(dp->dccps_swl, dp->dccps_isr));
+                dp->dccps_gsr = dp->dccps_isr = DCCP_SKB_CB(skb)->dccpd_seq;
 
                 dccp_sync_mss(sk, icsk->icsk_pmtu_cookie);