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

llext: Fix off-by-one in RISC-V truncation check #83198

Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
7 changes: 7 additions & 0 deletions arch/riscv/core/elf.c
Original file line number Diff line number Diff line change
Expand Up @@ -43,6 +43,13 @@ LOG_MODULE_REGISTER(elf, CONFIG_LLEXT_LOG_LEVEL);
static inline int riscv_relocation_fits(long long jump_target, long long max_distance,
elf_word reloc_type)
{
/*
* two's complement encoding
* e.g., [-128=0b10000000, 127=0b01111111] encodable with 8 bits
*/
if (jump_target < 0) {
max_distance++;
}
if (llabs(jump_target) > max_distance) {
LOG_ERR("%lld byte relocation is not possible for type %" PRIu64 " (max %lld)!",
jump_target, (uint64_t)reloc_type, max_distance);
Expand Down
10 changes: 10 additions & 0 deletions tests/subsys/llext/CMakeLists.txt
Original file line number Diff line number Diff line change
Expand Up @@ -79,3 +79,13 @@ if (CONFIG_LLEXT_TYPE_ELF_RELOCATABLE AND NOT CONFIG_ARM AND NOT CONFIG_RISCV)
$<TARGET_OBJECTS:${pre_located_target}> -o ${pre_located_file}
)
endif()

if(NOT CONFIG_LLEXT_TYPE_ELF_OBJECT AND CONFIG_RISCV AND CONFIG_RISCV_ISA_EXT_C)
add_llext_target(riscv_edge_case_cb_type_ext
OUTPUT ${ZEPHYR_BINARY_DIR}/riscv_edge_case_cb_type_ext.llext
SOURCES ${PROJECT_SOURCE_DIR}/src/riscv_edge_case_cb_type.c ${PROJECT_SOURCE_DIR}/src/riscv_edge_case_cb_type_trigger.S
)
generate_inc_file_for_target(app ${ZEPHYR_BINARY_DIR}/riscv_edge_case_cb_type_ext.llext
${ZEPHYR_BINARY_DIR}/include/generated/riscv_edge_case_cb_type.inc
)
endif()
34 changes: 34 additions & 0 deletions tests/subsys/llext/src/riscv_edge_case_cb_type.c
Original file line number Diff line number Diff line change
@@ -0,0 +1,34 @@
/*
* Copyright (c) 2024 CISPA Helmholtz Center for Information Security
*
* SPDX-License-Identifier: Apache-2.0
*/

/*
* This extension tests a relocation edge case in RISC-V:
* Immediates in branch/jump-type instructions are signed-extended.
* Thus, a jump with a negative offset can have a greater jump target than
* a jump with a positive offset.
* A compressed branch (cb-type) instruction is used to trigger the edge case.
* It has a 9-bit immediate (with an implicit LSB of 0), allowing it to jump
* 256 bytes backward and 254 bytes forward.
*/

#include <stdbool.h>
#include <zephyr/llext/symbol.h>
#include <zephyr/ztest_assert.h>

extern int _riscv_edge_case_cb_trigger_forward(void);
extern int _riscv_edge_case_cb_trigger_backward(void);

void test_entry(void)
{
int test_ok;

test_ok = _riscv_edge_case_cb_trigger_forward();
zassert_equal(test_ok, 0x1);

test_ok = _riscv_edge_case_cb_trigger_backward();
zassert_equal(test_ok, 0x1);
}
EXPORT_SYMBOL(test_entry);
120 changes: 120 additions & 0 deletions tests/subsys/llext/src/riscv_edge_case_cb_type_trigger.S
Original file line number Diff line number Diff line change
@@ -0,0 +1,120 @@
/*
* Copyright (c) 2024 CISPA Helmholtz Center for Information Security
*
* SPDX-License-Identifier: Apache-2.0
*/

#include <zephyr/toolchain.h>

GTEXT(_riscv_edge_case_cb_trigger_backward)

/*
* Tests that jumping 256 bytes (the maximum) backwards
* using CB-type instruction is feasible
*/
SECTION_FUNC(TEXT, _riscv_edge_case_cb_trigger_backward)
/*
* tentative fail
* this needs precise alignment - need explicit compressed instructions
*/
addi a0, zero, 0
c.j _do_jump

_backward_jump_target:

/*
* we need to force RISC-V compressed instructions for alignment
* this directive is standard in RISC-V, i.e., not toolchain-specific
*/
.option push
.option rvc

/* we made it to the correct target - success, return true */
c.addi a0, 0x1
/* explicit compressed return */
c.jr ra


/*
* we need a distance of 256 bytes between _do_jump and _backward_jump_target to trigger
* the edge case (max jump distance for c.beqz)
* _backward_jump_target itself needs 4 bytes (two compressed instructions)
* so we need to insert 252 additional padding bytes
* we pad with return instructions here, causing the test to return 0 (failure)
*/
.rept 126
/* explicit compressed return - 2 bytes */
c.jr ra
.endr

_do_jump:
/* jump precisely 256 bytes, the maximum distance, backwards */
c.beqz a0, _backward_jump_target

/*
* in case we erroneously jump FORWARD instead of backwards,
* the jump ends in the following return sled and we return 0
* this indicates test failure
* note that maximum distance for jump forwards is 254 bytes,
* which is also the size of this sled
*/
.rept 127
/* explicit compressed return - 2 bytes */
c.jr ra
.endr

/* assembler can decide whether to emit compressed instructions */
.option pop

GTEXT(_riscv_edge_case_cb_trigger_forward)

/*
* Tests that jumping 256 bytes (the maximum) forwards
* using CB-type instruction is feasible
*/
SECTION_FUNC(TEXT, _riscv_edge_case_cb_trigger_forward)
j _test_start

/* we need to force RISC-V compressed instructions for alignment */
.option push
.option rvc

/*
* in case the relocation is incorrect and the c.beqz jumps BACKWARDS,
* e.g., after arithmetic overflow, we jump into the following return sled
* the return sled is 256 bytes long, covering the maximum backward jump
*/
.rept 128
/* explicit compressed return - 2 bytes */
c.jr ra
.endr

_test_start:
/* tentative fail */
addi a0, zero, 0

/*
* jump precisely 254 bytes, the maximum distance, forwards
* in case the relocation is applied incorrectly, we jump into the padding bytes
* this causes test failure
* we cannot jump too far forwards, 254 bytes is the maximum distance
*/
c.beqz a0, _forward_jump_target

/*
* need to insert 252 padding bytes to pad to 254 byte jump
* we pad with return instructions here, causing the test to return 0 (failure)
*/
.rept 126
/* explicit compressed return - 2 bytes */
c.jr ra
.endr

/* assembler can decide whether to emit compressed instructions */
.option pop

_forward_jump_target:
/* we made it to the correct target - success, return true */
li a0, 1
/* should not be reached - causes return false */
ret
10 changes: 9 additions & 1 deletion tests/subsys/llext/src/test_llext.c
Original file line number Diff line number Diff line change
Expand Up @@ -321,7 +321,15 @@ static LLEXT_CONST uint8_t multi_file_ext[] ELF_ALIGN = {
#include "multi_file.inc"
};
LLEXT_LOAD_UNLOAD(multi_file)
#endif

#if defined(CONFIG_RISCV) && defined(CONFIG_RISCV_ISA_EXT_C)
static LLEXT_CONST uint8_t riscv_edge_case_cb_type_ext[] ELF_ALIGN = {
#include "riscv_edge_case_cb_type.inc"
};
LLEXT_LOAD_UNLOAD(riscv_edge_case_cb_type)
#endif /* CONFIG_RISCV && CONFIG_RISCV_ISA_EXT_C */

#endif /* !CONFIG_LLEXT_TYPE_ELF_OBJECT */

#ifndef CONFIG_USERSPACE
static LLEXT_CONST uint8_t export_dependent_ext[] ELF_ALIGN = {
Expand Down
Loading