From 786a4d1921d61d809af7f12020264327df409f3b Mon Sep 17 00:00:00 2001 From: Jiaxin Wu Date: Fri, 22 Dec 2017 14:53:03 +0800 Subject: [PATCH] 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 Cc: Fu Siyuan Cc: Wang Fan Contributed-under: TianoCore Contribution Agreement 1.0 Signed-off-by: Wu Jiaxin Reviewed-by: Ye Ting Reviewed-by: Fu Siyuan --- 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 f8845dca47..92a0ab8c35 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 a7e59f0ed6..1697514a92 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) {