Skip to content

Add sanity-check to TCP options for loop#871

Merged
phillip-stephens merged 5 commits intomainfrom
phillip/tcp-options-efficient
May 3, 2024
Merged

Add sanity-check to TCP options for loop#871
phillip-stephens merged 5 commits intomainfrom
phillip/tcp-options-efficient

Conversation

@phillip-stephens
Copy link
Contributor

This fixes the bug opened in #868. The cause was if a packet came back with a TCP option in which the length was 0 (this does not conform to the spec ofc), the callback would be stuck in the loop forever.

It also reverts the change I made in #869 that fixed PR #862 since that was not the root cause like I thought it was.

I'll close the PR #870 (which reverted the TCP options commit until I could figure out why it broke) since we should all be good now.

Testing

With the new fix, a large scan doesn't get stuck like it did AND we don't have the 0 rec/sec message in the monitor thread every 43 seconds.

 0:41 67% (20s left); send: 102299450 2.40 Mp/s (2.49 Mp/s avg); recv: 1334110 31.5 Kp/s (32.5 Kp/s avg); drops: 0 p/s (0 p/s avg); hitrate: 1.30%
 0:42 69% (19s left); send: 104799774 2.50 Mp/s (2.49 Mp/s avg); recv: 1366494 32.4 Kp/s (32.5 Kp/s avg); drops: 0 p/s (0 p/s avg); hitrate: 1.30%
 0:43 71% (18s left); send: 107280122 2.48 Mp/s (2.49 Mp/s avg); recv: 1399029 32.5 Kp/s (32.5 Kp/s avg); drops: 0 p/s (0 p/s avg); hitrate: 1.30%
 0:44 72% (17s left); send: 109764410 2.48 Mp/s (2.49 Mp/s avg); recv: 1431746 32.7 Kp/s (32.5 Kp/s avg); drops: 0 p/s (0 p/s avg); hitrate: 1.30%
 0:45 74% (16s left); send: 112245626 2.48 Mp/s (2.49 Mp/s avg); recv: 1463817 32.1 Kp/s (32.5 Kp/s avg); drops: 0 p/s (0 p/s avg); hitrate: 1.30%
 0:46 75% (15s left); send: 114761978 2.52 Mp/s (2.49 Mp/s avg); recv: 1496579 32.8 Kp/s (32.5 Kp/s avg); drops: 0 p/s (0 p/s avg); hitrate: 1.30%
 0:47 77% (14s left); send: 117283578 2.52 Mp/s (2.49 Mp/s avg); recv: 1529225 32.6 Kp/s (32.5 Kp/s avg); drops: 0 p/s (0 p/s avg); hitrate: 1.30%
 0:48 79% (13s left); send: 120058541 2.77 Mp/s (2.50 Mp/s avg); recv: 1565490 36.2 Kp/s (32.6 Kp/s avg); drops: 0 p/s (0 p/s avg); hitrate: 1.30%
 0:49 80% (12s left); send: 122718138 2.66 Mp/s (2.50 Mp/s avg); recv: 1600673 35.2 Kp/s (32.6 Kp/s avg); drops: 0 p/s (0 p/s avg); hitrate: 1.30%
 0:50 82% (11s left); send: 125245242 2.53 Mp/s (2.50 Mp/s avg); recv: 1633638 33.0 Kp/s (32.6 Kp/s avg); drops: 0 p/s (0 p/s avg); hitrate: 1.30%
 0:51 84% (10s left); send: 127766970 2.52 Mp/s (2.50 Mp/s avg); recv: 1666846 33.2 Kp/s (32.6 Kp/s avg); drops: 0 p/s (0 p/s avg); hitrate: 1.30%
 0:52 85% (9s left); send: 130257468 2.49 Mp/s (2.50 Mp/s avg); recv: 1699147 32.3 Kp/s (32.6 Kp/s avg); drops: 0 p/s (0 p/s avg); hitrate: 1.30%
 0:53 87% (8s left); send: 132737146 2.48 Mp/s (2.50 Mp/s avg); recv: 1731153 32.0 Kp/s (32.6 Kp/s avg); drops: 0 p/s (0 p/s avg); hitrate: 1.30%
 0:54 89% (7s left); send: 135225699 2.49 Mp/s (2.50 Mp/s avg); recv: 1763774 32.6 Kp/s (32.6 Kp/s avg); drops: 0 p/s (0 p/s avg); hitrate: 1.30%
 0:55 90% (6s left); send: 137702714 2.48 Mp/s (2.50 Mp/s avg); recv: 1795981 32.2 Kp/s (32.6 Kp/s avg); drops: 0 p/s (0 p/s avg); hitrate: 1.30%
 0:56 92% (5s left); send: 140163962 2.46 Mp/s (2.50 Mp/s avg); recv: 1828478 32.5 Kp/s (32.6 Kp/s avg); drops: 0 p/s (0 p/s avg); hitrate: 1.30%
 0:57 94% (4s left); send: 142642938 2.48 Mp/s (2.50 Mp/s avg); recv: 1861072 32.6 Kp/s (32.6 Kp/s avg); drops: 0 p/s (0 p/s avg); hitrate: 1.30%
 0:58 95% (3s left); send: 145128122 2.48 Mp/s (2.50 Mp/s avg); recv: 1893231 32.1 Kp/s (32.6 Kp/s avg); drops: 0 p/s (0 p/s avg); hitrate: 1.30%
 0:59 97% (2s left); send: 147675130 2.55 Mp/s (2.50 Mp/s avg); recv: 1925950 32.7 Kp/s (32.6 Kp/s avg); drops: 0 p/s (0 p/s avg); hitrate: 1.30%
 1:00 98% (1s left); send: 150252608 done (2.50 Mp/s avg); recv: 1961053 35.1 Kp/s (32.7 Kp/s avg); drops: 0 p/s (0 p/s avg); hitrate: 1.31%
 1:01 100% (0s left); send: 150252608 done (2.50 Mp/s avg); recv: 1964161 3.11 Kp/s (32.2 Kp/s avg); drops: 0 p/s (0 p/s avg); hitrate: 1.31%
May 03 00:06:55.405 [INFO] zmap: completed

Copy link
Member

@zakird zakird left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

If the packet is malformed, do we want to +=1, or should we just give up on parsing options?

@phillip-stephens
Copy link
Contributor Author

You're 100% right, thought about this on my walk home. We can't assume that the option that is malformed would have a length of 1 so we should just give up. I'll make that change.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants