The proper way to free MDL after MmBuildMdlForNonPagedPool

I am following the logic of WFP Stream Edit Callout Driver Sample from Microsoft.

I am concerned by the way they free MDL.

The pseudocode is:

Allocation and use:
Buffer = ExAllocatePoolWithTag(NonPagedPool, DataLength, STMEDIT_TAG_MDL_DATA);
mdl = IoAllocateMdl(Buffer, DataLength, FALSE, FALSE, NULL);
MmBuildMdlForNonPagedPool(mdl);
FwpsAllocateNetBufferAndNetBufferList(?, mdl, ?);
FwpsStreamInjectAsync(?, StreamEditInjectCompletionFn, mdl);

The MDL gets freed at StreamEditInjectCompletionFn callback:
if (mdl != NULL)
{
ExFreePoolWithTag(mdl->MappedSystemVa, STMEDIT_TAG_MDL_DATA);
IoFreeMdl(mdl);
}
FwpsFreeNetBufferList(NetBufferList);

The code is available here:
https://github.com/Microsoft/Windows-driver-samples/blob/aa6e0b36eb932099fa4eb950a6f5e289a23b6d6e/network/trans/stmedit/sys/StreamEdit.c

Is it all right to do the call like this?
ExFreePoolWithTag(mdl->MappedSystemVa, STMEDIT_TAG_MDL_DATA);

Thanks!

The code is being cute by using its knowledge of how the MDL code works (it
could really use a comment).

By way of implementation, MmBuildMdlForNonPagedPool sets the MappedSystemVa
to the address specified in the call to IoAllocateMdl. In this case, it’s a
non-paged pool allocation allocated by the driver.

At some point the driver needs to free both the non-paged pool allocation
and the MDL. Instead of carrying around the non-paged pool pointer AND the
MDL, the driver just “knows” that the MDL contains the pool pointer and
grabs it from there.

It would have been at least slightly clearer to retrieve the pointer by
using MmGetSystemAddressForMdlSafe. The fact that this returns the same
pointer passed when you allocated the MDL is at least documented:

“Passing an MDL built by MmBuildMdlForNonPagedPool to the
MmGetSystemAddressForMdlSafe routine is allowed. The
MmGetSystemAddressForMdlSafe call, in this case, simply returns the starting
virtual address of the buffer that is described by the MDL.”

https://docs.microsoft.com/en-us/windows-hardware/drivers/ddi/content/wdm/nf-wdm-mmbuildmdlfornonpagedpool

(And, yes, I know MmGetSystemAddressForMdlSafe is just a macro, but that’s
beside the point)

-scott
OSR
@OSRDrivers

Next Seminar: Windows Internals and Software Driver Development
9-13 April 2018, Sterling, VA
https://www.osr.com/seminars/software-drivers/

The real issue here is that you should free the MDL first, then ExFreePool, otherwise you have an MDL pointing to memory you don’t own.

Is that really an issue here or just a matter of taste? The driver allocated and owns both the MDL and the buffer.

-scott
OSR
@OSRDrivers

>The driver allocated and owns both the MDL and the buffer.

Yes, but after ExFreePool is called the driver doesn’t own the buffer anymore, while the MDL refers to it. For a non-paged buffer it’s most likely a non-issue (unless MS decides to track references to non-paged pages). Might be an issue in more exotic cases.

Thanks for the info.

So the correct way to do this is the following, right?

PVOID buf = mdl->MappedSystemVa;
IoFreeMdl(mdl);
ExFreePoolWithTag(buf, STMEDIT_TAG_MDL_DATA);
FwpsFreeNetBufferList(NetBufferList);

I too am having questions about order of deletion.
Most likely I am copying beyond the bounds of memory which I have found that sometimes, when running on a kernel debug host session, it doesn’t complain about it until you free the memory and have gotten, depending upon the scenario, various errors about “Page Pool Corrupt” and the like.
However, I am back at the same place where I am freeing the associated NBL, MDL, and Packet Data Context with the result ending up in an obtuse “Corrupt or Already Free” memory crashes.

Since there were no replies/confirmations posted on Sergey’s question, I figured I would bump this post to see if anyone who knows for sure the order of operations could confirm.

Now I went ahead and did a quick double check on some of the WDK 10 Samples and within the WPFSampler example and looking at the CompleteAdvancedPacketInjection method

It looks like it calls FwpsFreeNetBufferList before destroying the MDL and POOL via a call to AdvancedPacketInjectionCompletionDataDestroy.

Microsoft’s WFPSampler has the following order of operations:
1.) Free NBL → CompleteAdvancedPacketInjection
2.) Free MDL -->AdvancedPacketInjectionCompletionDataDestroy
3.) Free Pool -->AdvancedPacketInjectionCompletionDataDestroy

Sergey has the following order of operations:
1.) Free the MDL —> IoFreeMdl(mdl);
2.) Free the Pool —> ExFreePoolWithTag(buf, STMEDIT_TAG_MDL_DATA);
3.) Free the NBL —> FwpsFreeNetBufferList(NetBufferList);

So the MDL prior to Pool makes sense, but the FwpsFreeNetBufferList seems to vary.

My Question Is:
Which of the two order of operations are correct/recommended?
(Free NBL first or last?)

For anyone interested, I have been using the above “WPFSampler” order of operations and have not experienced any issues. So, it would appear that one should: 1.) Free the NBL 2.) Free the associated MDL(s) 3.) Free the MDL(s) allocated pool(s)

My Question Is: Which of the two order of operations are correct/recommended?

Well, as long as freeing X does not open the possibility of Y (which has not yet been freed) getting reused behind the scenes, the order of operations does not really matter, don’t you think. Therefore, both ways seem to be OK in this particular situation.

OTOH, I would rather suggest freeing in the reverse order of allocations. Doing things this way is guaranteed to save you from some unexpected “surprises” that you may get if X and Y are inter-related in some “obscure” ways that may be not-so-obvious to someone who is unfamiliar with the internal implementation details of the target functions. Certainly, it may be insignificant and irrelevant in context of this particular situation, but still it generally seems to be just a good rule of the thumb…

Anton Bassov