Only update SACK/DSACK lists when a non-empty segment was received.

This fixes hitting a KASSERT with a valid packet exchange.

Reviewed by:		rrs@, Richard Scheffenegger
MFC after:		3 days
Sponsored by:		Netflix, Inc.
Differential Revision:	https://reviews.freebsd.org/D21567

diff --git a/sys/netinet/tcp_input.c b/sys/netinet/tcp_input.c
index c5259e8b45bf33cef3c49021a4634f65d1eb3f3e..42a48181949b76ab9f9fde3644bfb59cf8304466 100644 (file)
--- a/sys/netinet/tcp_input.c
+++ b/sys/netinet/tcp_input.c
@@ -3045,9 +3045,8 @@ dodata:                                                   /* XXX */
                        thflags = tcp_reass(tp, th, &temp, &tlen, m);
                        tp->t_flags |= TF_ACKNOW;
                }
-               if (tp->t_flags & TF_SACK_PERMIT) {
-                       if (((tlen == 0) && (save_tlen > 0) &&
-                           (SEQ_LT(save_start, save_rnxt)))) {
+               if ((tp->t_flags & TF_SACK_PERMIT) && (save_tlen > 0)) {
+                       if ((tlen == 0) && (SEQ_LT(save_start, save_rnxt))) {
                                /*
                                 * DSACK actually handled in the fastpath
                                 * above.
@@ -3055,20 +3054,20 @@ dodata:                                                 /* XXX */
                                tcp_update_sack_list(tp, save_start,
                                    save_start + save_tlen);
                        } else if ((tlen > 0) && SEQ_GT(tp->rcv_nxt, save_rnxt)) {
-                               /*
-                                * Cleaning sackblks by using zero length
-                                * update.
-                                */
                                if ((tp->rcv_numsacks >= 1) &&
                                    (tp->sackblks[0].end == save_start)) {
-                                       /* partial overlap, recorded at todrop above */
-                                       tcp_update_sack_list(tp, tp->sackblks[0].start,
+                                       /*
+                                        * Partial overlap, recorded at todrop
+                                        * above.
+                                        */
+                                       tcp_update_sack_list(tp,
+                                           tp->sackblks[0].start,
                                            tp->sackblks[0].end);
                                } else {
                                        tcp_update_dsack_list(tp, save_start,
                                            save_start + save_tlen);
                                }
-                       } else if ((tlen > 0) && (tlen >= save_tlen)) {
+                       } else if (tlen >= save_tlen) {
                                /* Update of sackblks. */
                                tcp_update_dsack_list(tp, save_start,
                                    save_start + save_tlen);

diff --git a/sys/netinet/tcp_stacks/rack.c b/sys/netinet/tcp_stacks/rack.c
index 39f81a2f5d26cc7c2d83674daf8eb86b7a58c3c7..d70455c2f191c70e3a67e43fc66d9696fd7d07dd 100644 (file)
--- a/sys/netinet/tcp_stacks/rack.c
+++ b/sys/netinet/tcp_stacks/rack.c
@@ -4942,35 +4942,36 @@ dodata:                         /* XXX */
                        thflags = tcp_reass(tp, th, &temp, &tlen, m);
                        tp->t_flags |= TF_ACKNOW;
                }
-               if (((tlen == 0) && (save_tlen > 0) &&
-                   (SEQ_LT(save_start, save_rnxt)))) {
-                       /*
-                        * DSACK actually handled in the fastpath
-                        * above.
-                        */
-                       tcp_update_sack_list(tp, save_start,
-                           save_start + save_tlen);
-               } else if ((tlen > 0) && SEQ_GT(tp->rcv_nxt, save_rnxt)) {
-                       /*
-                        * Cleaning sackblks by using zero length
-                        * update.
-                        */
-                       if ((tp->rcv_numsacks >= 1) &&
-                           (tp->sackblks[0].end == save_start)) {
-                               /* partial overlap, recorded at todrop above */
-                               tcp_update_sack_list(tp, tp->sackblks[0].start,
-                                   tp->sackblks[0].end);
-                       } else {
+               if ((tp->t_flags & TF_SACK_PERMIT) && (save_tlen > 0)) {
+                       if ((tlen == 0) && (SEQ_LT(save_start, save_rnxt))) {
+                               /*
+                                * DSACK actually handled in the fastpath
+                                * above.
+                                */
+                               tcp_update_sack_list(tp, save_start,
+                                   save_start + save_tlen);
+                       } else if ((tlen > 0) && SEQ_GT(tp->rcv_nxt, save_rnxt)) {
+                               if ((tp->rcv_numsacks >= 1) &&
+                                   (tp->sackblks[0].end == save_start)) {
+                                       /*
+                                        * Partial overlap, recorded at todrop
+                                        * above.
+                                        */
+                                       tcp_update_sack_list(tp,
+                                           tp->sackblks[0].start,
+                                           tp->sackblks[0].end);
+                               } else {
+                                       tcp_update_dsack_list(tp, save_start,
+                                           save_start + save_tlen);
+                               }
+                       } else if (tlen >= save_tlen) {
+                               /* Update of sackblks. */
                                tcp_update_dsack_list(tp, save_start,
                                    save_start + save_tlen);
+                       } else if (tlen > 0) {
+                               tcp_update_dsack_list(tp, save_start,
+                                   save_start + tlen);
                        }
-               } else if ((tlen > 0) && (tlen >= save_tlen)) {
-                       /* Update of sackblks. */
-                       tcp_update_dsack_list(tp, save_start,
-                           save_start + save_tlen);
-               } else if (tlen > 0) {
-                       tcp_update_dsack_list(tp, save_start,
-                           save_start + tlen);
                }
        } else {
                m_freem(m);