riscv/arch_elf: Check for _HI20 relocation validity

As pointed out in #11322 there is a hardware design issue in RISC-V that
affects RV64 relocations. The problem is with how address bits are loaded
into registers via lui / auipc and sign extension.

If the hi20 relocation value happens to have its 32-bit sign bit set, i.e.
value is 0x80000000 (but not negative! i.e. negative in 64-bit format) the
relocation will fail, as the address is erroneously sign extended:

0x00000000_80000000 becomes 0xffffffff_80000000 which is not correct.

Also, make sure the correct opcode is used with PCREL_HI20, it expects
AUIPC (not LUI). The C compiler will never emit such code but when hand-
writing assembly code this can happen.
This commit is contained in:
Ville Juven 2023-12-13 17:52:08 +02:00 committed by Xiang Xiao
parent 80ca3e9308
commit 90d73153fb

View file

@ -36,7 +36,7 @@
* Pre-processor Definitions
****************************************************************************/
#define OPCODE_SW 0x23
#define OPCODE_AUIPC 0x17
#define OPCODE_LUI 0x37
#define RVI_OPCODE_MASK 0x7F
@ -256,6 +256,39 @@ static uintptr_t _find_hi20(void *arch_data, uintptr_t hi20_rel)
return 0;
}
/****************************************************************************
* Name: _valid_hi20_imm
*
* Description:
* Check that any XX_HI20 relocation has a valid upper 20-bit immediate.
* Note that this test is not necessary for RV32 targets, the problem is
* related to RV64 sign extension.
*
* Input Parameters:
* imm_hi - The upper immediate value.
*
* Returned Value:
* true if imm_hi is valid; false otherwise
*
****************************************************************************/
#ifdef CONFIG_LIBC_ARCH_ELF_64BIT
static inline bool _valid_hi20_imm(long imm_hi)
{
/* 32-bit sign extend imm_hi and compare with the original value */
long hi = imm_hi & ((1 << 20) - 1); /* 32-bit signed value */
long sign = -((imm_hi >> 19) & 1); /* 32-bit sign value */
hi = ((hi << 12) | sign << 32) >> 12; /* 32-bit sign extend */
/* If the values do not match, the immediate is invalid */
return imm_hi == hi;
}
#else
# define _valid_hi20_imm(imm_hi) 1
#endif
/****************************************************************************
* Public Functions
****************************************************************************/
@ -445,6 +478,7 @@ int up_relocateadd(const Elf_Rela *rel, const Elf_Sym *sym,
case R_RISCV_PCREL_HI20:
{
uint32_t insn;
long imm_hi;
long imm_lo;
@ -456,8 +490,19 @@ int up_relocateadd(const Elf_Rela *rel, const Elf_Sym *sym,
offset = (long)sym->st_value + (long)rel->r_addend - (long)addr;
insn = _get_val((uint16_t *)addr);
ASSERT(OPCODE_AUIPC == (insn & RVI_OPCODE_MASK));
_calc_imm(offset, &imm_hi, &imm_lo);
if (!_valid_hi20_imm(imm_hi))
{
berr("ERROR: %s at %08" PRIxPTR " bad:%08lx\n",
_get_rname(relotype), addr, imm_hi << 12);
return -EINVAL;
}
/* Adjust auipc (add upper immediate to pc) : 20bit */
_add_val((uint16_t *)addr, imm_hi << 12);
@ -484,6 +529,14 @@ int up_relocateadd(const Elf_Rela *rel, const Elf_Sym *sym,
_calc_imm(offset, &imm_hi, &imm_lo);
if (!_valid_hi20_imm(imm_hi))
{
berr("ERROR: %s at %08" PRIxPTR " bad:%08lx\n",
_get_rname(relotype), addr, imm_hi << 12);
return -EINVAL;
}
/* Adjust auipc (add upper immediate to pc) : 20bit */
_add_val((uint16_t *)addr, imm_hi << 12);
@ -558,6 +611,15 @@ int up_relocateadd(const Elf_Rela *rel, const Elf_Sym *sym,
long imm_hi;
long imm_lo;
_calc_imm(offset, &imm_hi, &imm_lo);
if (!_valid_hi20_imm(imm_hi))
{
berr("ERROR: %s at %08" PRIxPTR " bad:%08lx\n",
_get_rname(relotype), addr, imm_hi << 12);
return -EINVAL;
}
insn = (insn & 0x00000fff) | (imm_hi << 12);
_set_val((uint16_t *)addr, insn);