Re: [PATCH 1/2] ntdll: Restore stack guard and prevent stack from shrinking
Piotr Caban <piotr(a)codeweavers.com> writes:
@@ -1617,9 +1617,18 @@ BOOL virtual_handle_stack_fault( void *addr ) BYTE vprot = view->prot[((const char *)page - (const char *)view->base) >> page_shift]; if (vprot & VPROT_GUARD) { + struct _TEB *teb = NtCurrentTeb(); VIRTUAL_SetProt( view, page, page_size, vprot & ~VPROT_GUARD ); - if ((char *)page + page_size == NtCurrentTeb()->Tib.StackLimit) - NtCurrentTeb()->Tib.StackLimit = page; + if ((char *)page - page_size == teb->DeallocationStack) + teb->Tib.StackLimit = page; + else if ((char*)addr > (char*)teb->Tib.StackLimit + page_size && + teb->Tib.StackLimit == (char*)teb->DeallocationStack + page_size && + (view = VIRTUAL_FindView( (char*)teb->Tib.StackLimit, 0))) + { + vprot = view->prot[((char*)teb->Tib.StackLimit - (char*)view->base) >> page_shift]; + VIRTUAL_SetProt( view, teb->Tib.StackLimit, page_size, vprot | VPROT_GUARD); + teb->Tib.StackLimit = (char*)teb->Tib.StackLimit + page_size; + }
Why do you want to lookup the view again? -- Alexandre Julliard julliard(a)winehq.org
On 04/07/11 10:24, Alexandre Julliard wrote:
Piotr Caban<piotr(a)codeweavers.com> writes:
@@ -1617,9 +1617,18 @@ BOOL virtual_handle_stack_fault( void *addr ) BYTE vprot = view->prot[((const char *)page - (const char *)view->base)>> page_shift]; if (vprot& VPROT_GUARD) { + struct _TEB *teb = NtCurrentTeb(); VIRTUAL_SetProt( view, page, page_size, vprot& ~VPROT_GUARD ); - if ((char *)page + page_size == NtCurrentTeb()->Tib.StackLimit) - NtCurrentTeb()->Tib.StackLimit = page; + if ((char *)page - page_size == teb->DeallocationStack) + teb->Tib.StackLimit = page; + else if ((char*)addr> (char*)teb->Tib.StackLimit + page_size&& + teb->Tib.StackLimit == (char*)teb->DeallocationStack + page_size&& + (view = VIRTUAL_FindView( (char*)teb->Tib.StackLimit, 0))) + { + vprot = view->prot[((char*)teb->Tib.StackLimit - (char*)view->base)>> page_shift]; + VIRTUAL_SetProt( view, teb->Tib.StackLimit, page_size, vprot | VPROT_GUARD); + teb->Tib.StackLimit = (char*)teb->Tib.StackLimit + page_size; + }
Why do you want to lookup the view again? It's not needed (I thought it may be not valid for whole stack). I'll send fixed version.
There also should be "growing" instead of "shrinking" in commit message. It was meant to point that it's possible to change StackLimit more then once without this patch.
Piotr Caban <piotr.caban(a)gmail.com> writes:
It's not needed (I thought it may be not valid for whole stack). I'll send fixed version.
There also should be "growing" instead of "shrinking" in commit message. It was meant to point that it's possible to change StackLimit more then once without this patch.
I'm not sure what you mean, making it grow is the reason for that function. -- Alexandre Julliard julliard(a)winehq.org
On 04/07/11 11:00, Alexandre Julliard wrote:
Piotr Caban<piotr.caban(a)gmail.com> writes:
It's not needed (I thought it may be not valid for whole stack). I'll send fixed version.
There also should be "growing" instead of "shrinking" in commit message. It was meant to point that it's possible to change StackLimit more then once without this patch.
I'm not sure what you mean, making it grow is the reason for that function.
Without this patch there's following condition for stack growing: if ((char *)page + page_size == NtCurrentTeb()->Tib.StackLimit) NtCurrentTeb()->Tib.StackLimit = page; If after growing the stack application protects the memory between StackLimit and StackLimit-page_size, StackLimit will be changed again.
Piotr Caban <piotr.caban(a)gmail.com> writes:
Without this patch there's following condition for stack growing: if ((char *)page + page_size == NtCurrentTeb()->Tib.StackLimit) NtCurrentTeb()->Tib.StackLimit = page; If after growing the stack application protects the memory between StackLimit and StackLimit-page_size, StackLimit will be changed again.
StackLimit is supposed to be the last address that is unprotected, so changing it in that case would be correct. The last page should really to remain protected though, which application is modifying it? -- Alexandre Julliard julliard(a)winehq.org
On 04/07/11 11:22, Alexandre Julliard wrote:
Piotr Caban<piotr.caban(a)gmail.com> writes:
Without this patch there's following condition for stack growing: if ((char *)page + page_size == NtCurrentTeb()->Tib.StackLimit) NtCurrentTeb()->Tib.StackLimit = page; If after growing the stack application protects the memory between StackLimit and StackLimit-page_size, StackLimit will be changed again.
StackLimit is supposed to be the last address that is unprotected, so changing it in that case would be correct. The last page should really to remain protected though, which application is modifying it?
It happens when native _resetstkoflw is called when there's little memory left on the stack. I don't know if there's any real application that is calling it in that case. I thought that it's incorrect to grow the stack size above stack limit anyway.
Piotr Caban <piotr.caban(a)gmail.com> writes:
It happens when native _resetstkoflw is called when there's little memory left on the stack. I don't know if there's any real application that is calling it in that case. I thought that it's incorrect to grow the stack size above stack limit anyway.
No, the stack limit indicates the limit of the current growth. If the stack grows the limit is updated. -- Alexandre Julliard julliard(a)winehq.org
participants (2)
-
Alexandre Julliard -
Piotr Caban