Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Feature request: graceful malformed packet drop #75

Closed
jsmif opened this issue May 11, 2024 · 10 comments
Closed

Feature request: graceful malformed packet drop #75

jsmif opened this issue May 11, 2024 · 10 comments
Assignees
Labels
bug Something isn't working enhancement New feature or request

Comments

@jsmif
Copy link

jsmif commented May 11, 2024

I just went to do a headless long-term sniffing run, and when I got back I saw Sniffle had died prematurely with the following error:

Traceback (most recent call last):
  File "/home/user/Sniffle/python_cli/sniff_receiver.py", line 216, in <module>
    main()
  File "/home/user/Sniffle/python_cli/sniff_receiver.py", line 157, in main
    print_message(msg, args.quiet)
  File "/home/user/Sniffle/python_cli/sniff_receiver.py", line 161, in print_message
    print_packet(msg, quiet)
  File "/home/user/Sniffle/python_cli/sniff_receiver.py", line 168, in print_packet
    dpkt = DPacketMessage.decode(pkt)
  File "/home/user/Sniffle/python_cli/packet_decoder.py", line 61, in decode
    return AdvertMessage.decode(pkt)
  File "/home/user/Sniffle/python_cli/packet_decoder.py", line 111, in decode
    return tc(pkt)
  File "/home/user/Sniffle/python_cli/packet_decoder.py", line 275, in __init__
    self.aa_conn = struct.unpack('<L', self.body[14:18])[0]
struct.error: unpack requires a buffer of 4 bytes

(Note: the device was accidentally out of date, so those line numbers correspond to commit f4cc588).

So the ask would be to wrap instances of struct.unpack(), or anything else which might throw an error on malformed packets in a try-except, to just make it throw away the packet so it doesn't accidentally error out early on long runs. Thank you.

@sultanqasim
Copy link
Collaborator

Good idea, for long runs there are many situations in which there could be a glitch in the data coming in, for example I’ve seen intermittent cases of chunks of data being lost over USB CDC-ACM, especially during context switches when running inside a VM and under situations of high CPU load.

@sultanqasim sultanqasim added bug Something isn't working enhancement New feature or request labels May 13, 2024
@jsmif
Copy link
Author

jsmif commented May 14, 2024

OK, I thought it was because I had a mismatched firmware/software version, but I checked out v1.9.3 specifically and flashed the v1.9.3 release firmware and I'm still getting this error fairly frequently when I run with -q -a -e:

Traceback (most recent call last):
  File "/home/user/Sniffle/python_cli/sniff_receiver.py", line 203, in <module>
    main()
  File "/home/user/Sniffle/python_cli/sniff_receiver.py", line 161, in main
    msg = hw.recv_and_decode()
  File "/home/user/Sniffle/python_cli/sniffle_hw.py", line 334, in recv_and_decode
    mtype, mbody, pkt = self._recv_msg()
  File "/home/user/Sniffle/python_cli/sniffle_hw.py", line 305, in _recv_msg
    word_cnt = data[0]
IndexError: index out of range

@sultanqasim
Copy link
Collaborator

sultanqasim commented May 14, 2024

Hmm, did 5cecf12 cause that for you? Does the error appear right at startup? The only way that can happen is if pkt here is all whitespace or CRLF characters.

If you change this line to if len(pkt.strip()) < 4: does that fix it?

sultanqasim added a commit that referenced this issue May 14, 2024
Not sure how they're happening, I've never noticed any issues myself with
this, though jsmif reported seeing errors with this in issue #75.
@sultanqasim
Copy link
Collaborator

Try the latest master branch, hopefully that fixes this particular crash you reported.

@sultanqasim
Copy link
Collaborator

And this should improve general robustness: 87ac1bd

@sultanqasim sultanqasim self-assigned this May 14, 2024
@jsmif
Copy link
Author

jsmif commented May 28, 2024

The error is not currently occurring for me, but I'm also not physically where it was occurring, so I'm not subject to the same background packets. Once I'm back in the same location I'll let you know if it's still occurring (with the old, and then the new, code). Thank you.

@XenoKovah
Copy link

XenoKovah commented Jun 2, 2024

FWIW I left my sniffers (confirmed to be on 1.9.3) running overnight in a hotel last night (so I won't be able to replicate this in the future), and when I got up one of them had got basically the same error as the OP:

Traceback (most recent call last):
  File "/home/test/Downloads/Sniffle/python_cli/sniff_receiver.py", line 203, in <module>
    main()
  File "/home/test/Downloads/Sniffle/python_cli/sniff_receiver.py", line 162, in main
    print_message(msg, args.quiet)
  File "/home/test/Downloads/Sniffle/python_cli/sniff_receiver.py", line 166, in print_message
    print_packet(msg, quiet)
  File "/home/test/Downloads/Sniffle/python_cli/sniff_receiver.py", line 173, in print_packet
    dpkt = DPacketMessage.decode(pkt, hw.decoder_state)
  File "/home/test/Downloads/Sniffle/python_cli/packet_decoder.py", line 81, in decode
    dpkt = AdvertMessage.decode(pkt, dstate)
  File "/home/test/Downloads/Sniffle/python_cli/packet_decoder.py", line 146, in decode
    return tc(pkt)
  File "/home/test/Downloads/Sniffle/python_cli/packet_decoder.py", line 310, in __init__
    self.aa_conn = struct.unpack('<L', self.body[14:18])[0]
struct.error: unpack requires a buffer of 4 bytes

Top of the git log for conf:

commit 588e1638c352e95d62cbc48bfb1387731d58d06d (HEAD, tag: v1.9.3)
Author: Sultan Qasim Khan <>
Date:   Wed May 8 12:31:36 2024 -0400

    fw: Bump the revision
    
    Nothing changed in firmware since v1.9.2, but this is so it matches the
    overall release version.

@sultanqasim
Copy link
Collaborator

That should be fixed in the latest master branch; it will still work fine with the v1.9.3/v1.9.2 firmware.

@XenoKovah
Copy link

Ah, I didn't pay attention and realize the cited fix commit wasn't in 1.9.3. Thanks

@jsmif
Copy link
Author

jsmif commented Jul 9, 2024

FYI I don't seem to be seeing any script-killing-crashes due to unpack errors anymore. I do see the occasional error still popping up, but it seems to be handled gracefully, so I'm closing this now until I find another script-killer. Thanks!

@jsmif jsmif closed this as completed Jul 9, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
bug Something isn't working enhancement New feature or request
Projects
None yet
Development

No branches or pull requests

3 participants