public inbox for devel@edk2.groups.io
 help / color / mirror / Atom feed
* [Patch 3/3] NetworkPkg/TcpDxe: Check TCP payload for release version.
@ 2017-12-26  6:50 Jiaxin Wu
  2017-12-27  3:03 ` Fu, Siyuan
  2017-12-27  3:11 ` Ye, Ting
  0 siblings, 2 replies; 4+ messages in thread
From: Jiaxin Wu @ 2017-12-26  6:50 UTC (permalink / raw)
  To: edk2-devel; +Cc: Ye Ting, Fu Siyuan, Wang Fan, Wu Jiaxin

TCP payload check is implemented by TcpVerifySegment(), but all the function
calls of TcpVerifySegment() are placed in ASSERT(), which is only valid for
debug version:
  ASSERT (TcpVerifySegment (Nbuf) != 0);

This patch is to enable the check for release version.

Cc: Ye Ting <ting.ye@intel.com>
Cc: Fu Siyuan <siyuan.fu@intel.com>
Cc: Wang Fan <fan.wang@intel.com>
Contributed-under: TianoCore Contribution Agreement 1.0
Signed-off-by: Wu Jiaxin <jiaxin.wu@intel.com>
---
 NetworkPkg/TcpDxe/TcpInput.c  | 125 +++++++++++++++++++++++++++++++++---------
 NetworkPkg/TcpDxe/TcpOutput.c |  29 +++++++---
 2 files changed, 122 insertions(+), 32 deletions(-)

diff --git a/NetworkPkg/TcpDxe/TcpInput.c b/NetworkPkg/TcpDxe/TcpInput.c
index f8845dc..92a0ab8 100644
--- a/NetworkPkg/TcpDxe/TcpInput.c
+++ b/NetworkPkg/TcpDxe/TcpInput.c
@@ -279,12 +279,15 @@ TcpComputeRtt (
 
   @param[in]  Nbuf     The buffer that contains a received TCP segment without an IP header.
   @param[in]  Left     The sequence number of the window's left edge.
   @param[in]  Right    The sequence number of the window's right edge.
 
+  @retval     0        The segment is broken.
+  @retval     1        The segment is in good shape.
+
 **/
-VOID
+INTN
 TcpTrimSegment (
   IN NET_BUF   *Nbuf,
   IN TCP_SEQNO Left,
   IN TCP_SEQNO Right
   )
@@ -304,11 +307,11 @@ TcpTrimSegment (
     TCP_CLEAR_FLG (Seg->Flag, TCP_FLG_SYN);
     TCP_CLEAR_FLG (Seg->Flag, TCP_FLG_FIN);
 
     Seg->Seq = Seg->End;
     NetbufTrim (Nbuf, Nbuf->TotalSize, NET_BUF_HEAD);
-    return;
+    return 1;
   }
 
   //
   // Adjust the buffer header
   //
@@ -357,27 +360,30 @@ TcpTrimSegment (
     if (Drop != 0) {
       NetbufTrim (Nbuf, Drop, NET_BUF_TAIL);
     }
   }
 
-  ASSERT (TcpVerifySegment (Nbuf) != 0);
+  return TcpVerifySegment (Nbuf);
 }
 
 /**
   Trim off the data outside the tcb's receive window.
 
   @param[in]  Tcb      Pointer to the TCP_CB of this TCP instance.
   @param[in]  Nbuf     Pointer to the NET_BUF containing the received tcp segment.
 
+  @retval     0        The segment is broken.
+  @retval     1        The segment is in good shape.
+
 **/
