Preventing race condition

NTDEV
1

Hello guys,

Here is a scenario (I guess pretty typical) which I am working on.

There is driver component that has:

  • registered ps notify routine to keep track of incoming processes in system
  • on create under certain conditions e add process
  • on terminate we remove process (if exists)

So far nothing very uncommon.

Now - I want to add IOCTL that can eventually pass PID/SK of process that should be added to list - at first nothing very much problematic you have handler where you do PsLookupProcessById then add process (ref count +1). However here there seem to be potential race as service that is sending IOCTL is sending external PID right? So it may happen between PsLookupProcessById and adding to driver internal list there can happen termination. I guess OS is not synchronizing it very much thus I wanted to ask you if there is any mechanism that I could use in order to add such process without having memory leak?

I was thinking about ExAcquireRundownProtection function (wdm.h) - Windows drivers | Microsoft Learn so that code would be

//pseudo code
void myIOCTL(const HANDLE pid)
{
   PsLookupProcessById(pid);
   ExAcquireRundownProtection();
 
   AddPidSafelyToList();

   ExReleaseRundownProtection();
   ObDereference()
}

But not sure if rundown protection will prevent/postpone termination to happen.

If not do you happen to have any idea how to do it safely? If above is bad approach I was thinking to just keeping all contexts with flags. Then add woudl be on ps create, remove of ps terminate and in handler I would just make find (if not found skip) and update - all atomically and contexts ref counted.

What do you think?

All the best and thank you for help.

2

Here, take a read. This should clear things up.

Also, dereference the object when you are done with it. Not after you add it to list, which you still might access unknowingly.

Dereference it when its terminated, after removing it from the list.

3

Thank you for answer and referenced material, but explain me how shared/exclusive spinlock will prevent termination of a process while I am about adding it to list?

In Ps routine at create and terminate when I am about adding process to list all is synchronized. Moreover when there is access to list there is also synchronization. There is no issue with it. Issue is that I have no control over process (identified by PID in IOCTL handler) lifetime so it may happen then when I am inside handler of IOCTL I will make lookup and in meantime termination of process will happen then my handler thread will try to insert to list and I will never have option to delete this PID from list.

Here is example:

// pseudo code

// executed from Thread 2
void psnotify()
{
   //...

   // this is process termination

   // STEP 2 - Thread 2 call callback and remove process from list
   AcquireSpinLock();
   DeleteFromList(pid);
   ReleaseSpinLock();

   //...
}


// executed from Thread 1
void ioctlHandler(pid)
{
   
   // STEP 1 - Thread 1 take control and we lookup
   process = PsLookupProcessById(pid)
   
   // STEP 3 - Thread 1 is waiting on Thread 2 in spin lock (Thread 2 is removing)
   AcquireSpinLock();

   // STEP 4 - here is an issue. There is no such process anymore
   InsertToList(process);
   ReleaseSpinLock();

   
}

So we have 2 threads:

  • Thread 1 is usermode service thread
  • Thread 2 is executed in last thread of process identified by PID

Scenario is this:

  • [Thread 1] Perform ps lookup to reference EPROCESS of process identified by PID
  • [Thread 1] Process is happily found so it goes to AcquireSpinLock (or whatever exclusive lock)
  • [Thread 1] This acquisition waits, because…
  • [Thread 2] Executes at the moment and acquired spin lock to delete this PID from list
  • [Thread 2] release spin lock
  • [Thread 1] is acquiring lock and perform Insert + release lock

As you see this will not work correct. there will be leak.

Thanks in advance.

4

Simply put the lock before PsLookupProcessById. This way, the notify thread will have to wait until you really are done adding it.

AcquireLock();
process = PsLookupProcessById(pid);
InsertToList(process);
ReleaseLock();
5

Make sense!

But..

That would require quiet big changes. Code I am working on uses spinlocks still (old thing) and PsLookupProcessById can be called <= APC

Thanks

6

Use wait based locks, then. Is that feasible?