PacketGetStats() sometimes fails with ERROR_OPERATION_ABORTED

[guyharris]

A number of Wireshark bugs have been filed due to a

Can't get packet-drop statistics: PacketGetStats error: The I/O operation has been aborted because of either a thread exit or an application request. (995)

error.

That comes from a pcap_stats() call in the dumpcap program (the program that does traffic capture for Wireshark and TShark). That ultimately turns into a BIOCGSTATS DeviceIoControl() call; that call isn't overlapped, so:

• I'm not sure how it'd be aborted by an application request unless another thread closes the HANDLE in the pcap_t, and I don't see any path to close the pcap_t before the pcap_stats() call;
• the thread making the call hasn't exited as 1) it's in the middle of the call and 2) when the call fails it continues and reports the error.

Looking at the driver (from current Git master), I don't see any obvious way that ERROR_OPERATION_ABORTED would be returned.

I can reproduce this with Wireshark 3.0.2 and Npcap 0.995, running on Windows 7 in a virtual machine on my Mac, with a USB Wi-Fi adapter attached to the VM, by starting a capture on the Wi-Fi adapter and then suspending and restarting the virtual machine.

The first error I get is

"The network adapter on which the capture was being done is no longer running; the capture has stopped."

which is probably dumpcap's translation of a "read error: PacketReceivePacket failed" error reported by Npcap (dumpcap maps various errors reported by libpcap/WinPcap/Npcap for an interface going down into that message; I need to arrange for a way for pcap to return a common "interface went down, capture stopped" error).

When I dismiss that dialog, I get the

"Can't get packet-drop statistics: PacketGetStats error: The I/O operation has been aborted because of either a thread exit or an application request. (995)"

error.

[guyharris]

I could also reproduce it by capturing on the (virtual) Bluetooth device.

[guyharris]

I suspect this is a suspend/resume issue of some sort.

[dmiller-nmap]

When Windows suspends, NDIS filter driver modules are detached from the adapters, which means any open pcap handles ought to be invalid. I'm not sure how to communicate this to libpcap-using software. Maybe there's a useful error (GetLastError) from the PacketReceivePacket call? That basically returns the result of ReadFile on the handle, but I don't see the pcap's errbuf being filled in pcap_read_npf. Basically, once there's been a suspend, you have to re-open the handle with PacketCloseAdapter and PacketOpenAdapter.

[guyharris]

When Windows suspends, NDIS filter driver modules are detached from the adapters, which means any open pcap handles ought to be invalid.

"Invalid" in the sense that the HANDLE stored in the pcap_t is still a valid device handle but many calls passing that HANDLE to a Windows API will cause the driver for that device to return errors, or in the sense that the HANDLE itself will be invalid and errors will be returned at the Windows API layer before it even makes it to the driver?

I'm not sure how to communicate this to libpcap-using software. Maybe there's a useful error (GetLastError) from the PacketReceivePacket call? That basically returns the result of ReadFile on the handle,

If either 1) the USB Wi-Fi device is unplugged from the system or 2) I do a suspend and resume on the virtual machine, ERROR_GEN_FAILURE is returned by PacketReceivePacket().

Given that NPF_IoControl() returns STATUS_CANCELLED if NPF_StartUsingOpenInstance(Open) returns FALSE, I'm guessing that the HANDLE is still valid, so calls still make it to your driver, but your driver returns an error. In that particular case, it might be a good idea to still allow BIOCGSTATS ioctls, so that if a capture is forcibly terminated due to the adapter going away, you can still get the statistics on packets captured up to the point at which the device became unavailable.

but I don't see the pcap's errbuf being filled in pcap_read_npf.

It is, but it's not filled in with a string for the error code, it's just filled in with a generic error; I changed that to find out what error was returned from PacketReceivePacket() in the device-removed and suspend/resume cases, but haven't checked that in yet, because I wanted to treat that particular error specially, as the equivalent errors are treated in the *BSDs and macOS.

[guyharris]

Also, is there some way for some part of the NPF kernel code to catch suspends or resumes and re-plumb the filter so that capturing can continue in the suspend/resume case (it obviously can't continue if you unplug the device from the system)?

[dmiller-nmap]

Ok, this is starting to make sense! The Packet API gets NPF device handles with CreateFile, reads them with ReadFile, writes packets with WriteFile, and closes with CloseFile. These are all delivered to the driver as IRPs, and handled appropriately. NDIS binds the filter driver modules to adapters, detaches them on power events or unplug events, reattaches them on resume, etc. Npcap is getting mixed up between the two sides of things (device driver and filter driver), so even though the device is still open by processes, it burns down the filter driver side and returns inconsistent things like STATUS_CANCELLED.

I think there may be a way to preserve state on the filter driver side and re-attach the open device handles to the appropriate adapters when they are restored, but it would require a lot of changes and still we'd need a way to tell the owning process that the handle isn't available right now. I'm not sure that's worth it.

I will be checking in a change that might help a bit: in the NPF_ReleaseOpenInstanceResources routine that basically tears down an "Open instance" (device handle associated with a filter module instance on an adapter), I'm setting the OpenSignature to 0 so that NPF_IsOpenInstance returns false. In the places this is checked, it results in the pending IRP returning with STATUS_INVALID_HANDLE, which ought to make more sense.

[guyharris]

As long as the device driver is open, a DeviceIoControl() call with BIOCGSTATS should be made to work, even if this means storing the statistics in a structure that's associated with the device driver rather than the filter driver; otherwise, if a capture fails due to the device being unplugged, you can't get the packet statistics, such as packet drops, from the capture.

It would also be good if the packet buffer also remained available, so that ReadFile() call on a non-empty packet buffer can return the last of the packets captured; once the buffer is empty, if the filter driver is no longer attached, an appropriate error should be returned.

Replumbing the filter driver is less important - an application doing a long-running capture should try to prevent sleep during the capture.

[guyharris]

The appropriate error for the interface going away would probably be ERROR_DEV_NOT_EXIST - "The specified network resource or device is no longer available." That's correct for the "hot-pluggable interface was unplugged" case; it's not ideal for the "sleep/wakeup dismantled the plumbing" case, but if that can't be distinguished from the "hot-pluggable device was unplugged" case, so it goes (and "sleep/unplug/wakeup" is an example of the "hot-pluggable interface was unplugged" case).

[dmiller-nmap]

Yes, that seems doable. We have a release in testing already, so it won't be in 0.997, but I hope to work on it for 0.998. And one of these days, I suppose we'll have to have a 1.0 release :)

[guyharris]

Is there anything else - other than a sleep/wakeup pair or the removal of the device - that would cause 1) ReadFile() on the device to fail and 2) a subsequent BIOCGSTATS DeviceIoControl() call to fail with the "cancelled" error? Somebody claims that a capture that Just Stopped 2 minutes after it started and that the machine didn't go to sleep.

[dmiller-nmap]

This should be fixed in Npcap 0.9983, released today. Statistics and some of the other functions can still be called on a paused or removed network adapter, and it may be possible to continue capturing after a sleep, though I haven't tested that particularly.

If there are additional changes related to this issue that you would like to see, please open a new issue to describe them. Thanks again!