NetworkPkg/TcpDxe: Check TCP payload for release version.

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>
Reviewed-by: Ye Ting <ting.ye@intel.com>
Reviewed-by: Fu Siyuan <siyuan.fu@intel.com>

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
@@ -281,8 +281,11 @@ TcpComputeRtt (
   @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,

@@ -306,7 +309,7 @@ TcpTrimSegment (
 

     Seg->Seq = Seg->End;

     NetbufTrim (Nbuf, Nbuf->TotalSize, NET_BUF_HEAD);

-    return;

+    return 1;

   }

 

   //

@@ -359,7 +362,7 @@ TcpTrimSegment (
     }

   }

 

-  ASSERT (TcpVerifySegment (Nbuf) != 0);

+  return TcpVerifySegment (Nbuf);

 }

 

 /**

@@ -368,14 +371,17 @@ TcpTrimSegment (
   @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);

 }

 

 /**

@@ -421,7 +427,16 @@ TcpDeliverData (
     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)) {

@@ -561,8 +576,11 @@ TcpDeliverData (
   @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

@@ -588,7 +606,7 @@ TcpQueueData (
   if (IsListEmpty (Head)) {

 

     InsertTailList (Head, &Nbuf->List);

-    return;

+    return 1;

   }

 

   //

@@ -615,12 +633,12 @@ TcpQueueData (
     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;

+      }

     }

   }

 

@@ -648,16 +666,20 @@ TcpQueueData (
       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;

 }

 

 

@@ -667,8 +689,11 @@ TcpQueueData (
   @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

@@ -701,9 +726,10 @@ TcpAdjustSndQue (
       continue;

     }

 

-    TcpTrimSegment (Node, Ack, Seg->End);

-    break;

+    return TcpTrimSegment (Node, Ack, Seg->End);

   }

+

+  return 1;

 }

 

 /**

@@ -893,7 +919,15 @@ TcpInput (
 

       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;

     }

@@ -975,7 +1009,15 @@ TcpInput (
           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);

 

@@ -993,9 +1035,16 @@ TcpInput (
         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,

@@ -1081,7 +1130,15 @@ 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

@@ -1256,7 +1313,16 @@ TcpInput (
 

   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) &&

@@ -1489,7 +1555,16 @@ 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;

     }

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
@@ -292,7 +292,11 @@ TcpTransmitSegment (
   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;

 

@@ -634,7 +638,11 @@ TcpGetSegment (
     Nbuf = TcpGetSegmentSock (Tcb, Seq, Len);

   }

 

-  ASSERT (TcpVerifySegment (Nbuf) != 0);

+  if (TcpVerifySegment (Nbuf) == 0) {

+    NetbufFree (Nbuf);

+    return NULL;

+  }

+  

   return Nbuf;

 }

 

@@ -701,7 +709,9 @@ TcpRetransmit (
     return -1;

   }

 

-  ASSERT (TcpVerifySegment (Nbuf) != 0);

+  if (TcpVerifySegment (Nbuf) == 0) {

+    goto OnError;

+  }

 

   if (TcpTransmitSegment (Tcb, Nbuf) != 0) {

     goto OnError;

@@ -886,8 +896,14 @@ TcpToSendData (
     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.

@@ -899,8 +915,7 @@ TcpToSendData (
         Tcb)

         );

 

-      NetbufFree (Nbuf);

-      return Sent;

+      goto OnError;

     }

 

     if (TcpTransmitSegment (Tcb, Nbuf) != 0) {