Skip to content

Commit

Permalink
Add stack-heap collision check to BrkSyscall execution (#346)
Browse files Browse the repository at this point in the history
Implemented additional checking in the `execute()` method of the brk syscall, defined in the `BrkSyscall` class, to ensure that memory allocation does not overflow stack space.
The System Call has as its argument the new "program break", which is now compared with the current "stack pointer" before being set to avoid overlaps.

Added unit tests to verify that when the `brk` syscall is called with illegal arguments (causing a stack-heap collision), the syscall returns a non-zero value, indicating that it was not executed successfully.
  • Loading branch information
federicovilla55 authored Sep 3, 2024
1 parent f6627af commit d0c71e8
Show file tree
Hide file tree
Showing 3 changed files with 75 additions and 2 deletions.
15 changes: 13 additions & 2 deletions src/syscall/control.h
Original file line number Diff line number Diff line change
Expand Up @@ -56,8 +56,19 @@ class BrkSyscall : public BaseSyscall {

void execute() {
// Nothing to do - at the moment we allow infinite growth of the heap.
// @todo: Add checks to ensure that the heap doesn't grow into the stack
// segment...!

// Retrieves the argument of the brk syscall, the new program break
uint64_t newBreak = BaseSyscall::getArg(BaseSyscall::REG_FILE, 0);
// Retrieve the current stack pointer from the processor handler
uint64_t stackPointer =
ProcessorHandler::getProcessor()->getRegister(BaseSyscall::REG_FILE, 2);
if (newBreak >= stackPointer) {
SystemIO::printString(
"Error: Attempted to allocate memory overlapping stack segment\n");
BaseSyscall::setRet(BaseSyscall::REG_FILE, 0, -1); // Syscall error code
return;
}

BaseSyscall::setRet(BaseSyscall::REG_FILE, 0, 0);
}
};
Expand Down
35 changes: 35 additions & 0 deletions test/riscv-tests-64/brk_syscall.S
Original file line number Diff line number Diff line change
@@ -0,0 +1,35 @@
.text
.globl _start
_start: nop

#-------------------------------------------------------------
# Stack-Heap Collision brk syscall Test
#-------------------------------------------------------------

test_0: # Test Ecall Return Value
mv a0, sp # Assign the ecall parameter to the stack pointer value
lw a7, BRK # Load the brk syscall immediate value
ecall

# If the return value of the ecall is zero, then the ecall
# was executed successfully and therefore the test failed
beqz a0, fail

bne x0, gp, pass

pass:
li a0, 42
li a7, 93
ecall


fail:
li a0, 0
li a7, 93
ecall

.data
.align 4

# Syscall immediate value
BRK: .word 214
27 changes: 27 additions & 0 deletions test/riscv-tests/brk_syscall.s
Original file line number Diff line number Diff line change
@@ -0,0 +1,27 @@
.text

#-------------------------------------------------------------
# Stack-Heap Collision brk syscall Test
#-------------------------------------------------------------

test_0: # Test Ecall Return Value
mv a0, sp # Assign the ecall parameter to the stack pointer value
li a7, 214 # Load the brk syscall immediate value
ecall

# If the return value of the ecall is zero, then the ecall
# was executed successfully and therefore the test failed
beqz a0, fail

bne x0, gp, pass

pass:
li a0, 42
li a7, 93
ecall


fail:
li a0, 0
li a7, 93
ecall

0 comments on commit d0c71e8

Please sign in to comment.