-VOID
+INTN
 TcpTrimInWnd (
   IN TCP_CB  *Tcb,
   IN NET_BUF *Nbuf
   )
 {
-  TcpTrimSegment (Nbuf, Tcb->RcvNxt, Tcb->RcvWl2 + Tcb->RcvWnd);
+  return TcpTrimSegment (Nbuf, Tcb->RcvNxt, Tcb->RcvWl2 + Tcb->RcvWnd);
 }
 
 /**
   Process the data and FIN flag, and check whether to deliver
   data to the socket layer.
@@ -419,11 +425,20 @@ TcpDeliverData (
 
   while (Entry != &Tcb->RcvQue) {
     Nbuf  = NET_LIST_USER_STRUCT (Entry, NET_BUF, List);
     Seg   = TCPSEG_NETBUF (Nbuf);
 
-    ASSERT (TcpVerifySegment (Nbuf) != 0);
+    if (TcpVerifySegment (Nbuf) == 0) {
+      DEBUG (
+        (EFI_D_ERROR,
+        "TcpToSendData: discard a broken segment for TCB %p\n",
+        Tcb)
+        );
+      NetbufFree (Nbuf);
+      return -1;
+    }
+    
     ASSERT (Nbuf->Tcp == NULL);
 
     if (TCP_SEQ_GT (Seg->Seq, Seq)) {
       break;
     }
@@ -559,12 +574,15 @@ TcpDeliverData (
   Store the data into the reassemble queue.
 
   @param[in, out]  Tcb   Pointer to the TCP_CB of this TCP instance.
   @param[in]       Nbuf  Pointer to the buffer containing the data to be queued.
 
+  @retval          0     An error condition occurred.
+  @retval          1     No error occurred to queue data.
+
 **/
