Checking for NULL MDL Pointers

Conventional wisdom has it that you should always check a user-mode pointer for NULL before referencing it in a kernel driver. Sometimes we lose sight of that rule when dealing with DIRECT method I/O operations. This page of the WD-3 Security Notebook explains why forgetting NULL pointer checks can be a bad idea.

What's Wrong With This Picture?

Let's suppose I build a driver that uses the DIRECT method to access user buffers for IRP_MJ_READ and IRP_MJ_WRITE requests. As you know, in my AddDevice function, I'll have a line of code something like this one:

fdo->Flags |= DO_DIRECT_IO;

Setting DO_DIRECT_IO tells the I/O Manager to construct a Memory Descriptor List (MDL) for each read and write. I might plan to do something like this in a dispatch routine (note that I'm deliberately making this example trivial, just to illustrate the point):

NTSTATUS DispatchRead(PDEVICE_OBJECT fdo, PIRP Irp)
  {
  PULONG p = (PULONG) MmGetSystemAddressForMdlSafe(Irp->MdlAddress, NormalPagePriority);
  *p = 42;
  Irp->IoStatus.Status = STATUS_SUCCESS;
  Irp->IoStatus.Information = sizeof(ULONG);
  IoCompleteRequest(Irp, IO_NO_INCREMENT);
  return STATUS_SUCCESS;
  }

There's a bug here. Suppose the user-mode caller has specified zero for the data length:

char inbuf[50];
ReadFile(hDevice, &inbuf, 0);

Even though this API call specifies a valid buffer address, the fact that the data length is zero causes the I/O Manager not to create an MDL. In normal course, then, the code I showed you above will bug check. We might think to cure that problem this way:

NTSTATUS DispatchRead(PDEVICE_OBJECT fdo, PIRP Irp)
  {
  NTSTATUS status = STATUS_SUCCESS;
  __try
    {
    PULONG p = (PULONG) MmGetSystemAddressForMdlSafe(Irp->MdlAddress, NormalPagePriority);
    *p = 42;
    }
  __except(EXCEPTION_EXECUTE_HANDLER)
    {
    status = GetExceptionCode();
    }
  Irp->IoStatus.Status = status;
  Irp->IoStatus.Information = sizeof(ULONG);
  IoCompleteRequest(Irp, IO_NO_INCREMENT);
  return status;
  }

The idea is to catch the null pointer reference with the __try/__except clause.

Are you sufficiently paranoid to spot the potential problem with even the "fixed" version of this code?

Much Ado About Nothing

The problem is that a NULL pointer might not actually cause a page fault. There are ways of mapping the zero address to actual memory. I don't personally know how to do that, and I wouldn't publish instructions here if I did know. But let's take it as a given that Snidely Whiplash, our archetypal Evilly Disposed Person,¹ can contrive to map virtual memory in such a way that virtual location zero actually addresses a piece of physical memory that can be made to contain an MDL. When Snidely's application calls ReadFile with a zero length, the I/O Manager will create an IRP_MJ_READ with a NULL value for Irp->MdlAddress, just like before. Only now, when we dereference the NULL pointer, we are addressing something that looks to the Memory Manager like a perfectly good MDL. Only, no one has checked to see whether the virtual address range described by that MDL lies wholly in user-mode memory. Thus, our driver is going to happily plunk the number 42 down on top of whatever user or kernel memory Snidely has put into that unvalidated MDL.

I'm sure you can use your imagination here. If I can make my driver store one number in an arbitrary location, I can cause it to copy an entire malicious program into an arbitrary place in the kernel. I can take over the machine in a way that will be hard to spot or undo.

The fix for this problem is quite simple:

NTSTATUS DispatchRead(PDEVICE_OBJECT fdo, PIRP Irp)
  {
  NTSTATUS status = STATUS_SUCCESS;
  if (Irp->MdlAddress)
    {
    PULONG p = (PULONG) MmGetSystemAddressForMdlSafe(Irp->MdlAddress, NormalPagePriority);
    if (p)
      *p = 42;
    else
      status = STATUS_INSUFFICIENT_RESOURCES;  // i.e., not enough page table entries
    }
  else
    status = STATUS_INVALID_PARAMETER;
  Irp->IoStatus.Status = status;
  Irp->IoStatus.Information = sizeof(ULONG);
  IoCompleteRequest(Irp, IO_NO_INCREMENT);
  return status;
  }

The same analysis applies to IOCTL operations that use either METHOD_IN_DIRECT or METHOD_OUT_DIRECT, too. Both of these methods use an MDL to describe the "output" buffer for the call to DeviceIoControl. A zero-length output buffer leads to a NULL MdlAddress pointer for these IOCTLs, just as it does for reads and writes.

Do you see why it's not necessary to check Irp->MdlAddress for any special value besides zero? If you do, drop us a line at letters@wd-3.com. We'll publish the first and/or best answer we get next issue.

---

1 -- If you have absolutely no idea who I'm talking about, check out this web page: http://flyingmoose.org/moose/whiplash.htm