When working with Mouse and Keyboard filter drivers I have been suggested many different methods for handling the InputDataConsumed parameter inside their callback functions. Firstly, I know that InputDataConsumed always starts out as 0 and improper use of this parameter can result in your callback function looping, to packet drops being ignored etc. Essentially I want to know what I should set InputDataConsumed when I either A delete a packet, B modify a packet, C let said packet pass on its own, or D let all the packets go all at once.
VOID KbFilter_ServiceCallback(IN PDEVICE_OBJECT DeviceObject, IN PKEYBOARD_INPUT_DATA InputDataStart, IN PKEYBOARD_INPUT_DATA InputDataEnd, IN OUT PULONG InputDataConsumed)
{
static DOUBLE PreviousTime = 0;
WDFDEVICE hDevice = WdfWdmDeviceGetWdfDeviceHandle(DeviceObject);
PDEVICE_EXTENSION devExt = FilterGetData(hDevice);
if (Initialized) {
ULONG packetsConsumed = 0;
PKEYBOARD_INPUT_DATA current = InputDataStart;
while (current < InputDataEnd) {
BOOLEAN isKeyRelease = (current->Flags & KEY_BREAK) == KEY_BREAK;
BOOLEAN consume = TRUE;
if (isKeyRelease && current->MakeCode == (USHORT) ReconnectKey) {
consume = FALSE;
DebugMessage("CLICK RECONNECT KEY PacketCount:%d \n", packetCount);
}
if (isKeyRelease && current->MakeCode == (USHORT) ToggleKey) {
LONG oldValue, newValue;
do { // Thread safe way to set the enabled state to its opposite (invert it)
oldValue = Enabled;
newValue = oldValue == 0 ? 1 : 0;
// Attempt to set Enabled to newValue if it's still oldValue
} while (InterlockedCompareExchange(&Enabled, newValue, oldValue) != oldValue);
}
// This will check our ExemptMap and if it contains the pressed key we will allow it to pass by
size_t exemptMapSize = Exempt_Map.size;
if (exemptMapSize > 0 && hashmap_contains_key(&Exempt_Map, ¤t->MakeCode)) consume = FALSE;
if (Enabled && consume) { // This represents case A where I want to delete a packet
packetsConsumed++;
// This will ensure there is no way to determine what this packet contained since the pointer is still valid despite being denoted as consumed
current->UnitId = 0;
current->MakeCode = 0;
current->Flags = 0;
current->Reserved = 0;
current->ExtraInformation = 0;
}
else { // This represents case C where I want said packet to pass through unmodified
ULONG localInputDataConsumed = 0; // This gets updated with how many packets 'PSERVICE_CALLBACK_ROUTINE' consumes (should always be 1) but in testing its been coming up as 0 here
(*(PSERVICE_CALLBACK_ROUTINE)devExt->FilterContext.ConnectData.ClassService) (devExt->FilterContext.ConnectData.ClassDeviceObject, current, current + 1, &localInputDataConsumed);
DebugMessage("Not Consuming InputData:%lu PacketsConsumed:%lu InputDataConsumed:%lu PacketCount:%d \n", localInputDataConsumed, packetsConsumed, *InputDataConsumed, packetCount);
}
current++;
}
*InputDataConsumed = packetsConsumed;
PreviousTime = currentTime;
}
else { // This is case D aka let all packets go
DebugMessage(" Process normally Size:%llu Consumed:%lu \n", (InputDataEnd - InputDataStart), *InputDataConsumed);
(*(PSERVICE_CALLBACK_ROUTINE)(ULONG_PTR)devExt->FilterContext.ConnectData.ClassService)(devExt->FilterContext.ConnectData.ClassDeviceObject, InputDataStart, InputDataEnd, InputDataConsumed);
}
}
Now I have tried multiple things but I want to make sure I am doing it right so when and where should I update the value of InputDataConsumed in all 3 of these cases? Right now I am excluding a call to PSERVICE_CALLBACK_ROUTINE inside the code section that is designed to discard said keyboard packet but I am still not sure what InputDataConsumed should equal in all these cases.
A delete a packet: packetsConsumed++ (what you are doing in the example)
B modify a packet: assuming this is the same as case C where you pass it to the upper service callback after you modify the packet
D let all the packets go all at once: what you are doing in the example
C let said packet pass on its own
you need account for what the upper service callback processed,
packetsConsumed += localInputDataConsumed;
if (Enabled && consume) {
packetsConsumed++;
current++;
// This will ensure there is no way to determine what this packet contained since the pointer is still valid despite being denoted as consumed
current->UnitId = 0;
current->MakeCode = 0;
current->Flags = 0;
current->Reserved = 0;
current->ExtraInformation = 0;
}
else { // This represents case C where I want said packet to pass through unmodified
ULONG localInputDataConsumed = 0; // This gets updated with how many packets 'PSERVICE_CALLBACK_ROUTINE' consumes (should always be 1) but in testing its been coming up as 0 here
(*(PSERVICE_CALLBACK_ROUTINE)devExt->FilterContext.ConnectData.ClassService) (devExt->FilterContext.ConnectData.ClassDeviceObject, current, current + 1, &localInputDataConsumed);
DebugMessage("Not Consuming InputData:%lu PacketsConsumed:%lu InputDataConsumed:%lu PacketCount:%d \n", localInputDataConsumed, packetsConsumed, *InputDataConsumed, packetCount);
packetsConsumed += localInputDataConsumed;
current += localInputDataConsumed;
}
note that I moved mutation of current into both blocks of the if.
@Doron_Holan said:
A delete a packet: packetsConsumed++ (what you are doing in the example)
B modify a packet: assuming this is the same as case C where you pass it to the upper service callback after you modify the packet
D let all the packets go all at once: what you are doing in the example
C let said packet pass on its own
you need account for what the upper service callback processed,
packetsConsumed += localInputDataConsumed;
if (Enabled && consume) {
packetsConsumed++;
current++;
// This will ensure there is no way to determine what this packet contained since the pointer is still valid despite being denoted as consumed
current->UnitId = 0;
current->MakeCode = 0;
current->Flags = 0;
current->Reserved = 0;
current->ExtraInformation = 0;
}
else { // This represents case C where I want said packet to pass through unmodified
ULONG localInputDataConsumed = 0; // This gets updated with how many packets 'PSERVICE_CALLBACK_ROUTINE' consumes (should always be 1) but in testing its been coming up as 0 here
(*(PSERVICE_CALLBACK_ROUTINE)devExt->FilterContext.ConnectData.ClassService) (devExt->FilterContext.ConnectData.ClassDeviceObject, current, current + 1, &localInputDataConsumed);
DebugMessage("Not Consuming InputData:%lu PacketsConsumed:%lu InputDataConsumed:%lu PacketCount:%d \n", localInputDataConsumed, packetsConsumed, *InputDataConsumed, packetCount);
packetsConsumed += localInputDataConsumed;
current += localInputDataConsumed;
}
note that I moved mutation of current into both blocks of the if.
Ok thank you! Just to clarify why did you decide incrementing the value of current should be done through current += localInputDataConsumed rather than by what I originally had which was current++ outside of the if and else statement? Secondly, I am still not completely sure about why we are doing what we are doing. By that I mean why do we need to increment the value of InputDataConsumed in cases A - C but not for case D? Wait actually I am fairly sure case D is updating InputDataConsumed since its a pointer but I still don’t understand the purpose in updating InputDataConsumed. It seems to me that we could just set InputDataConsumed equal to the total number of packets at the top of my function since in cases A - C we just increment InputDataConsumed despite the goals of each being different. Maybe we dont do this in case a packet being sent fails which is why we read the value of localInputDataConsumed? If that is so then I assume localInputDataConsumed remains 0 after calling the service callback if said callback fails right? Anyways it seems like I always need to increment InputDataConsumed in all cases even when we consume said packet which then makes me wonder whats the point of InputDataConsumed in the first place?
Just to clarify why did you decide incrementing the value of current should be done through current += localInputDataConsumed rather than by what I originally had which was current++ outside of the if and else statement?
there is no guarantee that in case C
(*(PSERVICE_CALLBACK_ROUTINE)devExt->FilterContext.ConnectData.ClassService) (devExt->FilterContext.ConnectData.ClassDeviceObject, current, current + 1, &localInputDataConsumed);
will consume the data. so you increment current by how much it consumes (in your implementation either 0 or 1). If there are more packets after case C you have two choices when the upper callback does not consume the packet as InputDataConsumed is an absolute value based on the start, there is no way to indicate a hole in how the packets are consumed (e.g. consumed packets 1,2, 4, skipped 3):
- return immediately and report to the caller the number of previous packets consumed
- increment localInputDataConsumed and proceed iterating through the rest of the packets
in case D your filter is pass through and you are delegating all responsibility of consuming ALL input packets to the next callback. Case C is a derivative of D as you are only delegating one packet at a time.
Anyways it seems like I always need to increment InputDataConsumed in all cases even when we consume said packet which then makes me wonder whats the point of InputDataConsumed in the first place?
Yes, it is incremented for each packet consumed. To the caller indicates an ACK for successfully received and the caller will not to retry to report the packet later.