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

Stack overflow protection #1764

Merged
merged 8 commits into from
Oct 16, 2021
Merged

Stack overflow protection #1764

merged 8 commits into from
Oct 16, 2021

Conversation

occheung
Copy link
Contributor

@occheung occheung commented Oct 8, 2021

ARTIQ Pull Request

Description of Changes

This patch implements stack overflow protection using guard page, with the aid of physical memory protection (PMP) units from RISC-V.

Linker scripts (Except bootloader)

A symbol _sstack_guard is added to each of the linker script (except bootloader). It specifies the start of the stack guard.

Both stacks and guard pages are 4k aligned. Guard pages of these stacks are always active. PMP region 2 implements these guard pages.

Thread stacks

When spawning a new thread, Io now inflates it stack size by 4k to fit the guard page of the thread. Stacks requested by libfringe are 4k aligned, so the guard page will be 4k aligned as well.

The guard page of a thread stack is active only if the context is switched to the thread. Both PMP region 0 & 1 implement these guard pages.

Startup Flow

Satman/Kernel

A PMP region will be enabled to initialize their stack guard during their startup.

Runtime

Same as satman/kernel. In addition, runtime will then transfer control from machine privilege to user privilege. It will remain in user privilege unless PMP regions need to be modified.

Context switch

An environment call will be invoked during context switching (see arch::swap() implemented in libfringe). The exception handler then adjusts the PMP regions in machine mode. Finally, it resumes context switching with user privilege.

Testing

Main stack guard

Adding the following code after initializing the main stack guard will trigger a CPU load fault.

        extern {
            static mut _sstack_guard: u8;
        }

        unsafe {
            println!("stack border: {:?}", *((&_sstack_guard as *const u8).offset(4095)));
        }

Important serial output:

panic at runtime/main.rs:338:13: exception LoadFault at PC 0x4000301c, trap value 0x4015cfff

Thread stack guard

Spawning a task that initializes a large array will cause an access fault. For example,

    io.spawn(4096, |_io| {
        let large_array: [u32; 1024] = [0; 1024];
        println!("Array[0]: {}", large_array[0]);
    });

It resulted in the following serial output:

Trap frame: TrapFrame { ra: 40008214, t0: deaddead, t1: deaddead, t2: 7, t3: 7, t4: 40052ba4, t5: 4004dc00, t6: 0, a0: 4016cce0, a1: 4016cb60, a2: 4016d084, a3: 40181ff4, a4: 2000, a5: 4017fff4, a6: 201c, a7: 40180000 }
@ 0x400211f8
+0000: 00c12423 02010693 00452403 00000513
+0010: 00058613 00000893 808e70ef 00b12623
+0020: 00c10513 00a12823 00812503 00a12a23
+0030: 00812c23 00001637 02010413 00040513
panic at runtime/main.rs:328:13: exception StoreFault at PC 0x400211f8, trap value 0x40180f78
backtrace for software version 7.unknown.beta;nist_clock:
0x4002e63c
0x40004e00
0x4002dba0
halting.
use `artiq_coremgmt config write -s panic_reset 1` to restart instead

The PC value corresponds to the following assembly dump.

400211b0 <_ZN6fringe9generator39Generator$LT$Input$C$Output$C$Stack$GT$10unsafe_new17generator_wrapper17h7482ebcb48eab63eE>:
    unsafe extern "C" fn generator_wrapper<Input, Output, Stack, F>(env: usize, stack_ptr: StackPointer) -> !
400211b0:	81010113          	addi	sp,sp,-2032
400211b4:	7e112623          	sw	ra,2028(sp)
400211b8:	7e812423          	sw	s0,2024(sp)
400211bc:	7e912223          	sw	s1,2020(sp)
400211c0:	7f212023          	sw	s2,2016(sp)
400211c4:	7d312e23          	sw	s3,2012(sp)
400211c8:	7d412c23          	sw	s4,2008(sp)
400211cc:	7d512a23          	sw	s5,2004(sp)
400211d0:	7d612823          	sw	s6,2000(sp)
400211d4:	7d712623          	sw	s7,1996(sp)
400211d8:	7d812423          	sw	s8,1992(sp)
400211dc:	7d912223          	sw	s9,1988(sp)
400211e0:	7da12023          	sw	s10,1984(sp)
400211e4:	7bb12e23          	sw	s11,1980(sp)
400211e8:	00001637          	lui	a2,0x1
400211ec:	89060613          	addi	a2,a2,-1904 # 890 <.Lline_table_start0+0x658>
400211f0:	40c10133          	sub	sp,sp,a2
400211f4:	00052603          	lw	a2,0(a0)
400211f8:	00c12423          	sw	a2,8(sp)
400211fc:	02010693          	addi	a3,sp,32
40021200:	00452403          	lw	s0,4(a0)
40021204:	00000513          	li	a0,0
40021208:	00058613          	mv	a2,a1
4002120c:	00000893          	li	a7,0
40021210:	808e70ef          	jal	ra,40008218 <_ZN6fringe4arch3imp4swap10trampoline17h07dbcd4ec0d537a5E>
      let yielder = Yielder::new(stack_ptr);
