[Bug 43252] New: IcmpSendEcho doesn't work with a ReplySize<56
https://bugs.winehq.org/show_bug.cgi?id=43252 Bug ID: 43252 Summary: IcmpSendEcho doesn't work with a ReplySize<56 Product: Wine Version: unspecified Hardware: x86-64 OS: Linux Status: UNCONFIRMED Severity: normal Priority: P2 Component: iphlpapi Assignee: wine-bugs(a)winehq.org Reporter: slouslai(a)wegwerfemail.info Distribution: --- Created attachment 58564 --> https://bugs.winehq.org/attachment.cgi?id=58564 IcmpSendEcho trace IcmpSendEcho of icmp.c doesn't work with a ReplySize<56. If ReplySize<56 the function returns with IP_REQ_TIMED_OUT. In windows the function returns properly. This has been tested with ubuntu 16 and CentOs 7 (both x86-64) with wine 1.8.5(32 and 64bit prefix) and wine staging 2.1(32bit prefix). -- Do not reply to this email, post in Bugzilla using the above URL to reply. You are receiving this mail because: You are watching all bug changes.
https://bugs.winehq.org/show_bug.cgi?id=43252 Mark Mankins <Mark.Mankins(a)ngc.com> changed: What |Removed |Added ---------------------------------------------------------------------------- CC| |Mark.Mankins(a)ngc.com --- Comment #1 from Mark Mankins <Mark.Mankins(a)ngc.com> --- This bug affects IDA Pro (Floating License) v 7.0. When IDA Pro is run for the first time, it sends an icmp echo request to the floating license server. The icmp payload is set to a length of 17 bytes. IcmpSendEcho() incorrectly calculates the expected size of the icmp echo reply. Here's the pertinent code: ip_header=(struct ip *) ((char *) ReplyBuffer+sizeof(ICMP_ECHO_REPLY)); endbuf=(char *) ReplyBuffer+ReplySize; maxlen=ReplySize-sizeof(ICMP_ECHO_REPLY); [snip] res=recvfrom(icp->sid, (char*)ip_header, maxlen, 0, (struct sockaddr*)&addr,&addrlen); TRACE("received %d bytes from %s\n",res, inet_ntoa(addr.sin_addr)); ip_header is set to be a pointer ICMP_ECHO_REPLY characters past the start of ReplyBuffer. This appears to be incorrect and a contributor to the issue. The calculation of endbuf seems to be correct. This the end of the buffer that receives the echo reply packet. In my initial example, maxlen will be set to 25 bytes. The actual size of the icmp echo reply packet is 45 bytes. 20 bytes for the ip header, 8 bytes for the icmp header, and 17 bytes for the icmp data. maxlen should be 45 bytes to fully read the entire reply packet. Since only 25 bytes is read, the entire icmp header will not be read, and wine will incorrectly drop the icmp echo reply packet. I believe this diff corrects the issue: diff --git a/dlls/iphlpapi/icmp.c b/dlls/iphlpapi/icmp.c index ebc2f2b..8bfdf25 100644 --- a/dlls/iphlpapi/icmp.c +++ b/dlls/iphlpapi/icmp.c @@ -367,9 +367,9 @@ DWORD WINAPI IcmpSendEcho( fdr.events = POLLIN; addrlen=sizeof(addr); ier=ReplyBuffer; - ip_header=(struct ip *) ((char *) ReplyBuffer+sizeof(ICMP_ECHO_REPLY)); + ip_header=(struct ip *) ((char *) ReplyBuffer); endbuf=(char *) ReplyBuffer+ReplySize; - maxlen=ReplySize-sizeof(ICMP_ECHO_REPLY); + maxlen=ReplySize; /* Send the packet */ TRACE("Sending %d bytes (RequestSize=%d) to %s\n", reqsize, RequestSize, inet_ntoa(addr.sin_addr)); ip_header should be set to be the start of the ReplyBuffer. This is the start of the buffer recvfrom should write to. It should be large enough to hold the ip header, the icmp header, and the icmp payload. With this diff applied, IDA Pro runs as expected. I will attempt to create a simple console application that replicates this bug. -- Do not reply to this email, post in Bugzilla using the above URL to reply. You are receiving this mail because: You are watching all bug changes.
https://bugs.winehq.org/show_bug.cgi?id=43252 --- Comment #2 from Mark Mankins <Mark.Mankins(a)ngc.com> --- Upon further review, my analysis is not quite correct... stand by for a correction. -- Do not reply to this email, post in Bugzilla using the above URL to reply. You are receiving this mail because: You are watching all bug changes.
https://bugs.winehq.org/show_bug.cgi?id=43252 --- Comment #3 from Mark Mankins <Mark.Mankins(a)ngc.com> --- Ok...I'm happy to report that I think my analysis is basically correct. My test program proves that for small values of RequestSize passed to IcmpSendEcho, wine incorrectly ignores the icmp echo response. For large values passed to IcmpSendEcho, wine will handle the icmp echo response, but it will not pass back the complete payload to the caller. If the caller attempts to validate that the echo response matches the echo request (for any value of RequestSize), the validation will fail. I will attach my test program to this issue. Here is what I am seeing. Unpatched (unmodified source tree): osboxes(a)osboxes:~/wine64-build$ ./wine /mnt/hgfs/Shared/EchoTest/EchoTest.exe 55 Pinging localhost with 55 bytes of data... Address: 127.0.0.1 DataSize: 35 Echo data mismatch, byte 35 RoundTripTime: 0 Status: 0 osboxes(a)osboxes:~/wine64-build$ ./wine /mnt/hgfs/Shared/EchoTest/EchoTest.exe 20 Pinging localhost with 20 bytes of data... Address: 127.0.0.1 DataSize: 0 Echo data mismatch, byte 0 RoundTripTime: 0 Status: 0 osboxes(a)osboxes:~/wine64-build$ ./wine /mnt/hgfs/Shared/EchoTest/EchoTest.exe 17 Pinging localhost with 17 bytes of data... Unexpected result: 0 Patched with my patch in an earlier comment: osboxes(a)osboxes:~/wine64-build$ ./wine /mnt/hgfs/Shared/EchoTest/EchoTest.exe 55 Pinging localhost with 55 bytes of data... Address: 127.0.0.1 DataSize: 55 RoundTripTime: 0 Status: 0 osboxes(a)osboxes:~/wine64-build$ ./wine /mnt/hgfs/Shared/EchoTest/EchoTest.exe 20 Pinging localhost with 20 bytes of data... Address: 127.0.0.1 DataSize: 20 RoundTripTime: 0 Status: 0 osboxes(a)osboxes:~/wine64-build$ ./wine /mnt/hgfs/Shared/EchoTest/EchoTest.exe 17 Pinging localhost with 17 bytes of data... Address: 127.0.0.1 DataSize: 17 RoundTripTime: 0 Status: 0 -- Do not reply to this email, post in Bugzilla using the above URL to reply. You are receiving this mail because: You are watching all bug changes.
https://bugs.winehq.org/show_bug.cgi?id=43252 --- Comment #4 from Mark Mankins <Mark.Mankins(a)ngc.com> --- Created attachment 60273 --> https://bugs.winehq.org/attachment.cgi?id=60273 Test driver for IcmpSendEcho This program sends an icmp echo request to localhost and outputs the data returned to the caller. This program expects a single argument, the number of bytes to send in the icmp echo request payload. For the current version of wine (3.0rc6), for small values of the payload, wine fails to recognize the icmp echo response. For larger values of the payload, wine recognizes the response but doesn't return the complete payload to the caller. -- Do not reply to this email, post in Bugzilla using the above URL to reply. You are receiving this mail because: You are watching all bug changes.
https://bugs.winehq.org/show_bug.cgi?id=43252 Zhiyi Zhang <zzhang(a)codeweavers.com> changed: What |Removed |Added ---------------------------------------------------------------------------- CC| |zzhang(a)codeweavers.com --- Comment #5 from Zhiyi Zhang <zzhang(a)codeweavers.com> --- Great, if that's the issue, can you write a test for it and submit the patch? Thanks. -- Do not reply to this email, post in Bugzilla using the above URL to reply. You are receiving this mail because: You are watching all bug changes.
https://bugs.winehq.org/show_bug.cgi?id=43252 --- Comment #6 from Mark Mankins <Mark.Mankins(a)ngc.com> --- After looking at the requirements for patch submission, it appears I'm not eligible _at this time_ to submit patches to the project. However, all my previous comments on this thread were made when I would have been eligible to submit patches to the project. Someone else could pick up what I've documented in this issue and move this to a state where it can be integrated. Sorry for the inconvenience. -- Do not reply to this email, post in Bugzilla using the above URL to reply. You are receiving this mail because: You are watching all bug changes.
https://bugs.winehq.org/show_bug.cgi?id=43252 --- Comment #7 from Zhiyi Zhang <zzhang(a)codeweavers.com> --- Mark, thanks for your analysis. https://source.winehq.org/patches/data/147451 https://source.winehq.org/patches/data/147452 -- Do not reply to this email, post in Bugzilla using the above URL to reply. You are receiving this mail because: You are watching all bug changes.
https://bugs.winehq.org/show_bug.cgi?id=43252 --- Comment #8 from Gijs Vermeulen <gijsvrm(a)gmail.com> --- Was this fixed by: 417e996e97ebc3ff47d986e322dbaf3b3a83b91d ? Please retest with wine-3.14. -- Do not reply to this email, post in Bugzilla using the above URL to reply. You are receiving this mail because: You are watching all bug changes.
https://bugs.winehq.org/show_bug.cgi?id=43252 Gijs Vermeulen <gijsvrm(a)gmail.com> changed: What |Removed |Added ---------------------------------------------------------------------------- Fixed by SHA1| |417e996e97ebc3ff47d986e322d | |baf3b3a83b91d Resolution|--- |FIXED Status|UNCONFIRMED |RESOLVED --- Comment #9 from Gijs Vermeulen <gijsvrm(a)gmail.com> --- I compiled the test driver from Comment 4 and the results were identical to the correct results mentioned in Comment 3. I'm going to do ahead and mark this FIXED, if you are still experiencing issues, feel free to reopen. -- Do not reply to this email, post in Bugzilla using the above URL to reply. You are receiving this mail because: You are watching all bug changes.
https://bugs.winehq.org/show_bug.cgi?id=43252 Alexandre Julliard <julliard(a)winehq.org> changed: What |Removed |Added ---------------------------------------------------------------------------- Status|RESOLVED |CLOSED --- Comment #10 from Alexandre Julliard <julliard(a)winehq.org> --- Closing bugs fixed in 4.1. -- Do not reply to this email, post in Bugzilla using the above URL to reply. You are receiving this mail because: You are watching all bug changes.
participants (1)
-
wine-bugs＠winehq.org