[ovs-dev] [PATCH V3] datapath-windows: Handle possible NULL pointer dereference in STT
Paul Boca
pboca at cloudbasesolutions.com
Wed Jun 22 00:42:29 UTC 2016
Check if OvsAllocatememoryWithTag succeeded or not.
In case of failure propagate cleanup and return.
---
V2: Checked also NdisGetDataBuffer and MmGetSystemAddressForMdlSafe
if they return NULL and handle the error
V3: Don't deallocate and remove failed packets on STT reassembly.
They will be retransmited by sender or removed by the cleanup thread.
---
datapath-windows/ovsext/Stt.c | 50 +++++++++++++++++++++++++++++++++----------
1 file changed, 39 insertions(+), 11 deletions(-)
diff --git a/datapath-windows/ovsext/Stt.c b/datapath-windows/ovsext/Stt.c
index 86a719d..b5d2513 100644
--- a/datapath-windows/ovsext/Stt.c
+++ b/datapath-windows/ovsext/Stt.c
@@ -191,6 +191,10 @@ OvsDoEncapStt(POVS_VPORT_ENTRY vport,
bufferStart = (PUINT8)MmGetSystemAddressForMdlSafe(curMdl,
LowPagePriority);
+ if (bufferStart == NULL) {
+ status = NDIS_STATUS_RESOURCES;
+ goto ret_error;
+ }
bufferStart += NET_BUFFER_CURRENT_MDL_OFFSET(curNb);
if (layers->isIPv4) {
@@ -402,6 +406,9 @@ OvsValidateTCPChecksum(PNET_BUFFER_LIST curNbl, PNET_BUFFER curNb)
EthHdr *eth = (EthHdr *)NdisGetDataBuffer(curNb, sizeof(EthHdr),
NULL, 1, 0);
+ if (eth == NULL) {
+ return NDIS_STATUS_RESOURCES;
+ }
if (eth->Type == ntohs(NDIS_ETH_TYPE_IPV4)) {
IPHdr *ip = (IPHdr *)((PCHAR)eth + sizeof *eth);
@@ -641,7 +648,7 @@ OvsSttReassemble(POVS_SWITCH_CONTEXT switchContext,
POVS_STT_PKT_ENTRY entry;
entry = OvsAllocateMemoryWithTag(sizeof(OVS_STT_PKT_ENTRY),
OVS_STT_POOL_TAG);
- if (NULL == entry) {
+ if (entry == NULL) {
goto handle_error;
}
RtlZeroMemory(entry, sizeof (OVS_STT_PKT_ENTRY));
@@ -666,7 +673,7 @@ OvsSttReassemble(POVS_SWITCH_CONTEXT switchContext,
entry->allocatedLen = innerPacketLen;
entry->packetBuf = OvsAllocateMemoryWithTag(innerPacketLen,
OVS_STT_POOL_TAG);
- if (NULL == entry->packetBuf) {
+ if (entry->packetBuf == NULL) {
OvsFreeMemoryWithTag(entry, OVS_STT_POOL_TAG);
goto handle_error;
}
@@ -684,9 +691,6 @@ OvsSttReassemble(POVS_SWITCH_CONTEXT switchContext,
// don't copy more than it is allocated
goto handle_error;
}
- /* Add to recieved length to identify if this is the last fragment */
- pktFragEntry->recvdLen += fragmentLength;
- lastPacket = (pktFragEntry->recvdLen == innerPacketLen);
if (segOffset == 0) {
pktFragEntry->sttHdr = *sttHdr;
@@ -699,8 +703,13 @@ OvsSttReassemble(POVS_SWITCH_CONTEXT switchContext,
if (OvsGetPacketBytes(curNbl, fragmentLength, startOffset,
pktFragEntry->packetBuf + offset) == NULL) {
OVS_LOG_ERROR("Error when obtaining bytes from Packet");
+ /* Remove from list and free the allocate buffers */
goto handle_error;
}
+
+ /* Add to received length to identify if this is the last fragment */
+ pktFragEntry->recvdLen += fragmentLength;
+ lastPacket = (pktFragEntry->recvdLen == innerPacketLen);
}
handle_error:
@@ -744,7 +753,7 @@ handle_error:
*----------------------------------------------------------------------------
*/
NDIS_STATUS
-OvsDecapSetOffloads(PNET_BUFFER_LIST *curNbl,
+OvsDecapSetOffloads(PNET_BUFFER_LIST *curNbl,
SttHdr *sttHdr,
OVS_PACKET_HDR_INFO *layers)
{
@@ -804,6 +813,9 @@ OvsDecapSetOffloads(PNET_BUFFER_LIST *curNbl,
buf = (PUINT8)MmGetSystemAddressForMdlSafe(curMdl,
LowPagePriority);
+ if (buf == NULL) {
+ return NDIS_STATUS_RESOURCES;
+ }
buf += NET_BUFFER_CURRENT_MDL_OFFSET(curNb);
// apply pseudo checksum on extracted packet
@@ -883,7 +895,9 @@ OvsDecapStt(POVS_SWITCH_CONTEXT switchContext,
ipHdr = NdisGetDataBuffer(curNb, sizeof *ipHdr, (PVOID) &ipBuf,
1 /*no align*/, 0);
- ASSERT(ipHdr);
+ if (ipHdr == NULL) {
+ return NDIS_STATUS_RESOURCES;
+ }
TCPHdr *tcp = (TCPHdr *)((PCHAR)ipHdr + ipHdr->ihl * 4);
@@ -913,6 +927,9 @@ OvsDecapStt(POVS_SWITCH_CONTEXT switchContext,
/* STT Header */
sttHdr = NdisGetDataBuffer(curNb, sizeof *sttHdr,
(PVOID) &sttBuf, 1 /*no align*/, 0);
+ if (sttHdr == NULL) {
+ return NDIS_STATUS_RESOURCES;
+ }
/* Skip stt header, DataOffset points to inner pkt now. */
hdrLen = STT_HDR_LEN;
NdisAdvanceNetBufferDataStart(curNb, hdrLen, FALSE, NULL);
@@ -929,8 +946,8 @@ OvsDecapStt(POVS_SWITCH_CONTEXT switchContext,
status = NdisRetreatNetBufferDataStart(curNb, advanceCnt, 0, NULL);
if (status != NDIS_STATUS_SUCCESS) {
- OvsCompleteNBL(switchContext, *newNbl, TRUE);
- return NDIS_STATUS_FAILURE;
+ status = NDIS_STATUS_FAILURE;
+ goto dropNbl;
}
ASSERT(sttHdr);
@@ -949,8 +966,8 @@ OvsDecapStt(POVS_SWITCH_CONTEXT switchContext,
if (0 != ipHdr->tos) {
status = OvsExtractLayers(*newNbl, &layers);
if (status != NDIS_STATUS_SUCCESS) {
- OvsCompleteNBL(switchContext, *newNbl, TRUE);
- return NDIS_STATUS_FAILURE;
+ status = NDIS_STATUS_FAILURE;
+ goto dropNbl;
}
if (layers.isIPv4) {
@@ -973,6 +990,9 @@ OvsDecapStt(POVS_SWITCH_CONTEXT switchContext,
/* fix IP checksum */
innerIpHdr->check = IPChecksum((UINT8 *)innerIpHdr,
innerIpHdr->ihl * 4, 0);
+ } else {
+ status = NDIS_STATUS_RESOURCES;
+ goto dropNbl;
}
} else if (layers.isIPv6) {
IPv6Hdr ipv6_storage;
@@ -987,6 +1007,9 @@ OvsDecapStt(POVS_SWITCH_CONTEXT switchContext,
| ((innerIpv6Hdr->flow_lbl[0] & 0x3) << 2);
innerIpv6Hdr->flow_lbl[0] = (innerIpv6Hdr->flow_lbl[0] & 0xF)
| ((ipHdr->tos & 0xF) << 4);
+ } else {
+ status = NDIS_STATUS_RESOURCES;
+ goto dropNbl;
}
}
}
@@ -1005,4 +1028,9 @@ OvsDecapStt(POVS_SWITCH_CONTEXT switchContext,
OvsDecapSetOffloads(newNbl, sttHdr, &layers);
return NDIS_STATUS_SUCCESS;
+
+dropNbl:
+ OvsCompleteNBL(switchContext, *newNbl, TRUE);
+ *newNbl = NULL;
+ return status;
}
--
2.7.2.windows.1
More information about the dev
mailing list