MM Bugs

[0xBADC0DE]

Well, my MM is working, except for one aspect - When I create a Ring-3 MM Context, it overwrites my current Ring-0 MM Context, causing Page-Faults. I believe the error is somewhere in the code below, but I can't find it

Code: Select all

typedef struct
{
        unsigned long flags;
        unsigned long dpl;
        unsigned long cr3;
} KeMMContext;

KeMMContext* KeMemManagerCreateContext(unsigned long dpl)
{
   unsigned long index = 0;
   for(;index < 512;index++)
   {
      if(contextes[index].flags == 0)
      {
         break;
      }
   }
   if(index >= 512)
      return 0;

   unsigned long* pagedirectory = KePhysMemManagerAllocPage();

   for(index=0;index<1024;index++)
      * (KePhysMemManagerGetPage(pagedirectory) + index) = 0;
   KePagingInitPageDirectory(pagedirectory);

   contextes[index].cr3 = (unsigned long)pagedirectory;
   contextes[index].dpl = dpl;
   contextes[index].flags = 1;

   return (contextes + index);
}

void KeMemManagerFreeContext(KeMMContext* ctx)
{
   unsigned long index = 0;
   for(;index<1024;index++)
      if(!(*((unsigned long*)ctx->cr3 + index) & 1)) 
         KePhysMemManagerFreePage( (unsigned long*)(*(KePhysMemManagerGetPage((unsigned long*)ctx->cr3) + index) & ~0xFFF));
   KePhysMemManagerFreePage((unsigned long*)ctx->cr3);
   ctx->dpl = 0;
   ctx->cr3 = 0;
   ctx->flags = 0;
}

void KeInitMemManager( void )
{
   unsigned long index = 0;
   for(; index < 512;index++)
      contextes[index].flags = 0;
   for(index = 0;index<524288;index++)
      KernelMemoryMap[index] = 0;

   krnlctx = KeMemManagerCreateContext(0);
   KeMemManagerSwitchToContext(krnlctx);
}

void KeMemManagerSwitchToContext(KeMMContext* ctx)
{
   KeWriteCR3(ctx->cr3);
   KeWriteCR0(0x80000000 | KeReadCR0());
}

Could anyone help me?

[paulbarker]

In KeMemManagerCreateContext, you use the variable 'index' for 2 separate things. Consider what the value of index will be after the SECOND for loop.

Solution: Use a different variable for the second for loop.

Make sure you understand this rather than just following what I say.

[Kemp]

Additionally (not problem related though)

Code: Select all

   for(;index < 512;index++)
   {
      if(contextes[index].flags == 0)
      {
         break;
      }
   }
   if(index >= 512)
      return 0;

index can be equal to 512 if the break is never encountered, but never greater than unless something else also accidently modifies it

[Candamir]

Can index be equal 512? I mean, if the condition is that index must always stay smaller than 512... ???

[proxy]

index will be equal to 512 after the last iteration of the loop. It must be since this is the condition that would stop the loop, (this is why the condition works in the first place, "the loop will execute while index is less than 512").

The reason why this works is because index is declared outside of the for loop scope (in c++ and c99 for(int i;;){} i does not exist outside of the for loops block).

proxy

[Kemp]

Code: Select all

KeMMContext* KeMemManagerCreateContext(unsigned long dpl)
{
   unsigned long index = 0;
   while ((index < 512) && (contextes[index].flags != 0))
      index++;

   if (index == 512)
      return 0;

   unsigned long* pagedirectory = KePhysMemManagerAllocPage();

   for (index = 0; index < 1024; index++)
      *(KePhysMemManagerGetPage(pagedirectory) + index) = 0;

   KePagingInitPageDirectory(pagedirectory);

   contextes[index].cr3 = (unsigned long)pagedirectory;
   contextes[index].dpl = dpl;
   contextes[index].flags = 1;

   return (contextes + index);
}

This doesn't fix any bugs (especially pay attention to your use of index in the last four lines), but I just don't like commands that break program flow (such as the cunningly named break) and inconsistent styles.

Also, you are creating pagedirectory as a pointer to an unsigned long and then casting it as an unsigned long, I hope getting the address of the value is the intended goal...

Edit:
On further thinking, of course it is.

[Candy]

Code: Select all

   for (index = 0; index < 1024; index++)
      *(KePhysMemManagerGetPage(pagedirectory) + index) = 0;

   KePagingInitPageDirectory(pagedirectory);

   contextes[index].cr3 = (unsigned long)pagedirectory;
   contextes[index].dpl = dpl;
   contextes[index].flags = 1;

   return (contextes + index);
}

Index is always 1024 exactly in the last 4 lines.

[Kemp]

That's why we told him to think about how it was used ;D

[Candy]

That's why we told him to think about how it was used ;D

I figured as much, but at times you can throw people a bone. You can just tell them what's wrong and let them figure out how to solve it. If I don't understand why it isn't working, a vague hint about something that isn't directly related to the code I should change isn't going to help much.

[Kemp]

Not to be picky, but

In KeMemManagerCreateContext, you use the variable 'index' for 2 separate things. Consider what the value of index will be after the SECOND for loop.

Solution: Use a different variable for the second for loop.

I counted that as telling him, I was just reminding that the problem was there. Sorry if I seemed to be being difficult.

[Candy]

Not to be picky, but

I counted that as telling him, I was just reminding that the problem was there. Sorry if I seemed to be being difficult.

Well.... you're right, I'm just being repetitive... He hasn't reacted though...