40021214:	00b12623          	sw	a1,12(sp)
40021218:	00c10513          	addi	a0,sp,12

So the fault was actually triggered during the initialization of thread in libfringe.

Kernel stack guard

Added the following code after initializing the stack guard in kernel.

    println!("Access guard memory addr: {:p}", (_sstack_guard as usize as *const usize));
    core::ptr::read_volatile((_sstack_guard as *const usize).offset(1023));

After a library is loaded to the kernel, it panics with error messages like such.

[    38.838184s]  INFO(kernel): Access guard memory addr: 0x45061000
[    38.843520s]  INFO(kernel): panic at ksupport/lib.rs:539:5: Exception(LoadFault) at PC 0x450073dc, trap value 0x45061ffc

Related Issue

libfringe PR
Closes #544

Type of Changes

Type
🐛 Bug fix
✨ New feature
🔨 Refactoring
📜 Docs

Steps (Choose relevant, delete irrelevant before submitting)

All Pull Requests

  • Use correct spelling and grammar.
  • Update RELEASE_NOTES.rst if there are noteworthy changes, especially if there are changes to existing APIs.
  • Close/update issues.
  • Check the copyright situation of your changes and sign off your patches (git commit --signoff, see copyright).

Code Changes

Git Logistics

  • Split your contribution into logically separate changes (git rebase --interactive). Merge/squash/fixup commits that just fix or amend previous commits. Remove unintended changes & cleanup. See tutorial.
  • Write short & meaningful commit messages. Review each commit for messages (git show). Format:
    topic: description. < 50 characters total.
    
    Longer description. < 70 characters per line
    

Licensing

See copyright & licensing for more info.
ARTIQ files that do not contain a license header are copyrighted by M-Labs Limited and are licensed under LGPLv3+.

@sbourdeauducq
Copy link
Member

@lriesebos @b-bondurant

@occheung occheung changed the title [WIP] Stack overflow protection Stack overflow protection Oct 8, 2021
@occheung occheung marked this pull request as ready for review October 8, 2021 10:00
@lriesebos
Copy link
Contributor

That's great to see! Just wondering, what are the memory limits for the comm cpu (i.e. max size of uploaded binary) and a running kernel (i.e. allocation of a large array on the stack in a kernel)?

@occheung
Copy link
Contributor Author

what are the memory limits for the comm cpu (i.e. max size of uploaded binary)

In runtime.ld, 64MB is allocated to store the comm CPU binary (with NOLOAD sections, e.g. stack and heap) and the kernel support binary (ksupport_data.o, the part that performs dynamic linking, RPC with comm, include math library etc). But ksupport_data.o should be quite small. I built one with nist_clock variant on KC705 and it was < 400KB.

However, there might be some other factors that might limit the binary size. For example, the size of the memory region mapped to the SPI flash/ROM (IIRC 16MB for Kasli), and the gateware and other binaries (e.g. bootloader) is also included.

The actual size of runtime.fbi is generally a lot smaller than any of these limits, so most of the 64MB goes to the heap. (see traces from, #1761)

and a running kernel (i.e. allocation of a large array on the stack in a kernel)?

The stack size of the kernel depends its binary size. First, kernel CPU region is around 175MB large (from 0x45060000 to 0x4FFFFFF0). Then, the page guard is added after every linker section, effectively after the kernel binary (the dynamic library part). The linker for kernel is written as such.

/* kernel.ld */

    /* Include every sections before this comment */

    . = ALIGN(0x1000);
    _sstack_guard = .;
} /* EOF */

The guard page is also 4096 bytes long, so the size of the kernel stack is whatever remains of it after the stack guard.

I think most of these numbers are the same since #1612. This patch only reduces the kernel stack size by no more than 8192 bytes (stack guard & stack alignment).

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Trivial memory protection unit (stack overflow protection)
3 participants