diff mbox

[2/2,nf] netfilter: seqadj: Fix the wrong ack adjust for the RST packet without ack

Message ID 1473087725-22089-1-git-send-email-fgao@ikuai8.com
State Superseded
Delegated to: Pablo Neira
Headers show

Commit Message

高峰 Sept. 5, 2016, 3:02 p.m. UTC
From: Gao Feng <fgao@ikuai8.com>

It is valid that the TCP RST packet which does not set ack flag, and bytes
of ack number are zero. For these RST packets, seqadj could not adjust the
ack number.

Signed-off-by: Gao Feng <fgao@ikuai8.com>
---
 net/netfilter/nf_conntrack_seqadj.c | 34 +++++++++++++++++++---------------
 1 file changed, 19 insertions(+), 15 deletions(-)

Comments

Feng Gao Sept. 5, 2016, 3:06 p.m. UTC | #1
Hi Pablo,

On Mon, Sep 5, 2016 at 11:02 PM,  <fgao@ikuai8.com> wrote:
> From: Gao Feng <fgao@ikuai8.com>
>
> It is valid that the TCP RST packet which does not set ack flag, and bytes
> of ack number are zero. For these RST packets, seqadj could not adjust the
> ack number.
>
> Signed-off-by: Gao Feng <fgao@ikuai8.com>
> ---
>  net/netfilter/nf_conntrack_seqadj.c | 34 +++++++++++++++++++---------------
>  1 file changed, 19 insertions(+), 15 deletions(-)
>
> diff --git a/net/netfilter/nf_conntrack_seqadj.c b/net/netfilter/nf_conntrack_seqadj.c
> index 7f8d814..65bb4a6 100644
> --- a/net/netfilter/nf_conntrack_seqadj.c
> +++ b/net/netfilter/nf_conntrack_seqadj.c
> @@ -182,30 +182,34 @@ int nf_ct_seq_adjust(struct sk_buff *skb,
>
>         tcph = (void *)skb->data + protoff;
>         spin_lock_bh(&ct->lock);
> +
>         if (after(ntohl(tcph->seq), this_way->correction_pos))
>                 seqoff = this_way->offset_after;
>         else
>                 seqoff = this_way->offset_before;
>
> -       if (after(ntohl(tcph->ack_seq) - other_way->offset_before,
> -                 other_way->correction_pos))
> -               ackoff = other_way->offset_after;
> -       else
> -               ackoff = other_way->offset_before;
> -
>         newseq = htonl(ntohl(tcph->seq) + seqoff);
> -       newack = htonl(ntohl(tcph->ack_seq) - ackoff);
> -
>         inet_proto_csum_replace4(&tcph->check, skb, tcph->seq, newseq, false);
> -       inet_proto_csum_replace4(&tcph->check, skb, tcph->ack_seq, newack,
> -                                false);
> -
> -       pr_debug("Adjusting sequence number from %u->%u, ack from %u->%u\n",
> -                ntohl(tcph->seq), ntohl(newseq), ntohl(tcph->ack_seq),
> -                ntohl(newack));
>
> +       pr_debug("Adjusting sequence number from %u->%u\n",
> +                ntohl(tcph->seq), ntohl(newseq));
>         tcph->seq = newseq;
> -       tcph->ack_seq = newack;
> +
> +       if (likely(tcph->ack)) {
> +               if (after(ntohl(tcph->ack_seq) - other_way->offset_before,
> +                         other_way->correction_pos))
> +                       ackoff = other_way->offset_after;
> +               else
> +                       ackoff = other_way->offset_before;
> +
> +               newack = htonl(ntohl(tcph->ack_seq) - ackoff);
> +               inet_proto_csum_replace4(&tcph->check, skb, tcph->ack_seq,
> +                                        newack, false);
> +
> +               pr_debug("Adjusting ack number from %u->%u\n",
> +                        ntohl(tcph->ack_seq), ntohl(newack));
> +               tcph->ack_seq = newack;
> +       }
>
>         res = nf_ct_sack_adjust(skb, protoff, tcph, ct, ctinfo);
>         spin_unlock_bh(&ct->lock);
> --
> 1.9.1
>
>

This patch is generated base on the patch commit "netfilter: seqadj:
Fix one possible panic in seqadj when mem is exhausted" whose link is
http://patchwork.ozlabs.org/patch/665116/.

So its subject contains "2/2".

Best Regards
Feng



Best Regards
Feng
--
To unsubscribe from this list: send the line "unsubscribe netfilter-devel" in
the body of a message to majordomo@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
diff mbox

Patch

diff --git a/net/netfilter/nf_conntrack_seqadj.c b/net/netfilter/nf_conntrack_seqadj.c
index 7f8d814..65bb4a6 100644
--- a/net/netfilter/nf_conntrack_seqadj.c
+++ b/net/netfilter/nf_conntrack_seqadj.c
@@ -182,30 +182,34 @@  int nf_ct_seq_adjust(struct sk_buff *skb,
 
 	tcph = (void *)skb->data + protoff;
 	spin_lock_bh(&ct->lock);
+
 	if (after(ntohl(tcph->seq), this_way->correction_pos))
 		seqoff = this_way->offset_after;
 	else
 		seqoff = this_way->offset_before;
 
-	if (after(ntohl(tcph->ack_seq) - other_way->offset_before,
-		  other_way->correction_pos))
-		ackoff = other_way->offset_after;
-	else
-		ackoff = other_way->offset_before;
-
 	newseq = htonl(ntohl(tcph->seq) + seqoff);
-	newack = htonl(ntohl(tcph->ack_seq) - ackoff);
-
 	inet_proto_csum_replace4(&tcph->check, skb, tcph->seq, newseq, false);
-	inet_proto_csum_replace4(&tcph->check, skb, tcph->ack_seq, newack,
-				 false);
-
-	pr_debug("Adjusting sequence number from %u->%u, ack from %u->%u\n",
-		 ntohl(tcph->seq), ntohl(newseq), ntohl(tcph->ack_seq),
-		 ntohl(newack));
 
+	pr_debug("Adjusting sequence number from %u->%u\n",
+		 ntohl(tcph->seq), ntohl(newseq));
 	tcph->seq = newseq;
-	tcph->ack_seq = newack;
+
+	if (likely(tcph->ack)) {
+		if (after(ntohl(tcph->ack_seq) - other_way->offset_before,
+			  other_way->correction_pos))
+			ackoff = other_way->offset_after;
+		else
+			ackoff = other_way->offset_before;
+
+		newack = htonl(ntohl(tcph->ack_seq) - ackoff);
+		inet_proto_csum_replace4(&tcph->check, skb, tcph->ack_seq,
+					 newack, false);
+
+		pr_debug("Adjusting ack number from %u->%u\n",
+			 ntohl(tcph->ack_seq), ntohl(newack));
+		tcph->ack_seq = newack;
+	}
 
 	res = nf_ct_sack_adjust(skb, protoff, tcph, ct, ctinfo);
 	spin_unlock_bh(&ct->lock);