diff mbox series

[ovs-dev,v4,3/3] conntrack: Fix FTP seq_skew boundary adjustments.

Message ID 1547542519-26269-4-git-send-email-david.marchand@redhat.com
State Superseded
Headers show
Series fixes for ftp alg in userspace dp | expand

Commit Message

David Marchand Jan. 15, 2019, 8:55 a.m. UTC 
From: Darrell Ball <dlu998@gmail.com>

At the same time, splice out a function and also rely on the compiler
for overflow/underflow handling.

Found by inspection.

Fixes: bd5e81a0e596 ("Userspace Datapath: Add ALG infra and FTP."
Signed-off-by: Darrell Ball <dlu998@gmail.com>
Signed-off-by: David Marchand <david.marchand@redhat.com>
---

Changelog since v3:
- added the v2 patch [1] from Darrell and rebased it on my patchset

1: https://patchwork.ozlabs.org/patch/1024719/

---
 lib/conntrack.c | 38 ++++++++++----------------------------
 1 file changed, 10 insertions(+), 28 deletions(-)

Comments

0-day Robot Jan. 15, 2019, 3:24 p.m. UTC | #1 
Bleep bloop.  Greetings David Marchand, I am a robot and I have tried out your patch.
Thanks for your contribution.

I encountered some error that I wasn't expecting.  See the details below.


checkpatch:
WARNING: Unexpected sign-offs from developers who are not authors or co-authors or committers: David Marchand <david.marchand@redhat.com>
Lines checked: 77, Warnings: 1, Errors: 0


Please check this out.  If you feel there has been an error, please email aconole@bytheb.org

Thanks,
0-day Robot
Darrell Ball Jan. 16, 2019, 2:57 a.m. UTC | #2 
Hi David

I am resending the patchset with some minor format changes.

Darrell

On Tue, Jan 15, 2019 at 7:52 AM 0-day Robot <robot@bytheb.org> wrote:

> Bleep bloop.  Greetings David Marchand, I am a robot and I have tried out
> your patch.
> Thanks for your contribution.
>
> I encountered some error that I wasn't expecting.  See the details below.
>
>
> checkpatch:
> WARNING: Unexpected sign-offs from developers who are not authors or
> co-authors or committers: David Marchand <david.marchand@redhat.com>
> Lines checked: 77, Warnings: 1, Errors: 0
>
>
> Please check this out.  If you feel there has been an error, please email
> aconole@bytheb.org
>
> Thanks,
> 0-day Robot
> _______________________________________________
> dev mailing list
> dev@openvswitch.org
> https://mail.openvswitch.org/mailman/listinfo/ovs-dev
>
diff mbox series

Patch

diff --git a/lib/conntrack.c b/lib/conntrack.c
index c6864b9..f732b9e 100644
--- a/lib/conntrack.c
+++ b/lib/conntrack.c
@@ -3176,6 +3176,13 @@  repl_ftp_v6_addr(struct dp_packet *pkt, struct ct_addr v6_addr_rep,
     return overall_delta;
 }
 
+/* Increment/decrement a TCP sequence number. */
+static void
+adj_seqnum(ovs_16aligned_be32 *val, int32_t inc)
+{
+    put_16aligned_be32(val, htonl(ntohl(get_16aligned_be32(val)) + inc));
+}
+
 static void
 handle_ftp_ctl(struct conntrack *ct, const struct conn_lookup_ctx *ctx,
                struct dp_packet *pkt, const struct conn *ec, long long now,
@@ -3250,34 +3257,9 @@  handle_ftp_ctl(struct conntrack *ct, const struct conn_lookup_ctx *ctx,
     struct tcp_header *th = dp_packet_l4(pkt);
 
     if (nat && ec->seq_skew != 0) {
-        if (ctx->reply != ec->seq_skew_dir) {
-
-            uint32_t tcp_ack = ntohl(get_16aligned_be32(&th->tcp_ack));
-
-            if ((ec->seq_skew > 0) && (tcp_ack < ec->seq_skew)) {
-                /* Should not be possible; will be marked invalid. */
-                tcp_ack = 0;
-            } else if ((ec->seq_skew < 0) &&
-                       (UINT32_MAX - tcp_ack < -ec->seq_skew)) {
-                tcp_ack = (-ec->seq_skew) - (UINT32_MAX - tcp_ack);
-            } else {
-                tcp_ack -= ec->seq_skew;
-            }
-            ovs_be32 new_tcp_ack = htonl(tcp_ack);
-            put_16aligned_be32(&th->tcp_ack, new_tcp_ack);
-        } else {
-            uint32_t tcp_seq = ntohl(get_16aligned_be32(&th->tcp_seq));
-            if ((ec->seq_skew > 0) && (UINT32_MAX - tcp_seq < ec->seq_skew)) {
-                tcp_seq = ec->seq_skew - (UINT32_MAX - tcp_seq);
-            } else if ((ec->seq_skew < 0) && (tcp_seq < -ec->seq_skew)) {
-                /* Should not be possible; will be marked invalid. */
-                tcp_seq = 0;
-            } else {
-                tcp_seq += ec->seq_skew;
-            }
-            ovs_be32 new_tcp_seq = htonl(tcp_seq);
-            put_16aligned_be32(&th->tcp_seq, new_tcp_seq);
-        }
+        ctx->reply != ec->seq_skew_dir ?
+            adj_seqnum(&th->tcp_ack, -ec->seq_skew) :
+            adj_seqnum(&th->tcp_seq, ec->seq_skew);
     }
 
     th->tcp_csum = 0;