* [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
* Re: [Patch 3/3] NetworkPkg/TcpDxe: Check TCP payload for release version. 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 1 sibling, 1 reply; 4+ messages in thread From: Fu, Siyuan @ 2017-12-27 3:03 UTC (permalink / raw) To: Wu, Jiaxin, edk2-devel@lists.01.org; +Cc: Ye, Ting, Wang, Fan Hi, Jiaxin I think it's better to return a Boolean type than int 0-1 value in TcpTrimSegment(). Other part of good to me. Reviewed-by: Fu Siyuan <siyuan.fu@intel.com> > -----Original Message----- > From: Wu, Jiaxin > Sent: Tuesday, December 26, 2017 2:50 PM > To: edk2-devel@lists.01.org > Cc: Ye, Ting <ting.ye@intel.com>; Fu, Siyuan <siyuan.fu@intel.com>; Wang, > Fan <fan.wang@intel.com>; Wu, Jiaxin <jiaxin.wu@intel.com> > Subject: [Patch 3/3] 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> > --- > 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 [flat|nested] 4+ messages in thread
* Re: [Patch 3/3] NetworkPkg/TcpDxe: Check TCP payload for release version. 2017-12-27 3:03 ` Fu, Siyuan @ 2017-12-27 8:25 ` Wu, Jiaxin 0 siblings, 0 replies; 4+ messages in thread From: Wu, Jiaxin @ 2017-12-27 8:25 UTC (permalink / raw) To: Fu, Siyuan, edk2-devel@lists.01.org; +Cc: Ye, Ting, Wang, Fan After talked with Siyuan, we agree to use the int 0-1 value instead of Boolean type so as to keep the same coding style in TcpInput.c. Thanks, Jiaxin > -----Original Message----- > From: Fu, Siyuan > Sent: Wednesday, December 27, 2017 11:03 AM > To: Wu, Jiaxin <jiaxin.wu@intel.com>; edk2-devel@lists.01.org > Cc: Ye, Ting <ting.ye@intel.com>; Wang, Fan <fan.wang@intel.com> > Subject: RE: [Patch 3/3] NetworkPkg/TcpDxe: Check TCP payload for release > version. > > Hi, Jiaxin > > I think it's better to return a Boolean type than int 0-1 value in > TcpTrimSegment(). > Other part of good to me. > > > Reviewed-by: Fu Siyuan <siyuan.fu@intel.com> > > > > > -----Original Message----- > > From: Wu, Jiaxin > > Sent: Tuesday, December 26, 2017 2:50 PM > > To: edk2-devel@lists.01.org > > Cc: Ye, Ting <ting.ye@intel.com>; Fu, Siyuan <siyuan.fu@intel.com>; Wang, > > Fan <fan.wang@intel.com>; Wu, Jiaxin <jiaxin.wu@intel.com> > > Subject: [Patch 3/3] 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> > > --- > > 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 [flat|nested] 4+ messages in thread
* Re: [Patch 3/3] NetworkPkg/TcpDxe: Check TCP payload for release version. 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 3:11 ` Ye, Ting 1 sibling, 0 replies; 4+ messages in thread From: Ye, Ting @ 2017-12-27 3:11 UTC (permalink / raw) To: Wu, Jiaxin, edk2-devel@lists.01.org; +Cc: Fu, Siyuan, Wang, Fan Reviewed-by: Ye Ting <ting.ye@intel.com> -----Original Message----- From: Wu, Jiaxin Sent: Tuesday, December 26, 2017 2:50 PM To: edk2-devel@lists.01.org Cc: Ye, Ting <ting.ye@intel.com>; Fu, Siyuan <siyuan.fu@intel.com>; Wang, Fan <fan.wang@intel.com>; Wu, Jiaxin <jiaxin.wu@intel.com> Subject: [Patch 3/3] 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> --- 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 [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