-VOID
+INTN
 TcpQueueData (
   IN OUT TCP_CB  *Tcb,
   IN     NET_BUF *Nbuf
   )
 {
@@ -586,11 +604,11 @@ TcpQueueData (
   // no out-of-order segments are received.
   //
   if (IsListEmpty (Head)) {
 
     InsertTailList (Head, &Nbuf->List);
-    return;
+    return 1;
   }
 
   //
   // Find the point to insert the buffer
   //
@@ -613,16 +631,16 @@ TcpQueueData (
     Node = NET_LIST_USER_STRUCT (Prev, NET_BUF, List);
 
     if (TCP_SEQ_LT (Seg->Seq, TCPSEG_NETBUF (Node)->End)) {
 
       if (TCP_SEQ_LEQ (Seg->End, TCPSEG_NETBUF (Node)->End)) {
-
-        NetbufFree (Nbuf);
-        return;
+        return 1;
       }
 
-      TcpTrimSegment (Nbuf, TCPSEG_NETBUF (Node)->End, Seg->End);
+      if (TcpTrimSegment (Nbuf, TCPSEG_NETBUF (Node)->End, Seg->End) == 0) {
+        return 0;
+      }
     }
   }
 
   InsertHeadList (Prev, &Nbuf->List);
 
@@ -646,31 +664,38 @@ TcpQueueData (
     if (TCP_SEQ_LT (TCPSEG_NETBUF (Node)->Seq, Seg->End)) {
 
       if (TCP_SEQ_LEQ (TCPSEG_NETBUF (Node)->Seq, Seg->Seq)) {
 
         RemoveEntryList (&Nbuf->List);
-        NetbufFree (Nbuf);
-        return;
+        return 1;
       }
 
-      TcpTrimSegment (Nbuf, Seg->Seq, TCPSEG_NETBUF (Node)->Seq);
+      if (TcpTrimSegment (Nbuf, Seg->Seq, TCPSEG_NETBUF (Node)->Seq) == 0) {
+        RemoveEntryList (&Nbuf->List);
+        return 0;
+      }
       break;
     }
 
     Cur = Cur->ForwardLink;
   }
+
+  return 1;
 }
 
 
 /**
   Adjust the send queue or the retransmit queue.
 
   @param[in]  Tcb      Pointer to the TCP_CB of this TCP instance.
   @param[in]  Ack      The acknowledge seuqence number of the received segment.
 
+  @retval          0     An error condition occurred.
+  @retval          1     No error occurred.
+
 **/
-VOID
+INTN
 TcpAdjustSndQue (
   IN TCP_CB    *Tcb,
   IN TCP_SEQNO Ack
   )
 {
@@ -699,13 +724,14 @@ TcpAdjustSndQue (
       RemoveEntryList (&Node->List);
       NetbufFree (Node);
       continue;
     }
 
-    TcpTrimSegment (Node, Ack, Seg->End);
-    break;
+    return TcpTrimSegment (Node, Ack, Seg->End);
   }
+
+  return 1;
 }
 
 /**
   Process the received TCP segments.
 
@@ -891,11 +917,19 @@ TcpInput (
       TcpInitTcbLocal (Tcb);
       TcpInitTcbPeer (Tcb, Seg, &Option);
 
       TcpSetState (Tcb, TCP_SYN_RCVD);
       TcpSetTimer (Tcb, TCP_TIMER_CONNECT, Tcb->ConnectTimeout);
-      TcpTrimInWnd (Tcb, Nbuf);
+      if (TcpTrimInWnd (Tcb, Nbuf) == 0) {
+        DEBUG (
+          (EFI_D_ERROR,
+          "TcpInput: discard a broken segment for TCB %p\n",
+          Tcb)
+          );
+
+        goto DISCARD;
+      }
 
       goto StepSix;
     }
 
     goto DISCARD;
@@ -973,11 +1007,19 @@ TcpInput (
 
           TcpComputeRtt (Tcb, Tcb->RttMeasure);
           TCP_CLEAR_FLG (Tcb->CtrlFlag, TCP_CTRL_RTT_ON);
         }
 
-        TcpTrimInWnd (Tcb, Nbuf);
+        if (TcpTrimInWnd (Tcb, Nbuf) == 0) {
+          DEBUG (
+            (EFI_D_ERROR,
+            "TcpInput: discard a broken segment for TCB %p\n",
+            Tcb)
+            );
+
+          goto DISCARD;
+        }
 
         TCP_SET_FLG (Tcb->CtrlFlag, TCP_CTRL_ACK_NOW);
 
         DEBUG (
           (EFI_D_NET,
@@ -991,13 +1033,20 @@ TcpInput (
         // Received a SYN segment without ACK, simultanous open.
         //
         TcpSetState (Tcb, TCP_SYN_RCVD);
 
         ASSERT (Tcb->SndNxt == Tcb->Iss + 1);
-        TcpAdjustSndQue (Tcb, Tcb->SndNxt);
 
-        TcpTrimInWnd (Tcb, Nbuf);
+        if (TcpAdjustSndQue (Tcb, Tcb->SndNxt) == 0 || TcpTrimInWnd (Tcb, Nbuf) == 0) {
+          DEBUG (
+            (EFI_D_ERROR,
+            "TcpInput: discard a broken segment for TCB %p\n",
+            Tcb)
+            );
+
+          goto DISCARD;
+        }
 
         DEBUG (
           (EFI_D_WARN,
           "TcpInput: simultaneous open for TCB %p in SYN_SENT\n",
           Tcb)
@@ -1079,11 +1128,19 @@ TcpInput (
   }
 
   //
   // Trim the data and flags.
   //
-  TcpTrimInWnd (Tcb, Nbuf);
+  if (TcpTrimInWnd (Tcb, Nbuf) == 0) {
+    DEBUG (
+      (EFI_D_ERROR,
+      "TcpInput: discard a broken segment for TCB %p\n",
+      Tcb)
+      );
+
+    goto DISCARD;
+  }
 
   //
   // Third step: Check security and precedence, Ignored
   //
 
@@ -1254,11 +1311,20 @@ TcpInput (
     TcpFastRecover (Tcb, Seg);
   }
 
   if (TCP_SEQ_GT (Seg->Ack, Tcb->SndUna)) {
 
-    TcpAdjustSndQue (Tcb, Seg->Ack);
+    if (TcpAdjustSndQue (Tcb, Seg->Ack) == 0) {
+      DEBUG (
+        (EFI_D_ERROR,
+        "TcpInput: discard a broken segment for TCB %p\n",
+        Tcb)
+        );
+
+      goto DISCARD;
+    }
+    
     Tcb->SndUna = Seg->Ack;
 
     if (TCP_FLG_ON (Tcb->CtrlFlag, TCP_CTRL_SND_URG) &&
         TCP_SEQ_LT (Tcb->SndUp, Seg->Ack))
     {
@@ -1487,11 +1553,20 @@ StepSix:
         );
 
       goto RESET_THEN_DROP;
     }
 
-    TcpQueueData (Tcb, Nbuf);
+    if (TcpQueueData (Tcb, Nbuf) == 0) {
+      DEBUG (
+        (EFI_D_ERROR,
+        "TcpInput: discard a broken segment for TCB %p\n",
+        Tcb)
+        );
+
+      goto DISCARD;
+    }
+    
     if (TcpDeliverData (Tcb) == -1) {
       goto RESET_THEN_DROP;
     }
 
     if (!IsListEmpty (&Tcb->RcvQue)) {
diff --git a/NetworkPkg/TcpDxe/TcpOutput.c b/NetworkPkg/TcpDxe/TcpOutput.c
index a7e59f0..1697514 100644
--- a/NetworkPkg/TcpDxe/TcpOutput.c
+++ b/NetworkPkg/TcpDxe/TcpOutput.c
@@ -290,11 +290,15 @@ TcpTransmitSegment (
   TCP_HEAD  *Head;
   TCP_SEG   *Seg;
   BOOLEAN   Syn;
   UINT32    DataLen;
 
-  ASSERT ((Nbuf != NULL) && (Nbuf->Tcp == NULL) && (TcpVerifySegment (Nbuf) != 0));
+  ASSERT ((Nbuf != NULL) && (Nbuf->Tcp == NULL));
+
+  if (TcpVerifySegment (Nbuf) == 0) {
+    return -1; 
+  }
 
   DataLen = Nbuf->TotalSize;
 
   Seg     = TCPSEG_NETBUF (Nbuf);
   Syn     = TCP_FLG_ON (Seg->Flag, TCP_FLG_SYN);
@@ -632,11 +636,15 @@ TcpGetSegment (
   } else {
 
     Nbuf = TcpGetSegmentSock (Tcb, Seq, Len);
   }
 
-  ASSERT (TcpVerifySegment (Nbuf) != 0);
+  if (TcpVerifySegment (Nbuf) == 0) {
+    NetbufFree (Nbuf);
+    return NULL;
+  }
+  
   return Nbuf;
 }
 
 /**
   Retransmit the segment from sequence Seq.
@@ -699,11 +707,13 @@ TcpRetransmit (
   Nbuf = TcpGetSegmentSndQue (Tcb, Seq, Len);
   if (Nbuf == NULL) {
     return -1;
   }
 
-  ASSERT (TcpVerifySegment (Nbuf) != 0);
+  if (TcpVerifySegment (Nbuf) == 0) {
+    goto OnError;
+  }
 
   if (TcpTransmitSegment (Tcb, Nbuf) != 0) {
     goto OnError;
   }
 
@@ -884,12 +894,18 @@ TcpToSendData (
 
     Seg->Seq  = Seq;
     Seg->End  = End;
     Seg->Flag = Flag;
 
-    ASSERT (TcpVerifySegment (Nbuf) != 0);
-    ASSERT (TcpCheckSndQue (&Tcb->SndQue) != 0);
+    if (TcpVerifySegment (Nbuf) == 0 || TcpCheckSndQue (&Tcb->SndQue) == 0) {
+      DEBUG (
+        (EFI_D_ERROR,
+        "TcpToSendData: discard a broken segment for TCB %p\n",
+        Tcb)
+        );
+      goto OnError;
+    }
 
     //
     // Don't send an empty segment here.
     //
     if (Seg->End == Seg->Seq) {
@@ -897,12 +913,11 @@ TcpToSendData (
         (EFI_D_WARN,
         "TcpToSendData: created a empty segment for TCB %p, free it now\n",
         Tcb)
         );
 
-      NetbufFree (Nbuf);
-      return Sent;
+      goto OnError;
     }
 
     if (TcpTransmitSegment (Tcb, Nbuf) != 0) {
       NetbufTrim (Nbuf, (Nbuf->Tcp->HeadLen << 2), NET_BUF_HEAD);
       Nbuf->Tcp = NULL;
-- 
1.9.5.msysgit.1



^ permalink raw reply related	[flat|nested] 4+ messages in thread

end of thread, other threads:[~2017-12-27  8:20 UTC | newest]

Thread overview: 4+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
2017-12-26  6:50 [Patch 3/3] NetworkPkg/TcpDxe: Check TCP payload for release version Jiaxin Wu
2017-12-27  3:03 ` Fu, Siyuan
2017-12-27  8:25   ` Wu, Jiaxin
2017-12-27  3:11 ` Ye, Ting

This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox