* [PATCH] MdePkg/BaseLib AARCH64: terminate stack frame list on stack switch
@ 2016-09-09 7:21 Ard Biesheuvel
2016-09-09 11:18 ` Leif Lindholm
0 siblings, 1 reply; 4+ messages in thread
From: Ard Biesheuvel @ 2016-09-09 7:21 UTC (permalink / raw)
To: edk2-devel, liming.gao; +Cc: leif.lindholm, Ard Biesheuvel
When switching to the DXE phase stack, set the frame pointer to zero so
that code walking the stack frame will not try to access stack frames\
belonging to the old stack.
Contributed-under: TianoCore Contribution Agreement 1.0
Signed-off-by: Ard Biesheuvel <ard.biesheuvel@linaro.org>
---
MdePkg/Library/BaseLib/AArch64/SwitchStack.S | 1 +
1 file changed, 1 insertion(+)
diff --git a/MdePkg/Library/BaseLib/AArch64/SwitchStack.S b/MdePkg/Library/BaseLib/AArch64/SwitchStack.S
index 2bce9c998f4f..c3ac8d7e4dfe 100644
--- a/MdePkg/Library/BaseLib/AArch64/SwitchStack.S
+++ b/MdePkg/Library/BaseLib/AArch64/SwitchStack.S
@@ -40,6 +40,7 @@ InternalSwitchStackAsm (
);
**/
ASM_PFX(InternalSwitchStackAsm):
+ mov x29, #0
mov x30, x0
mov sp, x3
mov x0, x1
--
2.7.4
^ permalink raw reply related [flat|nested] 4+ messages in thread* Re: [PATCH] MdePkg/BaseLib AARCH64: terminate stack frame list on stack switch
2016-09-09 7:21 [PATCH] MdePkg/BaseLib AARCH64: terminate stack frame list on stack switch Ard Biesheuvel
@ 2016-09-09 11:18 ` Leif Lindholm
2016-09-09 12:03 ` Ard Biesheuvel
0 siblings, 1 reply; 4+ messages in thread
From: Leif Lindholm @ 2016-09-09 11:18 UTC (permalink / raw)
To: Ard Biesheuvel; +Cc: edk2-devel, liming.gao
On Fri, Sep 09, 2016 at 08:21:26AM +0100, Ard Biesheuvel wrote:
> When switching to the DXE phase stack, set the frame pointer to zero so
> that code walking the stack frame will not try to access stack frames\
Trailing '\'.
> belonging to the old stack.
Do you mean that code will check for zero and stop processing, or that
it will be accessing rubbish instead of parsing a valid-looking frame?
Either is an improvement, but if it is the latter I would prefer it
more explicitly stated.
You can fix up on commit:
Reviewed-by: Leif Lindholm <leif.lindholm@linaro.org>
> Contributed-under: TianoCore Contribution Agreement 1.0
> Signed-off-by: Ard Biesheuvel <ard.biesheuvel@linaro.org>
> ---
> MdePkg/Library/BaseLib/AArch64/SwitchStack.S | 1 +
> 1 file changed, 1 insertion(+)
>
> diff --git a/MdePkg/Library/BaseLib/AArch64/SwitchStack.S b/MdePkg/Library/BaseLib/AArch64/SwitchStack.S
> index 2bce9c998f4f..c3ac8d7e4dfe 100644
> --- a/MdePkg/Library/BaseLib/AArch64/SwitchStack.S
> +++ b/MdePkg/Library/BaseLib/AArch64/SwitchStack.S
> @@ -40,6 +40,7 @@ InternalSwitchStackAsm (
> );
> **/
> ASM_PFX(InternalSwitchStackAsm):
> + mov x29, #0
> mov x30, x0
> mov sp, x3
> mov x0, x1
> --
> 2.7.4
>
^ permalink raw reply [flat|nested] 4+ messages in thread
* Re: [PATCH] MdePkg/BaseLib AARCH64: terminate stack frame list on stack switch
2016-09-09 11:18 ` Leif Lindholm
@ 2016-09-09 12:03 ` Ard Biesheuvel
2016-09-09 12:10 ` Leif Lindholm
0 siblings, 1 reply; 4+ messages in thread
From: Ard Biesheuvel @ 2016-09-09 12:03 UTC (permalink / raw)
To: Leif Lindholm; +Cc: edk2-devel-01, Gao, Liming
On 9 September 2016 at 12:18, Leif Lindholm <leif.lindholm@linaro.org> wrote:
> On Fri, Sep 09, 2016 at 08:21:26AM +0100, Ard Biesheuvel wrote:
>> When switching to the DXE phase stack, set the frame pointer to zero so
>> that code walking the stack frame will not try to access stack frames\
>
> Trailing '\'.
>
>> belonging to the old stack.
>
> Do you mean that code will check for zero and stop processing, or that
> it will be accessing rubbish instead of parsing a valid-looking frame?
>
I don't understand this question. If it is zero, it will stop
processing. If it is not zero, it will proceed, and potentially
traverse stack frames in memory that is now owned by someone else.
> Either is an improvement, but if it is the latter I would prefer it
> more explicitly stated.
>
> You can fix up on commit:
> Reviewed-by: Leif Lindholm <leif.lindholm@linaro.org>
>
>> Contributed-under: TianoCore Contribution Agreement 1.0
>> Signed-off-by: Ard Biesheuvel <ard.biesheuvel@linaro.org>
>> ---
>> MdePkg/Library/BaseLib/AArch64/SwitchStack.S | 1 +
>> 1 file changed, 1 insertion(+)
>>
>> diff --git a/MdePkg/Library/BaseLib/AArch64/SwitchStack.S b/MdePkg/Library/BaseLib/AArch64/SwitchStack.S
>> index 2bce9c998f4f..c3ac8d7e4dfe 100644
>> --- a/MdePkg/Library/BaseLib/AArch64/SwitchStack.S
>> +++ b/MdePkg/Library/BaseLib/AArch64/SwitchStack.S
>> @@ -40,6 +40,7 @@ InternalSwitchStackAsm (
>> );
>> **/
>> ASM_PFX(InternalSwitchStackAsm):
>> + mov x29, #0
>> mov x30, x0
>> mov sp, x3
>> mov x0, x1
>> --
>> 2.7.4
>>
^ permalink raw reply [flat|nested] 4+ messages in thread
* Re: [PATCH] MdePkg/BaseLib AARCH64: terminate stack frame list on stack switch
2016-09-09 12:03 ` Ard Biesheuvel
@ 2016-09-09 12:10 ` Leif Lindholm
0 siblings, 0 replies; 4+ messages in thread
From: Leif Lindholm @ 2016-09-09 12:10 UTC (permalink / raw)
To: Ard Biesheuvel; +Cc: edk2-devel-01, Gao, Liming
On Fri, Sep 09, 2016 at 01:03:59PM +0100, Ard Biesheuvel wrote:
> On 9 September 2016 at 12:18, Leif Lindholm <leif.lindholm@linaro.org> wrote:
> > On Fri, Sep 09, 2016 at 08:21:26AM +0100, Ard Biesheuvel wrote:
> >> When switching to the DXE phase stack, set the frame pointer to zero so
> >> that code walking the stack frame will not try to access stack frames\
> >
> > Trailing '\'.
> >
> >> belonging to the old stack.
> >
> > Do you mean that code will check for zero and stop processing, or that
> > it will be accessing rubbish instead of parsing a valid-looking frame?
>
> I don't understand this question. If it is zero, it will stop
> processing. If it is not zero, it will proceed, and potentially
> traverse stack frames in memory that is now owned by someone else.
Which was exactly what I was asking :)
So no need to change (but drop the '\').
/
Leif
^ permalink raw reply [flat|nested] 4+ messages in thread
end of thread, other threads:[~2016-09-09 12:10 UTC | newest]
Thread overview: 4+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
2016-09-09 7:21 [PATCH] MdePkg/BaseLib AARCH64: terminate stack frame list on stack switch Ard Biesheuvel
2016-09-09 11:18 ` Leif Lindholm
2016-09-09 12:03 ` Ard Biesheuvel
2016-09-09 12:10 ` Leif Lindholm
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox