pcap_sendqueue_transmit() return value - sharppcap

[chmorgan]

Hello.

I'm calling pcap_sendqueue_transmit() with no packets in the queue and seeing 1073741811 returned. I can't figure out the code path in npcap/libpcap that results in this return value. This only occurs for npcap, the test passes on both linux and windows. Using npcap 1.10 in the case where the failure occurs.

I did find PacketSendPackets in pcap-nfp.c but can't find any docs on that function, a google search doesn't turn anything useful up.

If this is a known return value it seems odd, the upper bit isn't set so it doesn't appear to be a negative value. The signatures used for pinvoke look correct.

If I do call pcap_sendqueue_transmit() even with a single packet in the queue I do get the correct value.

You can see the logs from the Azure pipelines issue here, https://dev.azure.com/chmorgan/chmorgan/_build/results?buildId=594&view=logs&j=823afbdf-f891-574b-27d0-2d0231c9608a&t=f870ef66-9f98-5e46-d80f-0a348a0d71b1

Any pointers?

Chris

[guyharris]

the test passes on both linux and windows.

Presumably the test doesn't involve pcap_sendqueue_transmit() on Linux - that function is Windows-only, and unavailable on any UN*X.

[guyharris]

I did find PacketSendPackets in pcap-nfp.c

Presumably "I did find" means "I did find a call to PacketSendPackets() in pcap-npf.c" - the source isn't there, it's in the npcap source, in packetWin7/Dll/Packet32.cpp.

but can't find any docs on that function

The only WinPcap documentation for the packet.dll library says:

Important note, read carefully!

The source code of Packet.dll is freely available and completely documented. However, packet.dll should be considered an internal API, because its purpose inside WinPcap is to be a building block for the real public API: wpcap.dll.

As a consequence, since the normal and suggested way for an application to use WinPcap is through wpcap.dll, we don't guarantee that the packet.dll API will not be changed in future releases of winpcap, and we don't provide support for this API. For the same reason, this manual doesn't contain any more the Doxygen-generated documentation of Packet.dll: the user will have to run Doxygen on his own to create it, or read the comments in the source code.

I think the same applies to Npcap.

The return value of PacketSendPackets() is the sum of a number of "bytes transmitted" values supplied by DeviceIoControl() calls to the Npcap (NPF) driver. The ioctlDeviceIoControl code is BIOCSENDPACKETSSYNC for synchronous packet transmission and BIOCSENDPACKETSNOSYNC for non-synchronous packet transmission; that's controlled by the fourth argument to PacketSendPackets(), which is the third argument to pcap_sendqueue_transmit_npf().

So that looks as if it's a driver issue.

[guyharris]

Unfortunately, the documentation for DRIVER_DISPATCH callback functions in a driver, the documentation for IRP_MJ_DEVICE_CONTROL I/O request packets (IRPs) sent to the driver for a DeviceIoControl() call, and the documentation for the IO_STATUS_BLOCK structure in an IRP aren't as helpful as I'd like:

Information

This is set to a request-dependent value. For example, on successful completion of a transfer request, this is set to the number of bytes transferred. If a transfer request is completed with another STATUS_XXX, this member is set to zero.

OK, fine, but is that the case for a DeviceIoControl() request? Is the Information member value stored in * lpBytesReturned?

[chmorgan]

@guyharris from https://github.com/nmap/npcap/blob/master/packetWin7/Dll/Packet32.cpp#L2608 does this suggest that the return value indicated success? If it were FALSE (an error occurred) the loop would have broken and TotBytesTransferred returned would be the zero value it was initialized to right?

[chmorgan]

@guyharris I did also confirm that we are calling pcap_sendqueue_transmit() with the below values:

            var pcap_queue = new pcap_send_queue
            {
                maxlen = (uint)buffer.Length,      value of: 1024
                len = (uint)CurrentLength,           value of: 0
                ptrBuff = new IntPtr(buf)             value of: 49629236
            };

and the call to pcap_sendqueue_transmit() returns 1073741811.

The code for this is at https://github.com/chmorgan/sharppcap/blob/master/SharpPcap/LibPcap/SendQueue.cs#L206

In terms of how to resolve, should we go the route of avoiding calls to pcap_sendqueue_transmit() with a length of zero and make a note that there appears to be a bug there or is there some chance someone will be able to resolve the issue?

I certainly don't expect someone to pick this up and resolve, just trying to gauge how actively we might want to track or check back given that calling the function with a zero length seems like a bit of a corner case.

[kayoub5]

