TechnikEmpire / DivertPInvoke

PInvoke wrapper for WinDivert
GNU Lesser General Public License v3.0
25 stars 12 forks source link

Queue Overflow #2

Closed MrRandom92 closed 7 years ago

MrRandom92 commented 7 years ago

Is it possible for the WinDivert queue to overflow due to pPacket in WinDivertRecv not being an IntPtr, as byte[] pPacket appears to be simply copying the data not moving and freeing memory in WinDivert queue?

public static extern bool WinDivertRecv([In] IntPtr handle, byte[] pPacket

I have tried using IntPtr pPacket with the wrapper, but it throws a memory access violation. Any information on this problem would be helpful.

TechnikEmpire commented 7 years ago

@TodM0 I'm not sure what you're asking. WinDivert handles its queue all by itself, and you should not be trying to free any pointers given to you by WinDivert, which is why I'm not doing that anywhere. WinDivert copies data into the buffer you supply it, so that's normal. Any of those pointers you're working with in the wrapper shouldn't be manually freed or anything like that.

It's throwing the memory access violation because you're giving it a pointer to probably nothing, and then supplying a packet length, so it's trying to write into nowhere land memory wise. Or, what's happening is that the CLR is moving around whatever object you obtained an IntPtr to. PInvoke automatically handles things like pointer/object pinning (which is necessary to prevent the aforementioned problem), as well as marshaling of data to and from the unmanaged side.

I have used this wrapper for many tests, but nothing in production, so while I'm open to the idea that it's fundamentally broken, I simply haven't seen anything personally to justify that. If you do come up with a reproducible issue, please do post code to achieve it and I'll have a look.

MrRandom92 commented 7 years ago

@TechnikEmpire I have posted the code on stackoverflow, I cannot rule out DivertPInvoke as being problematic, I also included a CPU stack trace if you could verify the CPU usage is normal?

https://stackoverflow.com/questions/44136191/windivert-connection-loss

The issue tends to occur when performing bandwidth intensive actions and fails for me mainly upon the upload test (can also fail spontaneously), the code has been changed slightly since my last issue report.

TechnikEmpire commented 7 years ago

@TodM0 Unfortunately I have no time to review your code and I don't think you'll find much help on stackoverflow because it's a graveyard of Q&A for basic programming problems. From a super quick glance at your code, I can't imagine how this would work. Probably the biggest, out of all issues, is that packet order has gone out the window and is completely at the whim of two independent threads. Protocols that are not designed to handle this will completely fail, and protocols like TCP which are, will automatically throttle the connection while it attempts to reconstruct the very screwed up TCP stream in order to avoid choking up the network. Then, the endpoint seeing the screw up (probably both in this case) will begin querying the other end trying to get packets it can't reassemble or recover, and since your filter here just keeps spinning the way it does, this will become a cascading failure from which there is no return.

I'd suggest that you read up on the TCP protocol. I'd also suggest starting your testing by limiting it to a single protocol (using a "true" filter you're grabbing every packet of every protocol on every address space!!!). Networking is an extremely complex, multi-layered, multifaceted system that you can't just grab hold of and start messing with. Start off doing your delays inline. Forget queues and threads and so on and so forth. If you use the sendex function for starters, it automatically offloads sends to the driver using overlapped IO, so there's no need there. You're just compounding the complexity out of the gate by using multiple functions and multiple threads. Use a single function first, that way you know your control flow is from bracket A to bracket B and is guaranteed.

https://en.wikipedia.org/wiki/Transmission_Control_Protocol#Error_detection https://en.wikipedia.org/wiki/Transmission_Control_Protocol#TCP_timestamps https://en.wikipedia.org/wiki/Out-of-order_delivery

Also it might be worth reading over Clumbsy's source code to see how they accomplish simulated lag:

http://jagt.github.io/clumsy/

MrRandom92 commented 7 years ago

@TechnikEmpire To keep the packets in order I am using the C# Queue class which is a FIFO method. I have tested with a single thread first which does appear to function fine without adding Queues and latency, though I cannot keep loading packets into the Queue while waiting to send without another thread.

I am aware that TCP could have been an issue, but I have tested using only UDP as the filter and the same issue arises. Both Clumsy and Inssidious use WinDivert and manage to filter all protocols without doing anything special for TCP, I just figured using a higher level language like C# would make it simpler and it's unfortunate I may not be able to use your wrapper as a solution for my project.

If you have anything more to add that could help me create a working solution using your wrapper I would feel inclined to donate to you for your time if possible. In the meantime I will look for alternative solutions.

TechnikEmpire commented 7 years ago

@TodM0 The wrapper should be functionally equivalent to using WinDivert directly in C/C++. It really does absolutely nothing different except add a bit of syntactic sugar for auto byte order swapping on some structures, and as you note, brings you into a higher level language.

I appreciate the offer to donate something, but that's not the issue, it's just that I'm swamped with getting another project out the door right now. I will revisit this afterward because I am interested in why it's so difficult to do something that sounds conceptually very easy.