libc/machine/xtensa: make longjmp safe against context switch

In order to turn longjmp context-switch safe, it's necessary
to disable interrupts before modifying windowbase and windowstart.
Otherwise, after a context switch, windowstart and windowbase
would be different, leading to a wrongly set windowstart bit due to
longjmp writing it based on the windowbase before the context switch.
This corrupts the registers at the next window overflow reaching
that wrongly set bit.

*Background:*
This PR is related to an issue first observed on ESP-IDF
https://github.com/espressif/esp-idf/issues/5229 and it was, then,
checked on NuttX using a test application.

*The test application:*
To check if the problem affects ESP32, ESP32-S2 and ESP32-S3 on
NuttX, it was created an application based on:
https://en.cppreference.com/w/c/program/longjmp

The application creates 16 tasks (`#define NUMBER_OF_TASKS  16`)
that implements the following daemon:

```
static int setjmp_longjmp_daemon(int argc, char *argv[])
{
  for (int i = 0; i < NUMBER_OF_TASKS * 2; i++)
    {
      jmp_buf env;

      volatile int count = 0;
      if (setjmp(env) != UINT16_MAX)
        {
          foo(&env, ++count);
        }
    }

  sem_post(&g_sem);

  return EXIT_SUCCESS;
}
```

The main function also initializes a semaphore to avoid application
exiting before tasks return successfully:

```
  sem_init(&g_sem, 0, -NUMBER_OF_TASKS);
```

