| Message ID | 1465903586-17236-1-git-send-email-pboca@cloudbasesolutions.com |
|---|---|
| State | Superseded |
| Headers | show |
I looked briefly over stt.c and there is a list of issues to be fixed if you want to make part of this patch: NdisGetDataBuffer can be null and should be checked (https://msdn.microsoft.com/en-us/library/windows/hardware/ff562631(v=vs.85).aspx ) " The return value can also be NULL due to a low resource condition where a data buffer cannot be mapped. This may occur even if the data is contiguous or the Storage parameter is non-NULL." https://github.com/openvswitch/ovs/blob/master/datapath-windows/ovsext/Stt.c#L403 https://github.com/openvswitch/ovs/blob/master/datapath-windows/ovsext/Stt.c#L877 https://github.com/openvswitch/ovs/blob/master/datapath-windows/ovsext/Stt.c#L907 https://github.com/openvswitch/ovs/blob/master/datapath-windows/ovsext/Stt.c#L972-L983 needs an else branch https://github.com/openvswitch/ovs/blob/master/datapath-windows/ovsext/Stt.c#L957-L969 needs an else branch MmGetSystemAddressForMdlSafe (https://msdn.microsoft.com/en-us/library/windows/hardware/ff554559(v=vs.85).aspx) should be checked: https://github.com/openvswitch/ovs/blob/master/datapath-windows/ovsext/Stt.c#L192 https://github.com/openvswitch/ovs/blob/master/datapath-windows/ovsext/Stt.c#L798 https://github.com/openvswitch/ovs/blob/master/datapath-windows/ovsext/Stt.c#L666-L670 in the case it fails entry->packetBuf, entry should be freed I think if https://github.com/openvswitch/ovs/blob/master/datapath-windows/ovsext/Stt.c#L691-L696 fails we should free pktFragEntry->packetBuf and pktFragEntry. Also comment inlined. Thanks, Alin. > -----Mesaj original----- > De la: dev [mailto:dev-bounces@openvswitch.org] În numele Paul Boca > Trimis: Tuesday, June 14, 2016 2:27 PM > Către: dev@openvswitch.org > Subiect: [ovs-dev] [PATCH] datapath-windows: Handle possible NULL pointer > dereference in STT > > Check if OvsAllocatememoryWithTag succeeded or not. > In case of failure propagate cleanup and return. > > Signed-off-by: Paul-Daniel Boca <pboca@cloudbasesolutions.com> > --- > datapath-windows/ovsext/Stt.c | 7 +++++++ > 1 file changed, 7 insertions(+) > > diff --git a/datapath-windows/ovsext/Stt.c b/datapath- > windows/ovsext/Stt.c index 0bac5f2..e63211d 100644 > --- a/datapath-windows/ovsext/Stt.c > +++ b/datapath-windows/ovsext/Stt.c > @@ -641,6 +641,9 @@ OvsSttReassemble(POVS_SWITCH_CONTEXT > switchContext, > POVS_STT_PKT_ENTRY entry; > entry = OvsAllocateMemoryWithTag(sizeof(OVS_STT_PKT_ENTRY), > OVS_STT_POOL_TAG); > + if (NULL == entry) { > + goto handle_error; > + } > RtlZeroMemory(entry, sizeof (OVS_STT_PKT_ENTRY)); > > /* Update Key, timestamp and recvdLen */ @@ -663,6 +666,10 @@ > OvsSttReassemble(POVS_SWITCH_CONTEXT switchContext, > entry->allocatedLen = innerPacketLen; > entry->packetBuf = OvsAllocateMemoryWithTag(innerPacketLen, > OVS_STT_POOL_TAG); > + if (NULL == entry->packetBuf) { > + OvsFreeMemoryWithTag(pktFragEntry, OVS_STT_POOL_TAG); [Alin Gabriel Serdean: ] s/pktFragEntry/entry > + goto handle_error; > + } > if (OvsGetPacketBytes(curNbl, fragmentLength, startOffset, > entry->packetBuf + offset) == NULL) { > OVS_LOG_ERROR("Error when obtaining bytes from Packet"); > -- > 2.7.2.windows.1 > _______________________________________________ > dev mailing list > dev@openvswitch.org > http://openvswitch.org/mailman/listinfo/dev
diff --git a/datapath-windows/ovsext/Stt.c b/datapath-windows/ovsext/Stt.c index 0bac5f2..e63211d 100644 --- a/datapath-windows/ovsext/Stt.c +++ b/datapath-windows/ovsext/Stt.c @@ -641,6 +641,9 @@ OvsSttReassemble(POVS_SWITCH_CONTEXT switchContext, POVS_STT_PKT_ENTRY entry; entry = OvsAllocateMemoryWithTag(sizeof(OVS_STT_PKT_ENTRY), OVS_STT_POOL_TAG); + if (NULL == entry) { + goto handle_error; + } RtlZeroMemory(entry, sizeof (OVS_STT_PKT_ENTRY)); /* Update Key, timestamp and recvdLen */ @@ -663,6 +666,10 @@ OvsSttReassemble(POVS_SWITCH_CONTEXT switchContext, entry->allocatedLen = innerPacketLen; entry->packetBuf = OvsAllocateMemoryWithTag(innerPacketLen, OVS_STT_POOL_TAG); + if (NULL == entry->packetBuf) { + OvsFreeMemoryWithTag(pktFragEntry, OVS_STT_POOL_TAG); + goto handle_error; + } if (OvsGetPacketBytes(curNbl, fragmentLength, startOffset, entry->packetBuf + offset) == NULL) { OVS_LOG_ERROR("Error when obtaining bytes from Packet");
Check if OvsAllocatememoryWithTag succeeded or not. In case of failure propagate cleanup and return. Signed-off-by: Paul-Daniel Boca <pboca@cloudbasesolutions.com> --- datapath-windows/ovsext/Stt.c | 7 +++++++ 1 file changed, 7 insertions(+)