Gerrit,
> | --- a/net/dccp/input.c
> | +++ b/net/dccp/input.c
> | @@ -252,7 +253,10 @@ static int dccp_check_seqno(struct sock *sk, struct
> | sk_buff *skb)
> | if (between48(seqno, lswl, dp->dccps_swh) &&
> | (ackno == DCCP_PKT_WITHOUT_ACK_SEQ ||
> | between48(ackno, lawl, dp->dccps_awh))) {
> | - dccp_update_gsr(sk, seqno);
> | +
> | + if(after48(seqno, dp->dccps_gsr)){
> | + dccp_update_gsr(sk, seqno);
> | + }
> |
> | if (dh->dccph_type != DCCP_PKT_SYNC &&
> | ackno != DCCP_PKT_WITHOUT_ACK_SEQ &&
>
> I would like to move that change from dccp_check_seqno() into
> dccp_update_gsr(), since the latter function is also called for Sync/SyncAck
> packets, where the same problem exists.
In the case of Sync/SyncAck packets, RFC 4340 section 7.5.4 requires
that:
"On receiving a sequence-valid DCCP-Sync packet, the peer endpoint
(say, DCCP B) MUST update its GSR variable and reply with a DCCP-
SyncAck packet. The DCCP-SyncAck packet's Acknowledgement Number
will equal the DCCP-Sync's Sequence Number, which is not necessarily
GSR. Upon receiving this DCCP-SyncAck, which will be sequence-valid
since it acknowledges the DCCP-Sync, DCCP A will update its GSR
variable, and the endpoints will be back in sync."
Adding the check for > GSR inside dccp_update_gsr() breaks this
requirement.
Further, Sync/SyncAck packets are already checked for sequence validity
by dccp_check_seqno() before the call to dccp_update_gsr(). The checks
used follow RFC 4340 section 7.5.3:
DCCP-Sync SWL <= seqno AWL <= ackno <= AWH
DCCP-SyncAck SWL <= seqno AWL <= ackno <= AWH
As such, I would leave the check in dccp_check_seqno(). Alternatively,
it could be moved to dccp_update_gsr() and a further check for not
Sync/SyncAck included.
Samuel Jero
Internetworking Research Group
Ohio University
>
> The edited patch is below -- please have a look. This allows to safely call
> dccp_update_gsr() without moving the window backwards. Calling that function
> may be required not only when GSR changes, but also when the Sequence Window
> value is updated, which requires to update the SWL/SWH boundaries.
>
>
> >>>>>>>>>>>>>>>>>>>>>> Patch v2 <<<<<<<<<<<<<<<<<<<<<<<<<<<<<<<<<<<<<<<<<<<<<
> dccp: fix bug in updating the GSR
>
> Currently dccp_check_seqno allows any valid packet to update the Greatest
> Sequence Number Received, even if that packet's sequence number is less than
> the current GSR. This patch adds a check to make sure that the new packet's
> sequence number is greater than GSR.
>
> Signed-off-by: Samuel Jero <sj323707@xxxxxxxx>
> Signed-off-by: Gerrit Renker <gerrit@xxxxxxxxxxxxxx>
> ---
> net/dccp/dccp.h | 3 ++-
> 1 file changed, 2 insertions(+), 1 deletion(-)
>
> --- a/net/dccp/dccp.h
> +++ b/net/dccp/dccp.h
> @@ -426,7 +426,8 @@ static inline void dccp_update_gsr(struc
> {
> struct dccp_sock *dp = dccp_sk(sk);
>
> - dp->dccps_gsr = seq;
> + if (after48(seq, dp->dccps_gsr))
> + dp->dccps_gsr = seq;
> /* Sequence validity window depends on remote Sequence Window (7.5.1) */
> dp->dccps_swl = SUB48(ADD48(dp->dccps_gsr, 1), dp->dccps_r_seq_win / 4);
> /*
Attachment:
signature.asc
Description: This is a digitally signed message part
