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

Could an error other than ERROR_OPERATION_ABORTED be reported if a device is removed? #2036

Closed
guyharris opened this issue May 6, 2020 · 3 comments

Comments

@guyharris
Copy link

@guyharris guyharris commented May 6, 2020

I think the NT status STATUS_CANCELLED, returned by the driver if NPF_StartUsingOpenInstance() returns FALSE, gets mapped to ERROR_OPERATION_ABORTED in userland from a ReadFile() or WriteFile() call.

If NPF_StartUsingOpenInstance() returns FALSE because the device has been removed, returning a different error might be useful, so that libpcap can distinguish between "the capture stopped because the device was removed", which it could report with the standard "The interface disappeared" error for that case, and report "PacketReceivePacket error (995)" for other cases, which would be "shouldn't happen" cases (as was the case in older versions, where sleeping and waking up could provoke that error).

There is an NT status value STATUS_DEVICE_REMOVED; I don't know what Windows status that's mapped to.

This came up because of Wireshark bug 16555; the error message in Wireshark suggested reporting it as an error before the fixes in 0.9988, to try to get more information, but if it's due to a known cause, such as the device being removed, we should just tell the user that the device was removed.

@guyharris
Copy link
Author

@guyharris guyharris commented May 7, 2020

Or, if the only reason why an operation would get ERROR_OPERATION_ABORTED is now, and will continue to be, "the device was removed", I could handle this entirely in libpcap, by interpreting ERROR_OPERATION_ABORTED as "The interface disappeared".

(If not, a distinction should be made between "the device disappeared" and other reasons, using different status values.)

@dmiller-nmap
Copy link

@dmiller-nmap dmiller-nmap commented May 7, 2020

For the Read call, we returned STATUS_CANCELLED on that line and no other, so it should be pretty consistent. The only reason NPF_StartUsingOpenInstance() would return false was if NDIS removed Npcap from the stack (such as when an adapter is removed) or if something closed the handle returned by PacketOpenAdapter(). So I'd say you could map that error to "the adapter was removed."

Now, I just pushed a change that ought to make things much better by distinguishing between operations that require an adapter to be present (packet write operations and some IoControl/OID things) and those that do not (getting stats, reading remaining packets in the buffer, configuring the capture session, etc.). If an operation requires an adapter that is not present, we return STATUS_DEVICE_REMOVED. I haven't verified if network disconnect is the same as adapter removed in this case; it's based on NDIS calling our FilterDetach handler.

The Read operation (PacketReceivePacket()) has the following changed behavior if the adapter is not present:

  • MinToCopy is not respected, so a Read will return whatever packets are in the buffer immediately. I'm not sure if this is good enough, since the changes in 0.9991 mean that there could be packets in the WriterThread work queue unwritten to buffer. That operation should be too fast to beat in the race, though.
  • If no packets exist in the buffer, Read will return STATUS_DEVICE_REMOVED.

I don't currently have a way to "reattach" an instance if the adapter is plugged back in. This wouldn't be too hard to do if it gets the same GUID as before it was removed, but I haven't tested that. It will probably have to wait for another time.

@guyharris
Copy link
Author

@guyharris guyharris commented May 7, 2020

For the Read call, we returned STATUS_CANCELLED on that line and no other, so it should be pretty consistent.

So presumably that'll show up as ERROR_OPERATION_ABORTED (995) from the ReadFile() call.

Now, I just pushed a change that ought to make things much better by distinguishing between operations that require an adapter to be present (packet write operations and some IoControl/OID things) and those that do not (getting stats, reading remaining packets in the buffer, configuring the capture session, etc.). If an operation requires an adapter that is not present, we return STATUS_DEVICE_REMOVED.

Any idea what that userland status code STATUS_DEVICE_REMOVED is mapped to?

I haven't verified if network disconnect is the same as adapter removed in this case; it's based on NDIS calling our FilterDetach handler.

"Network disconnect" in what sense? Unplugging an Ethernet, disconnecting from a Wi-Fi network, etc.?

The Read operation (PacketReceivePacket()) has the following changed behavior if the adapter is not present:

  • MinToCopy is not respected, so a Read will return whatever packets are in the buffer immediately.

That's the right thing to do - it's not as if waiting for more packets to arrive is worthwhile, given that the disappearance of the adapter means that they won't arrive.

I'm not sure if this is good enough, since the changes in 0.9991 mean that there could be packets in the WriterThread work queue unwritten to buffer. That operation should be too fast to beat in the race, though.

Presumably packets being written won't get written if the adapter is gone, so they shouldn't get wrapped back as input.

  • If no packets exist in the buffer, Read will return STATUS_DEVICE_REMOVED.

So if that means that capturing on a removable device, and removing the device, will get STATUS_DEVICE_REMOVED from PacketReceivePacket(), then if you do a capture on a removable device and unplug it, Wireshark will probably report it as "PacketReceivePacket error (XXX)" for some value of XXX, which is the Windows system error code it gets mapped to. If it gets mapped to ERROR_GEN_FAILURE, it'll show up as "The interface disappeared".

I don't currently have a way to "reattach" an instance if the adapter is plugged back in. This wouldn't be too hard to do if it gets the same GUID as before it was removed, but I haven't tested that. It will probably have to wait for another time.

That's probably not a high priority - the capture should return an error if the interface goes away, and the app should probably note that to the user, because there's no guarantee that they'll plug it in again.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Linked pull requests

Successfully merging a pull request may close this issue.

None yet
2 participants