Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Vector table definition in startup_*.s templates lacks alignment, may result in invalid offset in SCB->VTOR #22

Open
apullin opened this issue Dec 1, 2020 · 5 comments
Assignees
Labels
cmsis CMSIS-related issue or pull-request. enhancement New feature or request internal bug tracker Issue confirmed and reported into a ticket in the internal bug tracking system

Comments

@apullin
Copy link

apullin commented Dec 1, 2020

Describe the set-up

This issue should apply to code built targeting any Cortex-M0/3/4 core when using the provided startup_stm32*.s templates to define the vector table and the use of the gcc toolchain.

This appears to be common to both ARMv6-M and ARMv7-M cores, as least per the ARM documentation.

v6-M : https://developer.arm.com/documentation/ddi0419/c/System-Level-Architecture/System-Address-Map/System-Control-Space--SCS-/Vector-Table-Offset-Register--VTOR?lang=en
V7-M: https://developer.arm.com/documentation/ddi0403/d/System-Level-Architecture/System-Address-Map/System-Control-Space--SCS-/Vector-Table-Offset-Register--VTOR?lang=en

In my case, I am encountering this issue on an STM32L496AG part on a custom-designed board.

  • IDE or at least the compiler and its version:

arm-none-eabi-gcc (GNU Tools for Arm Embedded Processors 7-2017-q4-major) 7.2.1 20170904 (release) [ARM/embedded-7-branch revision 255204]

Describe the bug

There appears to be no alignment constraint on .isr_vector / g_pfnVectors, thus the table could end up being located to any address, e.g. 0x08010020 .