Finally, the round-robin interval was lowered to 1ms to raise the
chances of the longjmp being interrupted by a context switch
(`CONFIG_RR_INTERVAL=1).

This setup was able to reproduce the problem prior to this patch
being applied.
This commit is contained in:
Tiago Medicci Serrano 2022-11-21 15:04:32 -03:00 committed by Petro Karashchenko
parent f23ec0f995
commit d3ffeb40a7
10 changed files with 66 additions and 0 deletions

View File

@ -554,6 +554,7 @@
* 0 == XEAX (extern) or TX */
#define XCHAL_HAVE_XEA1 0 /* Exception Architecture 1 */
#define XCHAL_HAVE_XEA2 1 /* Exception Architecture 2 */
#define XCHAL_HAVE_XEA3 0 /* Exception Architecture 3 */
#define XCHAL_HAVE_XEAX 0 /* External Exception Arch. */
#define XCHAL_HAVE_EXCEPTIONS 1 /* exception option */
#define XCHAL_HAVE_HALT 0 /* halt architecture option */

View File

@ -610,6 +610,7 @@
#define XCHAL_HAVE_XEA1 0 /* Exception Architecture 1 */
#define XCHAL_HAVE_XEA2 1 /* Exception Architecture 2 */
#define XCHAL_HAVE_XEA3 0 /* Exception Architecture 3 */
#define XCHAL_HAVE_XEAX 0 /* External Exception Arch. */
#define XCHAL_HAVE_EXCEPTIONS 1 /* exception option */
#define XCHAL_HAVE_HALT 0 /* halt architecture option */

View File

@ -568,6 +568,7 @@
#define XCHAL_HAVE_XEA1 0 /* Exception Architecture 1 */
#define XCHAL_HAVE_XEA2 1 /* Exception Architecture 2 */
#define XCHAL_HAVE_XEA3 0 /* Exception Architecture 3 */
#define XCHAL_HAVE_XEAX 0 /* External Exception Arch. */
#define XCHAL_HAVE_EXCEPTIONS 1 /* exception option */
#define XCHAL_HAVE_HALT 0 /* halt architecture option */

View File

@ -17,6 +17,7 @@ CONFIG_ARCH_CHIP="esp32"
CONFIG_ARCH_CHIP_ESP32=y
CONFIG_ARCH_CHIP_ESP32WROVER=y
CONFIG_ARCH_INTERRUPTSTACK=2048
CONFIG_ARCH_SETJMP_H=y
CONFIG_ARCH_STACKDUMP=y
CONFIG_ARCH_XTENSA=y
CONFIG_BOARD_LOOPSPERMSEC=16717

View File

@ -16,6 +16,7 @@ CONFIG_ARCH_BOARD_ESP32_DEVKITC=y
CONFIG_ARCH_CHIP="esp32"
CONFIG_ARCH_CHIP_ESP32=y
CONFIG_ARCH_CHIP_ESP32WROVER=y
CONFIG_ARCH_SETJMP_H=y
CONFIG_ARCH_STACKDUMP=y
CONFIG_ARCH_XTENSA=y
CONFIG_BOARD_LOOPSPERMSEC=16717

View File

@ -17,6 +17,7 @@ CONFIG_ARCH_CHIP="esp32"
CONFIG_ARCH_CHIP_ESP32=y
CONFIG_ARCH_CHIP_ESP32WROVER=y
CONFIG_ARCH_INTERRUPTSTACK=2048
CONFIG_ARCH_SETJMP_H=y
CONFIG_ARCH_STACKDUMP=y
CONFIG_ARCH_XTENSA=y
CONFIG_BOARD_LOOPSPERMSEC=16717

View File

@ -18,6 +18,7 @@ CONFIG_ARCH_CHIP="esp32"
CONFIG_ARCH_CHIP_ESP32=y
CONFIG_ARCH_CHIP_ESP32WROVER=y
CONFIG_ARCH_INTERRUPTSTACK=2048
CONFIG_ARCH_SETJMP_H=y
CONFIG_ARCH_STACKDUMP=y
CONFIG_ARCH_XTENSA=y
CONFIG_BOARD_LOOPSPERMSEC=16717

View File

@ -0,0 +1,46 @@
#
# This file is autogenerated: PLEASE DO NOT EDIT IT.
#
# You can use "make menuconfig" to make any modifications to the installed .config file.
# You can then do "make savedefconfig" to generate a new defconfig file that includes your
# modifications.
#
# CONFIG_ARCH_LEDS is not set
# CONFIG_NSH_ARGCAT is not set
# CONFIG_NSH_CMDOPT_HEXDUMP is not set
# CONFIG_NSH_CMDPARMS is not set
CONFIG_ARCH="xtensa"
CONFIG_ARCH_BOARD="esp32s2-saola-1"
CONFIG_ARCH_BOARD_COMMON=y
CONFIG_ARCH_BOARD_ESP32S2_SAOLA_1=y
CONFIG_ARCH_CHIP="esp32s2"
CONFIG_ARCH_CHIP_ESP32S2=y
CONFIG_ARCH_CHIP_ESP32S2WROVER=y
CONFIG_ARCH_SETJMP_H=y
CONFIG_ARCH_STACKDUMP=y
CONFIG_ARCH_XTENSA=y
CONFIG_BOARD_LOOPSPERMSEC=16717
CONFIG_BUILTIN=y
CONFIG_ESP32S2_UART0=y
CONFIG_FS_PROCFS=y
CONFIG_HAVE_CXX=y
CONFIG_HAVE_CXXINITIALIZE=y
CONFIG_IDLETHREAD_STACKSIZE=3072
CONFIG_INIT_ENTRYPOINT="nsh_main"
CONFIG_INTELHEX_BINARY=y
CONFIG_NSH_ARCHINIT=y
CONFIG_NSH_BUILTIN_APPS=y
CONFIG_NSH_FILEIOSIZE=512
CONFIG_NSH_LINELEN=64
CONFIG_NSH_READLINE=y
CONFIG_PREALLOC_TIMERS=4
CONFIG_RAM_SIZE=114688
CONFIG_RAM_START=0x20000000
CONFIG_RR_INTERVAL=200
CONFIG_SCHED_WAITPID=y
CONFIG_START_DAY=6
CONFIG_START_MONTH=12
CONFIG_START_YEAR=2011
CONFIG_SYSTEM_NSH=y
CONFIG_TESTING_OSTEST=y
CONFIG_UART0_SERIAL_CONSOLE=y

View File

@ -17,6 +17,7 @@ CONFIG_ARCH_CHIP="esp32s3"
CONFIG_ARCH_CHIP_ESP32S3=y
CONFIG_ARCH_CHIP_ESP32S3WROOM1=y
CONFIG_ARCH_INTERRUPTSTACK=2048
CONFIG_ARCH_SETJMP_H=y
CONFIG_ARCH_STACKDUMP=y
CONFIG_ARCH_XTENSA=y
CONFIG_BOARD_LOOPSPERMSEC=16717

View File

@ -258,6 +258,14 @@ longjmp:
/* Using this register is more efficient; it triggers less overflows. */
# define AR_WB a5
# endif
/* Deactivate interrupts in order to modify WindowBase
and WindowStart. */
rsr a7, PS /* to be restored after SPILL_ALL_WINDOWS */
movi a5, XCHAL_PS_EXCM_MASK /* PS_INTLEVEL_MASK */
or a5, a7, a5 /* get the current INTLEVEL */
wsr a5, PS
rsync
/* Invalidate all but the current window;
set WindowStart to (1 << WindowBase). */
rsr AR_WB, WINDOWBASE
@ -267,6 +275,10 @@ longjmp:
wsr a4, WINDOWSTART
rsync
/* Activate interrupts again after modifying WindowBase and WindowStart. */
wsr a7, PS
rsync
/* Return to the return address of the setjmp, using the
window size bits from the setjmp call so that the caller
will be able to find the return value that we put in a2. */