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...@intel.com>
Cc: Fu Siyuan <siyuan...@intel.com>
Cc: Wang Fan <fan.w...@intel.com>
Contributed-under: TianoCore Contribution Agreement 1.0
Signed-off-by: Wu Jiaxin <jiaxin...@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
_______________________________________________
edk2-devel mailing list
edk2-devel@lists.01.org
https://lists.01.org/mailman/listinfo/edk2-devel
