Page 1 of 1
MM Bugs
Posted: Sun Apr 16, 2006 5:00 am
by 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?
Re:MM Bugs
Posted: Sun Apr 16, 2006 3:16 pm
by 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.
Re:MM Bugs
Posted: Sun Apr 16, 2006 6:14 pm
by 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
Re:MM Bugs
Posted: Sun Apr 16, 2006 7:38 pm
by Candamir
Can index be equal 512? I mean, if the condition is that index must always stay smaller than 512... ???
Re:MM Bugs
Posted: Sun Apr 16, 2006 8:55 pm
by 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
Re:MM Bugs
Posted: Mon Apr 17, 2006 5:00 am
by 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.
Re:MM Bugs
Posted: Mon Apr 17, 2006 5:37 am
by Candy
Kemp wrote:
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.
Re:MM Bugs
Posted: Mon Apr 17, 2006 2:52 pm
by Kemp
That's why we told him to think about how it was used ;D
Re:MM Bugs
Posted: Mon Apr 17, 2006 3:09 pm
by Candy
Kemp wrote:
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.
Re:MM Bugs
Posted: Mon Apr 17, 2006 3:59 pm
by 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.
Re:MM Bugs
Posted: Mon Apr 17, 2006 4:02 pm
by Candy
Kemp wrote:
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...