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

arch: riscv: userspace: potential security risk when CONFIG_RISCV_GP=y #81372

Closed
ycsin opened this issue Nov 14, 2024 · 1 comment · Fixed by #81155
Closed

arch: riscv: userspace: potential security risk when CONFIG_RISCV_GP=y #81372

ycsin opened this issue Nov 14, 2024 · 1 comment · Fixed by #81155
Assignees
Labels
area: RISCV RISCV Architecture (32-bit & 64-bit) area: Security Security bug The issue is a bug, or the PR is fixing a bug

Comments

@ycsin
Copy link
Member

ycsin commented Nov 14, 2024

Describe the bug

When the Global Pointer (GP) relative addressing is enabled (CONFIG_RISCV_GP=y), the gp reg points at 0x800 bytes past the start of the .sdata section which is then used by the linker to relax accesses to global symbols.

#ifdef CONFIG_RISCV_GP
/*
* RISC-V architecture has 12-bit signed immediate offsets in the
* instructions. If we can put the most commonly accessed globals
* in a special 4K span of memory addressed by the GP register, then
* we can access those values in a single instruction, saving both
* codespace and runtime.
*
* Since these immediate offsets are signed, place gp 0x800 past the
* beginning of .sdata so that we can use both positive and negative
* offsets.
*/
. = ALIGN(8);
PROVIDE (__global_pointer$ = . + 0x800);
#endif

However, the gp reg is not protected against write from userspace, this means that a rogue userspace can corrupt the gp reg, and cause the compiled instruction to access random addresses.

To Reproduce
Steps to reproduce the behavior:

  1. Build the qemu_riscv64 board with userspace and CONFIG_RISCV_GP enabled
  2. Note the value of the gp register
  3. Write some random value to the gp reg from one userspace thread
  4. Notice that the gp reg is now changed

Expected behavior

The gp register should remain a constant.

Impact

A rogue thread can corrupt the gp reg and cause the entire system to hard fault at best, at worst, it can potentially trick the system to access another set of random global symbols.

Environment (please complete the following information):

  • Toolchain (e.g Zephyr SDK, ...): 0.16.8
  • Commit SHA or Version used: v3.7 branch
@ycsin ycsin added bug The issue is a bug, or the PR is fixing a bug area: Security Security labels Nov 14, 2024
@ycsin ycsin linked a pull request Nov 14, 2024 that will close this issue
@henrikbrixandersen henrikbrixandersen added the area: RISCV RISCV Architecture (32-bit & 64-bit) label Nov 15, 2024
@fkokosinski
Copy link
Member

I believe this issue was fixed in #81155, so I'm closing it.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
area: RISCV RISCV Architecture (32-bit & 64-bit) area: Security Security bug The issue is a bug, or the PR is fixing a bug
Projects
None yet
Development

Successfully merging a pull request may close this issue.

5 participants