[CRITICAL] Don't trust GCD to give accurate UDP packet sizes. #222

Merged
merged 3 commits into from Aug 7, 2015

Projects

None yet

3 participants

@erichocean
Contributor

GCD does not necessarily return the size of an entire UDP packet from dispatch_source_get_data(), so we must use the maximum packet size.

Without this fix, packets can be silently truncated, resulting in a loss of data. I (and others—see comments) have experienced packet truncation "in the wild".

@aasenjo
aasenjo commented Mar 13, 2014

Erich,

What is the behavior you were seeing with this issue? I am finding that sometimes my UDP packets contain less data than expected. As far as I can tell, the right amount of data is being passed out by the sender but on the receiver side I only get the first part of the message.

I was about to pull out the Wireshark when I decided to check here. I am wondering if this could be the problem.

Thanks,

@erichocean
Contributor

That is exactly the problem I was seeing, too. This patch fixes that. :)

The issue is that recvfrom() will either (a) give you up to the end of the packet, if you ask for more bytes than the packet contains, or (b) give you just up to the bytes you asked for, truncating any remaining bytes in the packet and dropping them forever (the next read will get you the next packet).

This interacted badly with GCD, which occasionally notifies that data is available with a "length" that is less than the number of bytes in the packet. When this "short" length is later passed to recvfrom(), you get the truncated packet.

In my testing, this is mostly likely to occur when packets are arriving and being processed at high speeds.

@aasenjo
aasenjo commented Mar 14, 2014

Awesome. Thanks for letting me know. It'll be a few days before I can give it a try but I'll report back.

Thanks for taking the time to fix it.

Alejandro

Sent from my autocorrect-happy iThingy

On Mar 12, 2014, at 10:55 PM, Erich Ocean notifications@github.com wrote:

That is exactly the problem I was seeing, too. This patch fixes that. :)


Reply to this email directly or view it on GitHub.

@erichocean erichocean changed the title from Don't trust GCD to give us UDP packet sizes. to [CRITICAL] Don't trust GCD to give us UDP packet sizes. Mar 17, 2014
@erichocean erichocean changed the title from [CRITICAL] Don't trust GCD to give us UDP packet sizes. to [CRITICAL] Don't trust GCD to give accurate UDP packet sizes. Mar 17, 2014
@chrisballinger
Collaborator

@erichocean Thanks!

@chrisballinger chrisballinger merged commit 3754a73 into robbiehanson:master Aug 7, 2015
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment