Just in case it¹s required for V2:
Acked-by: Sairam Venugopal <vsai...@vmware.com>
On 6/6/16, 9:45 AM, "Paul Boca" <pb...@cloudbasesolutions.com> wrote:
>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 <pb...@cloudbasesolutions.com>
>Acked-by: Sairam Venugopal <vsai...@vmware.com>
>---
>V2: No changes
>---
> datapath-windows/ovsext/Stt.c | 16 +++++++++++++---
> datapath-windows/ovsext/Stt.h | 1 +
> 2 files changed, 14 insertions(+), 3 deletions(-)
>
>diff --git a/datapath-windows/ovsext/Stt.c b/datapath-windows/ovsext/Stt.c
>index 9345255..fd9b32b 100644
>--- a/datapath-windows/ovsext/Stt.c
>+++ b/datapath-windows/ovsext/Stt.c
>@@ -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);
>@@ -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);
>@@ -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,
>@@ -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);
>diff --git a/datapath-windows/ovsext/Stt.h b/datapath-windows/ovsext/Stt.h
>index a3e3915..faa00d7 100644
>--- a/datapath-windows/ovsext/Stt.h
>+++ b/datapath-windows/ovsext/Stt.h
>@@ -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;
>--
>2.7.2.windows.1
>_______________________________________________
>dev mailing list
>dev@openvswitch.org
>https://urldefense.proofpoint.com/v2/url?u=http-3A__openvswitch.org_mailma
>n_listinfo_dev&d=CwIGaQ&c=Sqcl0Ez6M0X8aeM67LKIiDJAXVeAw-YihVMNtXt-uEs&r=Dc
>ruz40PROJ40ROzSpxyQSLw6fcrOWpJgEcEmNR3JEQ&m=KBEW9Ar6kdibHQGGkQYfU6dTh3ygBF
>z5PN5DJxfpGaA&s=53cmZhcWT6vigu_gFHbyFCXT7xeTL35GUSxob3A95EA&e=
_______________________________________________
dev mailing list
dev@openvswitch.org
http://openvswitch.org/mailman/listinfo/dev
