[ros-dev] [ros-diffs] [tkreuzer] 40963: MmGrowKernelStack: Don't assert, but fail, when the kernel stack can't grow any more. Fixes a crash with recursive user calls. See issue #4060 for more details.

[Timo Kreuzer]

Ok, I'm sure you know what you are talking about.

I was misled by the comment (/* Make sure we have reserved space for our
grow */) which should probably be /* Make sure the stack didn't overflow */

The behavious on Windows is described here:
http://news.jrsoftware.org/news/toolbar2000/msg07779.html

So on windows KiCallUserModecalls calls MmGrowStack and when that fails
with STATUS_STACK_OVERFLOW (yes I used a wrong status), KiCallUserMode
fails.
It does not state though what the exact check is or whether
KiCallUserMode would also check the Size before, but the latter seemes
to be redundand to me. So I there seems to be a different check.

Would you agree with this?
---
    /* Make sure the stack did not overflow */
    ASSERT(((PCHAR)Thread->Tcb.StackBase - (PCHAR)Thread->Tcb.StackLimit) <=
           (KERNEL_LARGE_STACK_SIZE + PAGE_SIZE));

    /* Check if we have reserved space for our grow */
    if (Thread->Tcb.StackBase - Thread->Tcb.StackLimit +
KERNEL_STACK_SIZE >
        KERNEL_LARGE_STACK_SIZE)
    {
        return STATUS_STACK_OVERFLOW;
    }
---

Regards,
Timo

Alex Ionescu schrieb:
> The code SHOULD assert.
> This is a hack.
>
> Best regards,
> Alex Ionescu
>
>
> On Sun, May 17, 2009 at 11:51 PM, <tkreuzer at svn.reactos.org> wrote:
>
>   
>> Author: tkreuzer
>> Date: Mon May 18 01:51:31 2009
>> New Revision: 40963
>>
>> URL: http://svn.reactos.org/svn/reactos?rev=40963&view=rev
>> Log:
>> MmGrowKernelStack: Don't assert, but fail, when the kernel stack can't grow
>> any more. Fixes a crash with recursive user calls.
>> See issue #4060 for more details.
>>
>> Modified:
>>    trunk/reactos/ntoskrnl/mm/procsup.c
>>
>> Modified: trunk/reactos/ntoskrnl/mm/procsup.c
>> URL:
>> http://svn.reactos.org/svn/reactos/trunk/reactos/ntoskrnl/mm/procsup.c?rev=40963&r1=40962&r2=40963&view=diff
>>
>> ==============================================================================
>> --- trunk/reactos/ntoskrnl/mm/procsup.c [iso-8859-1] (original)
>> +++ trunk/reactos/ntoskrnl/mm/procsup.c [iso-8859-1] Mon May 18 01:51:31
>> 2009
>> @@ -259,8 +259,11 @@
>>     PETHREAD Thread = PsGetCurrentThread();
>>
>>     /* Make sure we have reserved space for our grow */
>> -    ASSERT(((PCHAR)Thread->Tcb.StackBase - (PCHAR)Thread->Tcb.StackLimit)
>> <=
>> -           (KERNEL_LARGE_STACK_SIZE + PAGE_SIZE));
>> +    if (((PCHAR)Thread->Tcb.StackBase - (PCHAR)Thread->Tcb.StackLimit) >
>> +           (KERNEL_LARGE_STACK_SIZE + PAGE_SIZE))
>> +    {
>> +        return STATUS_NO_MEMORY;
>> +    }
>>
>>     /*
>>      * We'll give you three more pages.
>>
>>
>>     
>
>   
> ------------------------------------------------------------------------
>
> _______________________________________________
> Ros-dev mailing list
> Ros-dev at reactos.org
> http://www.reactos.org/mailman/listinfo/ros-dev

-------------- next part --------------
An HTML attachment was scrubbed...
URL: http://www.reactos.org/pipermail/ros-dev/attachments/20090518/b4b92108/attachment-0001.html