From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received-SPF: Pass (sender SPF authorized) identity=mailfrom; client-ip=134.134.136.31; helo=mga06.intel.com; envelope-from=jiaxin.wu@intel.com; receiver=edk2-devel@lists.01.org Received: from mga06.intel.com (mga06.intel.com [134.134.136.31]) (using TLSv1.2 with cipher ECDHE-RSA-AES256-GCM-SHA384 (256/256 bits)) (No client certificate requested) by ml01.01.org (Postfix) with ESMTPS id 448E821CB87A5 for ; Mon, 25 Dec 2017 22:45:20 -0800 (PST) X-Amp-Result: SKIPPED(no attachment in message) X-Amp-File-Uploaded: False Received: from fmsmga006.fm.intel.com ([10.253.24.20]) by orsmga104.jf.intel.com with ESMTP/TLS/DHE-RSA-AES256-GCM-SHA384; 25 Dec 2017 22:50:13 -0800 X-ExtLoop1: 1 X-IronPort-AV: E=Sophos;i="5.45,459,1508828400"; d="scan'208";a="190008418" Received: from jiaxinwu-mobl2.ccr.corp.intel.com ([10.239.196.165]) by fmsmga006.fm.intel.com with ESMTP; 25 Dec 2017 22:50:12 -0800 From: Jiaxin Wu To: edk2-devel@lists.01.org Cc: Ye Ting , Fu Siyuan , Wang Fan , Wu Jiaxin Date: Tue, 26 Dec 2017 14:50:11 +0800 Message-Id: <1514271011-9780-1-git-send-email-jiaxin.wu@intel.com> X-Mailer: git-send-email 1.9.5.msysgit.1 Subject: [Patch 3/3] NetworkPkg/TcpDxe: Check TCP payload for release version. X-BeenThere: edk2-devel@lists.01.org X-Mailman-Version: 2.1.23 Precedence: list List-Id: EDK II Development List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , X-List-Received-Date: Tue, 26 Dec 2017 06:45:20 -0000 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 --- 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