Skip to content

Commit

Permalink
datapath-windows: STT reassemble small fix
Browse files Browse the repository at this point in the history
Fixed possible deadlock in case NdisGetDataBuffer fails
Validate the segment length and offset on reassemble to avoid buffer overflow

Signed-off-by: Paul-Daniel Boca <pboca@cloudbasesolutions.com>
Acked-by: Sairam Venugopal <vsairam@vmware.com>
Signed-off-by: Ben Pfaff <blp@ovn.org>
  • Loading branch information
Paul Boca authored and blp committed Jun 7, 2016
1 parent bfc27f6 commit de88569
Show file tree
Hide file tree
Showing 2 changed files with 14 additions and 3 deletions.
16 changes: 13 additions & 3 deletions datapath-windows/ovsext/Stt.c
Original file line number Diff line number Diff line change
Expand Up @@ -612,9 +612,6 @@ OvsSttReassemble(POVS_SWITCH_CONTEXT switchContext,
SttHdr *sttHdr = NULL;
sourceNb = NET_BUFFER_LIST_FIRST_NB(curNbl);

/* XXX optimize this lock */
NdisAcquireSpinLock(&OvsSttSpinLock);

/* If this is the first fragment, copy the STT header */
if (segOffset == 0) {
sttHdr = NdisGetDataBuffer(sourceNb, sizeof(SttHdr), &stt, 1, 0);
Expand All @@ -626,6 +623,14 @@ OvsSttReassemble(POVS_SWITCH_CONTEXT switchContext,
startOffset = startOffset + STT_HDR_LEN;
}

if (offset + fragmentLength > innerPacketLen) {
// avoid buffer overflow on copy
return NULL;
}

/* XXX optimize this lock */
NdisAcquireSpinLock(&OvsSttSpinLock);

/* Lookup fragment */
OVS_STT_PKT_KEY pktKey = OvsGeneratePacketKey(ipHdr, tcp);
UINT32 hash = OvsSttGetPktHash(&pktKey);
Expand All @@ -652,6 +657,7 @@ OvsSttReassemble(POVS_SWITCH_CONTEXT switchContext,
}

/* Copy the data from Source to new buffer */
entry->allocatedLen = innerPacketLen;
entry->packetBuf = OvsAllocateMemoryWithTag(innerPacketLen,
OVS_STT_POOL_TAG);
if (OvsGetPacketBytes(curNbl, fragmentLength, startOffset,
Expand All @@ -664,6 +670,10 @@ OvsSttReassemble(POVS_SWITCH_CONTEXT switchContext,
InsertHeadList(&OvsSttPktFragHash[hash & STT_HASH_TABLE_MASK],
&entry->link);
} else {
if (offset + fragmentLength > pktFragEntry->allocatedLen) {
// 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);
Expand Down
1 change: 1 addition & 0 deletions datapath-windows/ovsext/Stt.h
Original file line number Diff line number Diff line change
Expand Up @@ -68,6 +68,7 @@ typedef struct _OVS_STT_PKT_ENTRY {
OVS_STT_PKT_KEY ovsPktKey;
UINT64 timeout;
UINT32 recvdLen;
UINT32 allocatedLen;
SttHdr sttHdr;
PCHAR packetBuf;
LIST_ENTRY link;
Expand Down

0 comments on commit de88569

Please sign in to comment.