[PATCH 1/2] iphlpapi: Use a standalone buffer in IcmpSendEcho().
Zhiyi Zhang
zzhang at codeweavers.com
Thu Jun 14 23:23:23 CDT 2018
Fix https://bugs.winehq.org/show_bug.cgi?id=43252
The old implementation uses user provided buffer to receive
packet data, which is alway not enough, causing data corruptions
or incorrectly timeout.
Signed-off-by: Zhiyi Zhang <zzhang at codeweavers.com>
---
dlls/iphlpapi/icmp.c | 38 +++++++++++++++++++++++++++-------
dlls/iphlpapi/tests/iphlpapi.c | 21 +++++++++++++++----
2 files changed, 47 insertions(+), 12 deletions(-)
diff --git a/dlls/iphlpapi/icmp.c b/dlls/iphlpapi/icmp.c
index ebc2f2b65c..7c91443598 100644
--- a/dlls/iphlpapi/icmp.c
+++ b/dlls/iphlpapi/icmp.c
@@ -113,6 +113,9 @@ typedef struct {
#define IP_OPTS_DEFAULT 1
#define IP_OPTS_CUSTOM 2
+#define MAXIPLEN 60
+#define MAXICMPLEN 76
+
/* The sequence number is unique process wide, so that all threads
* have a distinct sequence number.
*/
@@ -270,13 +273,14 @@ DWORD WINAPI IcmpSendEcho(
icmp_t* icp=(icmp_t*)IcmpHandle;
unsigned char* reqbuf;
int reqsize;
+ unsigned char* repbuf;
+ int repsize;
struct icmp_echo_reply* ier;
struct ip* ip_header;
struct icmp* icmp_header;
char* endbuf;
int ip_header_len;
- int maxlen;
struct pollfd fdr;
DWORD send_time,recv_time;
struct sockaddr_in addr;
@@ -312,6 +316,16 @@ DWORD WINAPI IcmpSendEcho(
return 0;
}
+ /* max ip header + max icmp header and error data + reply size(max 65535 on Windows) */
+ /* FIXME: request size of 65535 is not supported yet because max buffer size of raw socket on linux is 32767 */
+ repsize=MAXIPLEN+MAXICMPLEN+(ReplySize&0xFFFF);
+ repbuf=HeapAlloc(GetProcessHeap(), 0, repsize);
+ if (reqbuf==NULL) {
+ HeapFree(GetProcessHeap(), 0, reqbuf);
+ SetLastError(ERROR_OUTOFMEMORY);
+ return 0;
+ }
+
icmp_header=(struct icmp*)reqbuf;
icmp_header->icmp_type=ICMP_ECHO;
icmp_header->icmp_code=0;
@@ -367,9 +381,7 @@ DWORD WINAPI IcmpSendEcho(
fdr.events = POLLIN;
addrlen=sizeof(addr);
ier=ReplyBuffer;
- ip_header=(struct ip *) ((char *) ReplyBuffer+sizeof(ICMP_ECHO_REPLY));
endbuf=(char *) ReplyBuffer+ReplySize;
- maxlen=ReplySize-sizeof(ICMP_ECHO_REPLY);
/* Send the packet */
TRACE("Sending %d bytes (RequestSize=%d) to %s\n", reqsize, RequestSize, inet_ntoa(addr.sin_addr));
@@ -407,10 +419,11 @@ DWORD WINAPI IcmpSendEcho(
}
/* Get the reply */
+ ip_header=(struct ip*)repbuf;
ip_header_len=0; /* because gcc was complaining */
while (poll(&fdr,1,Timeout)>0) {
recv_time = GetTickCount();
- res=recvfrom(icp->sid, (char*)ip_header, maxlen, 0, (struct sockaddr*)&addr,&addrlen);
+ res=recvfrom(icp->sid, (char*)repbuf, repsize, 0, (struct sockaddr*)&addr, &addrlen);
TRACE("received %d bytes from %s\n",res, inet_ntoa(addr.sin_addr));
ier->Status=IP_REQ_TIMED_OUT;
@@ -508,6 +521,12 @@ DWORD WINAPI IcmpSendEcho(
else Timeout = 0;
continue;
} else {
+ /* Check free space, should be large enough for an ICMP_ECHO_REPLY and remainning icmp data */
+ if (endbuf-(char *)ier < sizeof(struct icmp_echo_reply)+(res-ip_header_len-ICMP_MINLEN)) {
+ res=ier-(ICMP_ECHO_REPLY *)ReplyBuffer;
+ SetLastError(IP_GENERAL_FAILURE);
+ goto done;
+ }
/* This is a reply to our packet */
memcpy(&ier->Address,&ip_header->ip_src,sizeof(IPAddr));
/* Status is already set */
@@ -515,7 +534,7 @@ DWORD WINAPI IcmpSendEcho(
ier->DataSize=res-ip_header_len-ICMP_MINLEN;
ier->Reserved=0;
ier->Data=endbuf-ier->DataSize;
- memmove(ier->Data,((char*)ip_header)+ip_header_len+ICMP_MINLEN,ier->DataSize);
+ memcpy(ier->Data, ((char *)ip_header)+ip_header_len+ICMP_MINLEN, ier->DataSize);
ier->Options.Ttl=ip_header->ip_ttl;
ier->Options.Tos=ip_header->ip_tos;
ier->Options.Flags=ip_header->ip_off >> 13;
@@ -523,7 +542,7 @@ DWORD WINAPI IcmpSendEcho(
if (ier->Options.OptionsSize!=0) {
ier->Options.OptionsData=(unsigned char *) ier->Data-ier->Options.OptionsSize;
/* FIXME: We are supposed to rearrange the option's 'source route' data */
- memmove(ier->Options.OptionsData,((char*)ip_header)+ip_header_len,ier->Options.OptionsSize);
+ memcpy(ier->Options.OptionsData, ((char *)ip_header)+ip_header_len, ier->Options.OptionsSize);
endbuf=(char*)ier->Options.OptionsData;
} else {
ier->Options.OptionsData=NULL;
@@ -531,9 +550,8 @@ DWORD WINAPI IcmpSendEcho(
}
/* Prepare for the next packet */
+ endbuf-=ier->DataSize;
ier++;
- ip_header=(struct ip*)(((char*)ip_header)+sizeof(ICMP_ECHO_REPLY));
- maxlen=endbuf-(char*)ip_header;
/* Check out whether there is more but don't wait this time */
Timeout=0;
@@ -542,6 +560,10 @@ DWORD WINAPI IcmpSendEcho(
res=ier-(ICMP_ECHO_REPLY*)ReplyBuffer;
if (res==0)
SetLastError(IP_REQ_TIMED_OUT);
+ else
+ SetLastError(NO_ERROR);
+done:
+ HeapFree(GetProcessHeap(), 0, repbuf);
TRACE("received %d replies\n",res);
return res;
}
diff --git a/dlls/iphlpapi/tests/iphlpapi.c b/dlls/iphlpapi/tests/iphlpapi.c
index d5613d7b4b..f964878d68 100644
--- a/dlls/iphlpapi/tests/iphlpapi.c
+++ b/dlls/iphlpapi/tests/iphlpapi.c
@@ -951,6 +951,8 @@ static void testIcmpSendEcho(void)
char senddata[32], replydata[sizeof(senddata) + sizeof(ICMP_ECHO_REPLY)];
DWORD ret, error, replysz = sizeof(replydata);
IPAddr address;
+ ICMP_ECHO_REPLY *reply;
+ INT i;
if (!pIcmpSendEcho || !pIcmpCreateFile)
{
@@ -1038,12 +1040,10 @@ todo_wine
replysz = sizeof(replydata) - 1;
ret = pIcmpSendEcho(icmp, address, senddata, sizeof(senddata), NULL, replydata, replysz, 1000);
error = GetLastError();
- todo_wine {
ok (!ret, "IcmpSendEcho succeeded unexpectedly\n");
ok (error == IP_GENERAL_FAILURE
|| broken(error == IP_BUF_TOO_SMALL) /* <= 2003 */,
"expected 11050, got %d\n", error);
- }
SetLastError(0xdeadbeef);
replysz = sizeof(ICMP_ECHO_REPLY);
@@ -1056,7 +1056,6 @@ todo_wine
replysz = sizeof(ICMP_ECHO_REPLY) + ICMP_MINLEN;
ret = pIcmpSendEcho(icmp, address, senddata, ICMP_MINLEN, NULL, replydata, replysz, 1000);
error = GetLastError();
-todo_wine
ok (ret, "IcmpSendEcho failed unexpectedly with error %d\n", error);
SetLastError(0xdeadbeef);
@@ -1064,7 +1063,6 @@ todo_wine
ret = pIcmpSendEcho(icmp, address, senddata, ICMP_MINLEN + 1, NULL, replydata, replysz, 1000);
error = GetLastError();
ok (!ret, "IcmpSendEcho succeeded unexpectedly\n");
-todo_wine
ok (error == IP_GENERAL_FAILURE
|| broken(error == IP_BUF_TOO_SMALL) /* <= 2003 */,
"expected 11050, got %d\n", error);
@@ -1111,6 +1109,21 @@ todo_wine
{
skip ("Failed to ping with error %d, is lo interface down?.\n", error);
}
+
+ /* check reply data */
+ SetLastError(0xdeadbeef);
+ address = htonl(INADDR_LOOPBACK);
+ for (i = 0; i < ARRAY_SIZE(senddata); i++) senddata[i] = i & 0xff;
+ ret = pIcmpSendEcho(icmp, address, senddata, sizeof(senddata), NULL, replydata, replysz, 1000);
+ error = GetLastError();
+ reply = (ICMP_ECHO_REPLY *)replydata;
+ ok(ret, "IcmpSendEcho failed unexpectedly\n");
+ ok(error == NO_ERROR, "Expect last error:0x%08x, got:0x%08x\n", NO_ERROR, error);
+ ok(INADDR_LOOPBACK == ntohl(reply->Address), "Address mismatch, expect:%s, got: %s\n", ntoa(INADDR_LOOPBACK),
+ ntoa(reply->Address));
+ ok(reply->Status == IP_SUCCESS, "Expect status:0x%08x, got:0x%08x\n", IP_SUCCESS, reply->Status);
+ ok(reply->DataSize == sizeof(senddata), "Expect data size:%d, got:%d\n", sizeof(senddata), reply->DataSize);
+ ok(!memcmp(senddata, reply->Data, min(sizeof(senddata), reply->DataSize)), "Data mismatch\n");
}
/*
--
2.17.1
More information about the wine-devel
mailing list