Looking at Npcap source code (specially PacketSendPackets), there seems to be a missing check for empty queue, the transmission function starts sending the first packet without checking the queue content.

This causes an Attempt to access memory outside filled packets bounds.

See here TotBytesTransfered >= Size is done after sending first packet. https://github.com/nmap/npcap/blob/10d4de94fdfa7ec4d84ba3311c41c95226473a75/packetWin7/Dll/Packet32.cpp#L2598-L2615

[dmiller-nmap]

1073741811 is 0xC000000D, which is STATUS_INVALID_PARAMETER. The function which processes packet queues in the driver, NPF_BufferedWrite, has many places that this value can be returned:

• if the input buffer is NULL
• if the buffer size is too small to hold a BPF header
• If the BPF header specifies a 0-length packet
• if the BPF header specifies a packet length that doesn't fit in the input buffer
• if the packet is larger than the MTU of the adapter.

So this is the expected return value when calling PacketSendPackets with an empty queue. We could and probably should add a preliminary check for this condition to avoid bothering the driver, but the result would be the same.

@guyharris, yes, the Information member is returned in *lpBytesReturned. For all of Npcap's IoCtls, we set this to 0 unless we need to return a size: either the size read or written, or a required buffer size when the return value is STATUS_BUFFER_TOO_SMALL.

[kayoub5]

@dmiller-nmap I think it would be better to either document STATUS_INVALID_PARAMETER, or make pcap_sendqueue_transmit return 0 in this case, to match pcap_sendqueue_transmit documentation:

The return value is the amount of bytes actually sent. If it is smaller than the size parameter, an error occurred during the send. The error can be caused by a driver/adapter problem or by an inconsistent/bogus send queue.

[guyharris]

I'm calling pcap_sendqueue_transmit() with no packets in the queue and seeing 1073741811 returned.

So you call pcap_sendqueue_transmit() with no packets in the queue and it returns 1073741811, not 0?

1073741811 is 0xC000000D, which is STATUS_INVALID_PARAMETER.

But I don't see any obvious way in which PacketSendPackets() would return an NT status code rather than a count of bytes transmitted. It returns either 0 or TotBytesTransfered. TotBytesTransfered is initialized to 0; so if the first DeviceIoControl() call fails, it will break out of the loop and return TotBytesTransfered, so it'll return 0.

(BTW, there's a comment before the DeviceIoControl() call that says "TODO Res is NEVER checked, this is REALLY bad." - which is false, as Res is checked immediately after the DeviceIoControl() call.)

[guyharris]

OK, here's the problem.

NPF_IoControl() returns an NTSTATUS value, with STATUS_SUCCESS meaning "no problem" and another status meaning an error.

For BIOCSENDPACKETSSYNC and BIOCSENDPACKETSNOSYNC, it does:

    WriteRes = NPF_BufferedWrite(Irp,
        (PUCHAR) Irp->AssociatedIrp.SystemBuffer,
        IrpSp->Parameters.DeviceIoControl.InputBufferLength,
        SyncWrite);

...

    if (WriteRes >= 0)
    {
        SET_RESULT_SUCCESS(WriteRes);
    }
    else
    {
        SET_FAILURE_CUSTOM(-WriteRes);
    }
    break;

...

return Status;

NPF_BufferedWrite() either returns a byte count on success, or the negative of an NTSTATUS on failure.

Unfortunately, for warning and error NTSTATUS values, the uppermost bit of the status code is 1, as per the format of NTSTATUS values, which means that...

...the negative of those NTSTATUS values doesn't have the uppermost bit set, and thus isn't a negative number.

I.e., if (WriteRes >= 0) doesn't test the difference between success and failure - WriteRes >= 0 is true on success (if less than 2^31 bytes were written) and on a failure indicated by returning the negative of an NTSTATUS value.

So returning an NTSTATUS value gets reported as a success.

If NPF_BufferedWrite() will never write more than 2^31-1 bytes of data, and will never return a STATUS_SEVERITY_SUCCESS or STATUS_SEVERITY_INFORMATIONAL NTSTATUS value, then it could return the NTSTATUS value, rather than the negative of the NTSTATUS value, and its caller would do something such as

    if (!(WriteRes & 0x80000000))
    {
        SET_RESULT_SUCCESS(WriteRes);
    }
    else
    {
        SET_FAILURE_CUSTOM(WriteRes);
    }
    break;

Otherwise, perhaps NPF_BufferedWrite() needs to be passed a pointer to an NTSTATUS variable as an additional argument, return some special indication, such as 0, on failure, and set the pointed-to NTSTATUS variable to the error status.