[GitHub] [incubator-nuttx] xiaoxiang781216 commented on pull request #1341: Call c++ global variables in nxtask_startup

[GitBox <gi...@apache.org>]

xiaoxiang781216 commented on pull request #1341:
URL: https://github.com/apache/incubator-nuttx/pull/1341#issuecomment-652168334


   > It is an incorrect design to call C++ constructors or destructors from the OS in kernel mode (PROTECTED and KERNEL builds) or with interrupts disabled. Calling the constructors.destructors introduces new problems similar to those discussed in Issue #1263 and which I am working toward fixing in PR #1328
   >
   
   nxtask_startup is the part of libc in PROTECTED and run in the user mode:
   ```
   nxtask_start->up_task_start->switch to user mode->nxtask_startup->c++ constructors
   ```
   KERNEL mode doesn't go this path.
   Note: the constructors here is the global c++ objects which is part of nuttx.bin and nuttx_user.bin(NOT in elf binary).
   You can see there are many places call gnu_cxxinitialize:
   https://github.com/apache/incubator-nuttx-apps/pull/316
   This patch try to centralize the builtin constructors in one place, but don't break the rule: call constructor should happen inside the user mode without the interrupt disable.
   
   > There is also a bug already listed in the to-level TODO list (This is also Issue #1265 ):
   > 
   > ```
   >   Title:       C++ CONSTRUCTORS HAVE TOO MANY PRIVILEGES (PROTECTED MODE)
   >   Description: When a C++ ELF module is loaded, its C++ constructors are called
   >                via sched/task_starthook.c logic.  This logic runs in protected mode.
   >                The is a security hole because the user code runs with kernel-
   >               privileges when the constructor executes.
   > 
   >                Destructors likely have the opposite problem.  The probably try to
   >                execute some kernel logic in user mode?  Obviously this needs to
   >                be investigated further.
   >   Status:      Open
   >   Priority:    Low (unless you need build a secure C++ system).
   > ```
   
   This patch move the construction inside nuttx.bin(FLAT build)/nuttx_user.bin(PROTECTED) to nxtask_startup. it's different from ELF binary problem.
   
   > Think of how this is done in Linux: Each task has a wrapper function called __start in crt0.S and will handle both constructors and destructors. And the first layer or exit logic is part of the C library.
   
   To follow the Linux approach, the major change is the build system:
   1.Statically link crt0.o into each ELF binary
   2.__start call constructors, and main, destructors and exit in order
   3.Remove all stuff related to elf_loadctors/elf_loaddtors
   But this approach can't be used for apps which part of nuttx.bin/nuttx_user.bin, nxtask_start can avoid in this case.
   
   > Look how I did the pthread startup function in PR #1328. This is EXACTLY the same issue. I created a user-space pthread_startup (NOT and OS startup function) in libs/libc/pthread/pthread_create.c. This is exactly the kind of user-space startup function that you need: It runs in user mode with interrupts enabled.
   
   The same approach is workable for ELF binary. But how do we handle the builtin apps, do you have the idea to avoid nxtask_start in this case?


----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.

For queries about this service, please contact Infrastructure at:
users@infra.apache.org