This could result in a vector table for which the offset cannot be loaded into SCB->VTOR.
As there is no checking for verification in SystemInit, the image-correct value of 0x08010020` might be loaded into SCB->VTOR, but due to the architecture constraints, the value of VTOR after write will be 0x08010000, thus pointing to an invalid view of the vector table.

This may also be an oversight on my part, but I believe I have run into it as an edge case.

How To Reproduce

Specific to my application (and this is where the mistake may be on my end):
I am building bootloadable images to be loaded into flash by a bootloader residing in the first 64K of flash.
I am not using a "bank switching" technique; rather, the bootloader will erase main flash and write the contents of a new binary to a fixed address.

To build an image that will always load VTOR for the vector table provided in the image, In system_stem32L4xx.c I add:
extern void* g_pfnVectors;
then, in SystemInit, I change:
SCB->VTOR = FLASH_BASE | VECT_TAB_OFFSET;
to
SCB->VTOR = (__IOM uint32_t)(&g_pfnVectors);

I am using a linker script that provides a MEMORY region with this configuration:

MEMORY
{
    RAM (xrw)      : ORIGIN = 0x20000000, LENGTH = 320K
    FLASH (rx)      : ORIGIN = 0x8010020, LENGTH = 942080
    DATA (rwx)      : ORIGIN = 0x080f6000, LENGTH = 40K
    QSPI (rx)      :   ORIGIN = 0x90000000, LENGTH = 8192K
}
  1. Build any example project using GCC, changing the FLASH start address in the linker script to e.g. 0x08010020

  2. Using a debugger, flash, and run with entry set to 0x08010020 to simulate entry via bootloader

  3. Break at SystemInit, run to the end of the function where SCB->VTOR is loaded

  4. Verify that g_pfnVectors is located to 0x08010020

  5. Observe that SCB->VTOR == 0x08010000 after write

  6. Continuing the program will likely result in a hardfault

Additional context

I believe the only remediation needed is to add an alignment constraint to the startup_stm*.s template file.

e.g., change

        .section	.isr_vector,"a",%progbits
	.type	g_pfnVectors, %object
	.size	g_pfnVectors, .-g_pfnVectors

g_pfnVectors:
	.word	_estack
        /* rest of the vector table ... */

to:

        .section	.isr_vector,"a",%progbits
	.type	g_pfnVectors, %object
	.size	g_pfnVectors, .-g_pfnVectors
	.align  7     /* require alignment to a value loadable into SCB->VTOR */

g_pfnVectors:
	.word	_estack
        /* rest of the vector table ... */

Although this is a change that may need to be made in every instance of the startup_stm*.s file, across all STM32Cube repos.

Beyond this fix, a more extensive solution may be to transition away from the asm startup file, and reimplement the template in C, where gcc alignment attributes could be applied.

FWIW, this issue arose in my usage of the bootloader provided in the aws/amazon-freertos repository. Their bootloader scheme relies on prepending any image with a 32B image "Image Descriptor", thus the 0x20 offset in my linker script.

@RKOUSTM
Copy link
Contributor

RKOUSTM commented Dec 15, 2020

Hi @apullin,

Thank you for this other report. You are right about this point. The issue has already been fixed internally. The fix below will be made available in the frame of a future release.

/** @addtogroup STM32L4xx_System_Private_Defines
  * @{
  */

+/* Note: Following vector table addresses must be defined in line with linker
+         configuration. */
+/*!< Uncomment the following line if you need to relocate the vector table
+    anywhere in Flash or Sram, else the vector table is kept at the automatic
+     remap of boot address selected */
+/* #define USER_VECT_TAB_ADDRESS */

+#if defined(USER_VECT_TAB_ADDRESS)
+/*!< Uncomment the following line if you need to relocate your vector Table
+     in Sram else user remap will be done in Flash. */
+/* #define VECT_TAB_SRAM */

+#if defined(VECT_TAB_SRAM)
+#define VECT_TAB_BASE_ADDRESS   SRAM1_BASE      /*!< Vector Table base address field.
+                                                     This value must be a multiple of 0x200. */
+#define VECT_TAB_OFFSET         0x00000000U     /*!< Vector Table base offset field.
+                                                     This value must be a multiple of 0x200. */
+#else
+#define VECT_TAB_BASE_ADDRESS   FLASH_BASE      /*!< Vector Table base address field.
                                                     This value must be a multiple of 0x200. */
+#define VECT_TAB_OFFSET         0x00000000U     /*!< Vector Table base offset field.
                                                     This value must be a multiple of 0x200. */
+#endif /* VECT_TAB_SRAM */
+#endif /* USER_VECT_TAB_ADDRESS */

In SystemInit() function, the following fix is proposed :

-  /* Configure the Vector Table location add offset address ------------------*/
-#ifdef VECT_TAB_SRAM
-  SCB->VTOR = SRAM_BASE | VECT_TAB_OFFSET; /* Vector Table Relocation in Internal SRAM */
-#else
-  SCB->VTOR = FLASH_BASE | VECT_TAB_OFFSET; /* Vector Table Relocation in Internal FLASH */
-#endif
+#if defined(USER_VECT_TAB_ADDRESS)
+  /* Configure the Vector Table location -------------------------------------*/
+  SCB->VTOR = VECT_TAB_BASE_ADDRESS | VECT_TAB_OFFSET;
+#endif

Thank you again for your contribution.

With regards,

@RKOUSTM RKOUSTM self-assigned this Dec 15, 2020
@RKOUSTM RKOUSTM added bug Something isn't working spotted before customer Spotted and fixed internally before being pointed out by users but not published yet labels Dec 15, 2020
@RKOUSTM RKOUSTM moved this from To do to In progress in stm32cube-mcu-fw-dashboard Dec 15, 2020
@RKOUSTM RKOUSTM added the cmsis CMSIS-related issue or pull-request. label Dec 15, 2020
@apullin
Copy link
Author

apullin commented Dec 20, 2020

My feedback here is (unless I am missing something):
This would still allow someone to build an un-runnable binary.

Adding a _Static_assert might be a worthwhile addition, e.g.

#ifdef __GNUC__
_Static_assert( ((VECT_TAB_BASE_ADDRESS | VECT_TAB_OFFSET) % 0x200) == 0, "VTOR must be aligned to 0x200" )
#endif /* __GNUC__ */

@RKOUSTM RKOUSTM moved this from In progress to To release in stm32cube-mcu-fw-dashboard Mar 3, 2021
@RKOUSTM
Copy link
Contributor

RKOUSTM commented Mar 4, 2021

Hi @apullin,

I hope you are fine. The issue you reported has been fixed in the frame of version v1.17.0 of the STM32CubeL4 published recently on GitHub.
Thank you again for having reported.
With regards,

@RKOUSTM RKOUSTM closed this as completed Mar 4, 2021
stm32cube-mcu-fw-dashboard automation moved this from To release to Done Mar 4, 2021
@RKOUSTM RKOUSTM added this to the v1.17.0 milestone Mar 4, 2021
@ALABSTM
Copy link
Contributor

ALABSTM commented Nov 9, 2021

Hi @apullin,

Back to you about this point you reported. Indeed, it is not exactly related to the fix mentioned above. This issue is, hence, to be reopened and your report forwarded again to our development teams.

We will back to you as soon as we have their feedback. Thank you for your patience and thank you again for having reported.

With regards,

@ALABSTM ALABSTM reopened this Nov 9, 2021
stm32cube-mcu-fw-dashboard automation moved this from Done to To do Nov 9, 2021
@ALABSTM ALABSTM removed this from the v1.17.0 milestone Nov 9, 2021
@ALABSTM ALABSTM added enhancement New feature or request internal bug tracker Issue confirmed and reported into a ticket in the internal bug tracking system and removed bug Something isn't working spotted before customer Spotted and fixed internally before being pointed out by users but not published yet labels Nov 9, 2021
@ALABSTM ALABSTM self-assigned this Nov 9, 2021
@ALABSTM
Copy link
Contributor

ALABSTM commented Nov 9, 2021

ST Internal Reference: 117156

@ALABSTM ALABSTM moved this from To do to In progress in stm32cube-mcu-fw-dashboard Nov 9, 2021
@STMicroelectronics STMicroelectronics deleted a comment from RKOUSTM Aug 7, 2023
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
cmsis CMSIS-related issue or pull-request. enhancement New feature or request internal bug tracker Issue confirmed and reported into a ticket in the internal bug tracking system
Projects
Development

No branches or pull requests

3 participants