Re: Fwd: Undelivered Mail Returned to Sender

[Torbjorn SVENSSON <torbjorn.svensson@foss.st.com>]

[-- Attachment #1: Type: text/plain, Size: 389 bytes --]

Hi again,

On 2022-10-21 21:45, Frank Ch. Eigler wrote:
> Hi -
> 
>> FYI, it looks like there is an issue with the mail system at sourceware.org
>> (maybe this mail wont be delivered either...).
> 
> Thanks, we tweaked two related things, pls let us know if this reoccurs.

I just noticed another failure (attached), identical to the one yesterday.

Kind regards,
Torbjörn

> 
> - FChE
> 

[-- Attachment #2: Undelivered Mail Returned to Sender.eml --]
[-- Type: message/rfc822, Size: 14612 bytes --]

[-- Attachment #2.1.1: Notification --]
[-- Type: text/plain, Size: 521 bytes --]

This is the mail system at host sourceware.org.

I'm sorry to have to inform you that your message could not
be delivered to one or more recipients. It's attached below.

For further assistance, please send mail to postmaster.

If you do so, please include this problem report. You can
delete your own text from the attached returned message.

                   The mail system

<inbox@sourceware.org> (expanded from <gdb-patches@sourceware.org>): Command
    time limit exceeded: "/home/inbox/public-inbox-mda-true.sh"

[-- Attachment #2.1.2: Delivery report --]
[-- Type: message/delivery-status, Size: 371 bytes --]

[-- Attachment #2.1.3: Undelivered Message --]
[-- Type: message/rfc822, Size: 9150 bytes --]

From: Torbjorn SVENSSON <torbjorn.svensson@foss.st.com>
To: Tomas Vanek <vanekt@fbl.cz>, <gdb-patches@sourceware.org>
Subject: Re: [PATCH v2] gdb/arm: Fix M-profile EXC_RETURN exception_domain_is_secure logic
Date: Sat, 22 Oct 2022 10:11:28 +0200
Message-ID: <48df83b6-054f-6d0b-c1e3-076eef904eb3@foss.st.com>

Hi Tomas,

On 2022-10-22 10:07, Tomas Vanek wrote:
> Arm v8-M Architecture Reference Manual,
> D1.2.95 EXC_RETURN, Exception Return Payload
> describes ES bit:
> 
> "ES, bit [0]
>       Exception Secure. The security domain the exception was taken to.
>       The possible values of this bit are:
>         0 Non-secure.
>         1 Secure"
> 
> arm-tdep.c:3443, arm_m_exception_cache () function tests this bit:
> 
>    exception_domain_is_secure = (bit (lr, 0) == 0);
> 
> The test is negated!

Good catch! I'm not sure how I thought when I wrote this, but thanks for 
correcting it.

> 
> Later on line 3553, the condition evaluates if an additional state
> context is stacked:
> 
>    /* With the Security extension, the hardware saves R4..R11 too.  */
>    if (tdep->have_sec_ext && secure_stack_used
>        && (!default_callee_register_stacking || exception_domain_is_secure))
> 
> RM, B3.19 Exception entry, context stacking
> reads:
> RPLHM "In a PE with the Security Extension, on taking an exception,
> the PE hardware:
>    ...
>    2. If exception entry requires a transition from Secure state to
>       Non-secure state, the PE hardware extends the stack frame and also
>       saves additional state context."
> 
> So we should test for !exception_domain_is_secure instead of non-negated
> value!
> These two bugs compensate each other so unstacking works correctly.
> 
> But another test of exception_domain_is_secure (negated due to the
> first bug) prevents arm_unwind_secure_frames to work as expected:
> 
>    /* Unwinding from non-secure to secure can trip security
>       measures.  In order to avoid the debugger being
>       intrusive, rely on the user to configure the requested
>       mode.  */
>    if (secure_stack_used && !exception_domain_is_secure
>        && !arm_unwind_secure_frames)
> 
> Test with GNU gdb (GDB) 13.0.50.20221016-git.
> Stopped in a non-secure handler:
> 
>   (gdb) set arm unwind-secure-frames 0
>   (gdb) bt
>   #0  HAL_SYSTICK_Callback () at C:/dvl/stm32l5trustzone/GPIO_IOToggle_TrustZone/NonSecure/Src/nsmain.c:490
>   #1  0x0804081c in SysTick_Handler ()
>       at C:/dvl/stm32l5trustzone/GPIO_IOToggle_TrustZone/NonSecure/Src/nsstm32l5xx_it.c:134
>   #2  <signal handler called>
>   #3  HAL_GPIO_ReadPin (GPIOx=0x52020800, GPIO_Pin=8192)
>       at C:/dvl/stm32l5trustzone/GPIO_IOToggle_TrustZone/Drivers/STM32L5xx_HAL_Driver/Src/stm32l5xx_hal_gpio.c:386
>   #4  0x0c000338 in SECURE_Mode () at C:/dvl/stm32l5trustzone/GPIO_IOToggle_TrustZone/Secure/Src/main.c:86
>   #5  0x080403f2 in main () at C:/dvl/stm32l5trustzone/GPIO_IOToggle_TrustZone/NonSecure/Src/nsmain.c:278
>   Backtrace stopped: previous frame inner to this frame (corrupt stack?)
> 
> The frames #3 and #4 are secure. backtrace should stop before #3.
> 
> Stopped in a secure handler:
> 
>   (gdb) bt
>   #0  HAL_SYSTICK_Callback () at C:/dvl/stm32l5trustzone/GPIO_IOToggle_TrustZone/Secure/Src/main.c:425
>   #1  0x0c000b6a in SysTick_Handler ()
>       at C:/dvl/stm32l5trustzone/GPIO_IOToggle_TrustZone/Secure/Src/stm32l5xx_it.c:234
>   warning: Non-secure to secure stack unwinding disabled.
>   #2  <signal handler called>
> 
> The exception from secure to secure erroneously stops unwinding. It should
> continue as far as the security unlimited backtrace:
> 
>   (gdb) set arm unwind-secure-frames 1
>   (gdb) si <-- used to rebuild frame cache after change of unwind-secure-frames

Is there any way to make gdb rebuild the frame cache directly when doing 
the "set arm unwind-secure-frames"? Feels dirty to do a instruction step 
just to get the right trace...
Regardless of the answer to the above question, it's not something to 
address in this patch.

>   0x0c0008e6      425       if (SecureTimingDelay != 0U)
>   (gdb) bt
>   #0  0x0c0008e6 in HAL_SYSTICK_Callback () at C:/dvl/stm32l5trustzone/GPIO_IOToggle_TrustZone/Secure/Src/main.c:425
>   #1  0x0c000b6a in SysTick_Handler ()
>       at C:/dvl/stm32l5trustzone/GPIO_IOToggle_TrustZone/Secure/Src/stm32l5xx_it.c:234
>   #2  <signal handler called>
>   #3  0x0c000328 in SECURE_Mode () at C:/dvl/stm32l5trustzone/GPIO_IOToggle_TrustZone/Secure/Src/main.c:88
>   #4  0x080403f2 in main () at C:/dvl/stm32l5trustzone/GPIO_IOToggle_TrustZone/NonSecure/Src/nsmain.c:278
> 
>   Backtrace stopped: previous frame inner to this frame (corrupt stack?)
> 
> Set exception_domain_is_secure to the value expected by its name.
> Fix exception_domain_is_secure usage in the additional state context
> stacking condition.
> 
> v2: Corrected backtrace logs in commit message
> 
> Signed-off-by: Tomas Vanek <vanekt@fbl.cz>
> ---
>   gdb/arm-tdep.c | 4 ++--
>   1 file changed, 2 insertions(+), 2 deletions(-)
> 
> diff --git a/gdb/arm-tdep.c b/gdb/arm-tdep.c
> index 55295e1..20b6f3f 100644
> --- a/gdb/arm-tdep.c
> +++ b/gdb/arm-tdep.c
> @@ -3496,7 +3496,7 @@ struct frame_unwind arm_stub_unwind = {
>   	{
>   	  secure_stack_used = (bit (lr, 6) != 0);
>   	  default_callee_register_stacking = (bit (lr, 5) != 0);
> -	  exception_domain_is_secure = (bit (lr, 0) == 0);
> +	  exception_domain_is_secure = (bit (lr, 0) != 0);
>   
>   	  /* Unwinding from non-secure to secure can trip security
>   	     measures.  In order to avoid the debugger being
> @@ -3606,7 +3606,7 @@ struct frame_unwind arm_stub_unwind = {
>   
>         /* With the Security extension, the hardware saves R4..R11 too.  */
>         if (tdep->have_sec_ext && secure_stack_used
> -	  && (!default_callee_register_stacking || exception_domain_is_secure))
> +	  && (!default_callee_register_stacking || !exception_domain_is_secure))
>   	{
>   	  /* Read R4..R11 from the integer callee registers.  */
>   	  cache->saved_regs[4].set_addr (unwound_sp + 0x08);

Kind regards,
Torbjörn