Skip to content

Commit fbfa4d1

Browse files
Joseph Hindinvrozenfe
Joseph Hindin
authored andcommitted
NetKVM: BZ#1169718: More rigoruous testing of incoming packet
Signed-off-by: Joseph Hindin <yhindin@rehat.com>
1 parent 7410b96 commit fbfa4d1

File tree

2 files changed

+48
-12
lines changed

2 files changed

+48
-12
lines changed

Diff for: NetKVM/Common/ParaNdis-Common.cpp

+8-2
Original file line numberDiff line numberDiff line change
@@ -2222,11 +2222,10 @@ tChecksumCheckResult ParaNdis_CheckRxChecksum(
22222222
ULONG ulDataOffset)
22232223
{
22242224
tOffloadSettingsFlags f = pContext->Offload.flags;
2225-
tChecksumCheckResult res, resIp;
2225+
tChecksumCheckResult res;
22262226
tTcpIpPacketParsingResult ppr;
22272227
ULONG flagsToCalculate = 0;
22282228
res.value = 0;
2229-
resIp.value = 0;
22302229

22312230
//VIRTIO_NET_HDR_F_NEEDS_CSUM - we need to calculate TCP/UDP CS
22322231
//VIRTIO_NET_HDR_F_DATA_VALID - host tells us TCP/UDP CS is OK
@@ -2250,6 +2249,13 @@ tChecksumCheckResult ParaNdis_CheckRxChecksum(
22502249

22512250
ppr = ParaNdis_CheckSumVerify(pPacketPages, ulPacketLength - ETH_HEADER_SIZE, ulDataOffset + ETH_HEADER_SIZE, flagsToCalculate, __FUNCTION__);
22522251

2252+
if (ppr.ipCheckSum == ppresIPTooShort || ppr.xxpStatus == ppresXxpIncomplete)
2253+
{
2254+
res.flags.IpOK = FALSE;
2255+
res.flags.IpFailed = TRUE;
2256+
return res;
2257+
}
2258+
22532259
if (virtioFlags & VIRTIO_NET_HDR_F_DATA_VALID)
22542260
{
22552261
pContext->extraStatistics.framesRxCSHwOK++;

Diff for: NetKVM/Common/sw-offload.cpp

+40-10
Original file line numberDiff line numberDiff line change
@@ -156,19 +156,21 @@ ProcessTCPHeader(tTcpIpPacketParsingResult _res, PVOID pIpHeader, ULONG len, USH
156156
ULONG tcpipDataAt;
157157
tTcpIpPacketParsingResult res = _res;
158158
tcpipDataAt = ipHeaderSize + sizeof(TCPHeader);
159-
res.xxpStatus = ppresXxpIncomplete;
160159
res.TcpUdp = ppresIsTCP;
161160

162161
if (len >= tcpipDataAt)
163162
{
164163
TCPHeader *pTcpHeader = (TCPHeader *)RtlOffsetToPointer(pIpHeader, ipHeaderSize);
165164
res.xxpStatus = ppresXxpKnown;
165+
res.xxpFull = TRUE;
166166
tcpipDataAt = ipHeaderSize + TCP_HEADER_LENGTH(pTcpHeader);
167167
res.XxpIpHeaderSize = tcpipDataAt;
168168
}
169169
else
170170
{
171171
DPrintf(2, ("tcp: %d < min headers %d\n", len, tcpipDataAt));
172+
res.xxpFull = FALSE;
173+
res.xxpStatus = ppresXxpIncomplete;
172174
}
173175
return res;
174176
}
@@ -178,42 +180,67 @@ ProcessUDPHeader(tTcpIpPacketParsingResult _res, PVOID pIpHeader, ULONG len, USH
178180
{
179181
tTcpIpPacketParsingResult res = _res;
180182
ULONG udpDataStart = ipHeaderSize + sizeof(UDPHeader);
181-
res.xxpStatus = ppresXxpIncomplete;
182183
res.TcpUdp = ppresIsUDP;
183184
res.XxpIpHeaderSize = udpDataStart;
184185
if (len >= udpDataStart)
185186
{
186187
UDPHeader *pUdpHeader = (UDPHeader *)RtlOffsetToPointer(pIpHeader, ipHeaderSize);
187188
USHORT datagramLength = swap_short(pUdpHeader->udp_length);
188189
res.xxpStatus = ppresXxpKnown;
190+
res.xxpFull = TRUE;
189191
// may be full or not, but the datagram length is known
190192
DPrintf(2, ("udp: len %d, datagramLength %d\n", len, datagramLength));
191193
}
194+
else
195+
{
196+
res.xxpFull = FALSE;
197+
res.xxpStatus = ppresXxpIncomplete;
198+
}
192199
return res;
193200
}
194201

195202
static __inline tTcpIpPacketParsingResult
196203
QualifyIpPacket(IPHeader *pIpHeader, ULONG len)
197204
{
198205
tTcpIpPacketParsingResult res;
206+
res.value = 0;
207+
208+
if (len < 4)
209+
{
210+
res.ipStatus = ppresNotIP;
211+
return res;
212+
}
213+
199214
UCHAR ver_len = pIpHeader->v4.ip_verlen;
200215
UCHAR ip_version = (ver_len & 0xF0) >> 4;
201216
USHORT ipHeaderSize = 0;
202217
USHORT fullLength = 0;
203218
res.value = 0;
204-
219+
205220
if (ip_version == 4)
206221
{
222+
if (len < sizeof(IPv4Header))
223+
{
224+
res.ipStatus = ppresNotIP;
225+
return res;
226+
}
207227
ipHeaderSize = (ver_len & 0xF) << 2;
208228
fullLength = swap_short(pIpHeader->v4.ip_length);
209-
DPrintf(3, ("ip_version %d, ipHeaderSize %d, protocol %d, iplen %d\n",
210-
ip_version, ipHeaderSize, pIpHeader->v4.ip_protocol, fullLength));
229+
DPrintf(3, ("ip_version %d, ipHeaderSize %d, protocol %d, iplen %d, L2 payload length %d\n",
230+
ip_version, ipHeaderSize, pIpHeader->v4.ip_protocol, fullLength, len));
231+
211232
res.ipStatus = (ipHeaderSize >= sizeof(IPv4Header)) ? ppresIPV4 : ppresNotIP;
212-
if (len < ipHeaderSize) res.ipCheckSum = ppresIPTooShort;
213-
if (fullLength) {}
214-
else
233+
if (res.ipStatus == ppresNotIP)
215234
{
216-
DPrintf(2, ("ip v.%d, iplen %d\n", ip_version, fullLength));
235+
return res;
236+
}
237+
238+
if (ipHeaderSize >= fullLength || len < fullLength)
239+
{
240+
DPrintf(2, ("[%s] - truncated packet - ip_version %d, ipHeaderSize %d, protocol %d, iplen %d, L2 payload length %d\n",
241+
ip_version, ipHeaderSize, pIpHeader->v4.ip_protocol, fullLength, len));
242+
res.ipCheckSum = ppresIPTooShort;
243+
return res;
217244
}
218245
}
219246
else if (ip_version == 6)
@@ -291,7 +318,7 @@ QualifyIpPacket(IPHeader *pIpHeader, ULONG len)
291318
if (res.ipStatus == ppresIPV4)
292319
{
293320
res.ipHeaderSize = ipHeaderSize;
294-
res.xxpFull = len >= fullLength ? 1 : 0;
321+
295322
// bit "more fragments" or fragment offset mean the packet is fragmented
296323
res.IsFragment = (pIpHeader->v4.ip_offset & ~0xC0) != 0;
297324
switch (pIpHeader->v4.ip_protocol)
@@ -615,6 +642,9 @@ tTcpIpPacketParsingResult ParaNdis_CheckSumVerify(
615642
IPHeader *pIpHeader = (IPHeader *) RtlOffsetToPointer(pDataPages[0].Virtual, ulStartOffset);
616643

617644
tTcpIpPacketParsingResult res = QualifyIpPacket(pIpHeader, ulDataLength);
645+
if (res.ipStatus == ppresNotIP || res.ipCheckSum == ppresIPTooShort)
646+
return res;
647+
618648
if (res.ipStatus == ppresIPV4)
619649
{
620650
if (flags & pcrIpChecksum)

0 commit comments

Comments
 (0)