2023-10-31 07:26:06

by Charlie Jenkins

[permalink] [raw]
Subject: [PATCH v7 0/3] riscv: Add remaining module relocations and tests

A handful of module relocations were missing, this patch includes the
remaining ones. I also wrote some test cases to ensure that module
loading works properly. Some relocations cannot be supported in the
kernel, these include the ones that rely on thread local storage and
dynamic linking.

This patch also overhauls the implementation of ADD/SUB/SET/ULEB128
relocations to handle overflow. "Overflow" is different for ULEB128
since it is a variable-length encoding that the compiler can be expected
to generate enough space for. Instead of overflowing, ULEB128 will
expand into the next 8-bit segment of the location.

A psABI proposal [1] was merged that mandates that SET_ULEB128 and
SUB_ULEB128 are paired, however the discussion following the merging of
the pull request revealed that while the pull request was valid, it
would be better for linkers to properly handle this overflow. This patch
proactively implements this methodology for future compatibility.

This can be tested by enabling KUNIT, RUNTIME_KERNEL_TESTING_MENU, and
RISCV_MODULE_LINKING_KUNIT.

[1] https://github.com/riscv-non-isa/riscv-elf-psabi-doc/pull/403

Signed-off-by: Charlie Jenkins <[email protected]>
---
Changes in v7:
- Overhaul ADD/SUB/SET/ULEB128 relocations
- Fix ULEB128 so it produces correct values when more than 1 byte is
needed
- Link to v6: https://lore.kernel.org/r/[email protected]

Changes in v6:
- Use (void *) instead of (u32 *) for handler type
- Constrain ULEB128 to be consecutive relocations
- Link to v5: https://lore.kernel.org/r/[email protected]

Changes in v5:
- Brought in patch by Emil and fixed it up to force little endian
- Fixed up issues with apply_r_riscv_32_pcrel_rela and
apply_r_riscv_plt32_rela (Samuel)
- Added u8 cast in apply_r_riscv_sub6_rela (Andreas)
- Link to v4: https://lore.kernel.org/r/[email protected]

Changes in v4:
- Complete removal of R_RISCV_RVC_LUI
- Fix bug in R_RISCV_SUB6 linking
- Only build ULEB128 tests if supported by toolchain
- Link to v3: https://lore.kernel.org/r/[email protected]

Changes in v3:
- Add prototypes to test_module_linking_main as recommended by intel
zero day bot
- Improve efficiency of ULEB128 pair matching
- Link to v2: https://lore.kernel.org/r/[email protected]

Changes in v2:
- Added ULEB128 relocations
- Link to v1: https://lore.kernel.org/r/[email protected]

---
Charlie Jenkins (2):
riscv: Add remaining module relocations
riscv: Add tests for riscv module loading

Emil Renner Berthing (1):
riscv: Avoid unaligned access when relocating modules

arch/riscv/Kconfig.debug | 1 +
arch/riscv/include/uapi/asm/elf.h | 5 +-
arch/riscv/kernel/Makefile | 1 +
arch/riscv/kernel/module.c | 685 ++++++++++++++++++---
arch/riscv/kernel/tests/Kconfig.debug | 35 ++
arch/riscv/kernel/tests/Makefile | 1 +
arch/riscv/kernel/tests/module_test/Makefile | 15 +
.../tests/module_test/test_module_linking_main.c | 88 +++
arch/riscv/kernel/tests/module_test/test_set16.S | 23 +
arch/riscv/kernel/tests/module_test/test_set32.S | 20 +
arch/riscv/kernel/tests/module_test/test_set6.S | 23 +
arch/riscv/kernel/tests/module_test/test_set8.S | 23 +
arch/riscv/kernel/tests/module_test/test_sub16.S | 22 +
arch/riscv/kernel/tests/module_test/test_sub32.S | 22 +
arch/riscv/kernel/tests/module_test/test_sub6.S | 22 +
arch/riscv/kernel/tests/module_test/test_sub64.S | 27 +
arch/riscv/kernel/tests/module_test/test_sub8.S | 22 +
arch/riscv/kernel/tests/module_test/test_uleb128.S | 31 +
18 files changed, 963 insertions(+), 103 deletions(-)
---
base-commit: 3bcce01fcbcd868b8cf3a5632fde283e122d7213
change-id: 20230908-module_relocations-f63ced651bd7
--
- Charlie


2023-10-31 07:26:30

by Charlie Jenkins

[permalink] [raw]
Subject: [PATCH v7 1/3] riscv: Avoid unaligned access when relocating modules

From: Emil Renner Berthing <[email protected]>

With the C-extension regular 32bit instructions are not
necessarily aligned on 4-byte boundaries. RISC-V instructions
are in fact an ordered list of 16bit little-endian
"parcels", so access the instruction as such.

This should also make the code work in case someone builds
a big-endian RISC-V machine.

Signed-off-by: Emil Renner Berthing <[email protected]>
Signed-off-by: Charlie Jenkins <[email protected]>
---
arch/riscv/kernel/module.c | 153 +++++++++++++++++++++++----------------------
1 file changed, 77 insertions(+), 76 deletions(-)

diff --git a/arch/riscv/kernel/module.c b/arch/riscv/kernel/module.c
index 7c651d55fcbd..a9e94e939cb5 100644
--- a/arch/riscv/kernel/module.c
+++ b/arch/riscv/kernel/module.c
@@ -27,68 +27,86 @@ static bool riscv_insn_valid_32bit_offset(ptrdiff_t val)
#endif
}

-static int apply_r_riscv_32_rela(struct module *me, u32 *location, Elf_Addr v)
+static int riscv_insn_rmw(void *location, u32 keep, u32 set)
+{
+ u16 *parcel = location;
+ u32 insn = (u32)le16_to_cpu(parcel[0]) | (u32)le16_to_cpu(parcel[1]) << 16;
+
+ insn &= keep;
+ insn |= set;
+
+ parcel[0] = cpu_to_le32(insn);
+ parcel[1] = cpu_to_le16(insn >> 16);
+ return 0;
+}
+
+static int riscv_insn_rvc_rmw(void *location, u16 keep, u16 set)
+{
+ u16 *parcel = location;
+
+ *parcel = cpu_to_le16((le16_to_cpu(*parcel) & keep) | set);
+ return 0;
+}
+
+static int apply_r_riscv_32_rela(struct module *me, void *location, Elf_Addr v)
{
if (v != (u32)v) {
pr_err("%s: value %016llx out of range for 32-bit field\n",
me->name, (long long)v);
return -EINVAL;
}
- *location = v;
+ *(u32 *)location = v;
return 0;
}

-static int apply_r_riscv_64_rela(struct module *me, u32 *location, Elf_Addr v)
+static int apply_r_riscv_64_rela(struct module *me, void *location, Elf_Addr v)
{
*(u64 *)location = v;
return 0;
}

-static int apply_r_riscv_branch_rela(struct module *me, u32 *location,
+static int apply_r_riscv_branch_rela(struct module *me, void *location,
Elf_Addr v)
{
- ptrdiff_t offset = (void *)v - (void *)location;
+ ptrdiff_t offset = (void *)v - location;
u32 imm12 = (offset & 0x1000) << (31 - 12);
u32 imm11 = (offset & 0x800) >> (11 - 7);
u32 imm10_5 = (offset & 0x7e0) << (30 - 10);
u32 imm4_1 = (offset & 0x1e) << (11 - 4);

- *location = (*location & 0x1fff07f) | imm12 | imm11 | imm10_5 | imm4_1;
- return 0;
+ return riscv_insn_rmw(location, 0x1fff07f, imm12 | imm11 | imm10_5 | imm4_1);
}

-static int apply_r_riscv_jal_rela(struct module *me, u32 *location,
+static int apply_r_riscv_jal_rela(struct module *me, void *location,
Elf_Addr v)
{
- ptrdiff_t offset = (void *)v - (void *)location;
+ ptrdiff_t offset = (void *)v - location;
u32 imm20 = (offset & 0x100000) << (31 - 20);
u32 imm19_12 = (offset & 0xff000);
u32 imm11 = (offset & 0x800) << (20 - 11);
u32 imm10_1 = (offset & 0x7fe) << (30 - 10);

- *location = (*location & 0xfff) | imm20 | imm19_12 | imm11 | imm10_1;
- return 0;
+ return riscv_insn_rmw(location, 0xfff, imm20 | imm19_12 | imm11 | imm10_1);
}

-static int apply_r_riscv_rvc_branch_rela(struct module *me, u32 *location,
+static int apply_r_riscv_rvc_branch_rela(struct module *me, void *location,
Elf_Addr v)
{
- ptrdiff_t offset = (void *)v - (void *)location;
+ ptrdiff_t offset = (void *)v - location;
u16 imm8 = (offset & 0x100) << (12 - 8);
u16 imm7_6 = (offset & 0xc0) >> (6 - 5);
u16 imm5 = (offset & 0x20) >> (5 - 2);
u16 imm4_3 = (offset & 0x18) << (12 - 5);
u16 imm2_1 = (offset & 0x6) << (12 - 10);

- *(u16 *)location = (*(u16 *)location & 0xe383) |
- imm8 | imm7_6 | imm5 | imm4_3 | imm2_1;
- return 0;
+ return riscv_insn_rvc_rmw(location, 0xe383,
+ imm8 | imm7_6 | imm5 | imm4_3 | imm2_1);
}

-static int apply_r_riscv_rvc_jump_rela(struct module *me, u32 *location,
+static int apply_r_riscv_rvc_jump_rela(struct module *me, void *location,
Elf_Addr v)
{
- ptrdiff_t offset = (void *)v - (void *)location;
+ ptrdiff_t offset = (void *)v - location;
u16 imm11 = (offset & 0x800) << (12 - 11);
u16 imm10 = (offset & 0x400) >> (10 - 8);
u16 imm9_8 = (offset & 0x300) << (12 - 11);
@@ -98,16 +116,14 @@ static int apply_r_riscv_rvc_jump_rela(struct module *me, u32 *location,
u16 imm4 = (offset & 0x10) << (12 - 5);
u16 imm3_1 = (offset & 0xe) << (12 - 10);

- *(u16 *)location = (*(u16 *)location & 0xe003) |
- imm11 | imm10 | imm9_8 | imm7 | imm6 | imm5 | imm4 | imm3_1;
- return 0;
+ return riscv_insn_rvc_rmw(location, 0xe003,
+ imm11 | imm10 | imm9_8 | imm7 | imm6 | imm5 | imm4 | imm3_1);
}

-static int apply_r_riscv_pcrel_hi20_rela(struct module *me, u32 *location,
+static int apply_r_riscv_pcrel_hi20_rela(struct module *me, void *location,
Elf_Addr v)
{
- ptrdiff_t offset = (void *)v - (void *)location;
- s32 hi20;
+ ptrdiff_t offset = (void *)v - location;

if (!riscv_insn_valid_32bit_offset(offset)) {
pr_err(
@@ -116,23 +132,20 @@ static int apply_r_riscv_pcrel_hi20_rela(struct module *me, u32 *location,
return -EINVAL;
}

- hi20 = (offset + 0x800) & 0xfffff000;
- *location = (*location & 0xfff) | hi20;
- return 0;
+ return riscv_insn_rmw(location, 0xfff, (offset + 0x800) & 0xfffff000);
}

-static int apply_r_riscv_pcrel_lo12_i_rela(struct module *me, u32 *location,
+static int apply_r_riscv_pcrel_lo12_i_rela(struct module *me, void *location,
Elf_Addr v)
{
/*
* v is the lo12 value to fill. It is calculated before calling this
* handler.
*/
- *location = (*location & 0xfffff) | ((v & 0xfff) << 20);
- return 0;
+ return riscv_insn_rmw(location, 0xfffff, (v & 0xfff) << 20);
}

-static int apply_r_riscv_pcrel_lo12_s_rela(struct module *me, u32 *location,
+static int apply_r_riscv_pcrel_lo12_s_rela(struct module *me, void *location,
Elf_Addr v)
{
/*
@@ -142,15 +155,12 @@ static int apply_r_riscv_pcrel_lo12_s_rela(struct module *me, u32 *location,
u32 imm11_5 = (v & 0xfe0) << (31 - 11);
u32 imm4_0 = (v & 0x1f) << (11 - 4);

- *location = (*location & 0x1fff07f) | imm11_5 | imm4_0;
- return 0;
+ return riscv_insn_rmw(location, 0x1fff07f, imm11_5 | imm4_0);
}

-static int apply_r_riscv_hi20_rela(struct module *me, u32 *location,
+static int apply_r_riscv_hi20_rela(struct module *me, void *location,
Elf_Addr v)
{
- s32 hi20;
-
if (IS_ENABLED(CONFIG_CMODEL_MEDLOW)) {
pr_err(
"%s: target %016llx can not be addressed by the 32-bit offset from PC = %p\n",
@@ -158,22 +168,20 @@ static int apply_r_riscv_hi20_rela(struct module *me, u32 *location,
return -EINVAL;
}

- hi20 = ((s32)v + 0x800) & 0xfffff000;
- *location = (*location & 0xfff) | hi20;
- return 0;
+ return riscv_insn_rmw(location, 0xfff, ((s32)v + 0x800) & 0xfffff000);
}

-static int apply_r_riscv_lo12_i_rela(struct module *me, u32 *location,
+static int apply_r_riscv_lo12_i_rela(struct module *me, void *location,
Elf_Addr v)
{
/* Skip medlow checking because of filtering by HI20 already */
s32 hi20 = ((s32)v + 0x800) & 0xfffff000;
s32 lo12 = ((s32)v - hi20);
- *location = (*location & 0xfffff) | ((lo12 & 0xfff) << 20);
- return 0;
+
+ return riscv_insn_rmw(location, 0xfffff, (lo12 & 0xfff) << 20);
}

-static int apply_r_riscv_lo12_s_rela(struct module *me, u32 *location,
+static int apply_r_riscv_lo12_s_rela(struct module *me, void *location,
Elf_Addr v)
{
/* Skip medlow checking because of filtering by HI20 already */
@@ -181,20 +189,18 @@ static int apply_r_riscv_lo12_s_rela(struct module *me, u32 *location,
s32 lo12 = ((s32)v - hi20);
u32 imm11_5 = (lo12 & 0xfe0) << (31 - 11);
u32 imm4_0 = (lo12 & 0x1f) << (11 - 4);
- *location = (*location & 0x1fff07f) | imm11_5 | imm4_0;
- return 0;
+
+ return riscv_insn_rmw(location, 0x1fff07f, imm11_5 | imm4_0);
}

-static int apply_r_riscv_got_hi20_rela(struct module *me, u32 *location,
+static int apply_r_riscv_got_hi20_rela(struct module *me, void *location,
Elf_Addr v)
{
- ptrdiff_t offset = (void *)v - (void *)location;
- s32 hi20;
+ ptrdiff_t offset = (void *)v - location;

/* Always emit the got entry */
if (IS_ENABLED(CONFIG_MODULE_SECTIONS)) {
- offset = module_emit_got_entry(me, v);
- offset = (void *)offset - (void *)location;
+ offset = (void *)module_emit_got_entry(me, v) - location;
} else {
pr_err(
"%s: can not generate the GOT entry for symbol = %016llx from PC = %p\n",
@@ -202,22 +208,19 @@ static int apply_r_riscv_got_hi20_rela(struct module *me, u32 *location,
return -EINVAL;
}

- hi20 = (offset + 0x800) & 0xfffff000;
- *location = (*location & 0xfff) | hi20;
- return 0;
+ return riscv_insn_rmw(location, 0xfff, (offset + 0x800) & 0xfffff000);
}

-static int apply_r_riscv_call_plt_rela(struct module *me, u32 *location,
+static int apply_r_riscv_call_plt_rela(struct module *me, void *location,
Elf_Addr v)
{
- ptrdiff_t offset = (void *)v - (void *)location;
+ ptrdiff_t offset = (void *)v - location;
u32 hi20, lo12;

if (!riscv_insn_valid_32bit_offset(offset)) {
/* Only emit the plt entry if offset over 32-bit range */
if (IS_ENABLED(CONFIG_MODULE_SECTIONS)) {
- offset = module_emit_plt_entry(me, v);
- offset = (void *)offset - (void *)location;
+ offset = (void *)module_emit_plt_entry(me, v) - location;
} else {
pr_err(
"%s: target %016llx can not be addressed by the 32-bit offset from PC = %p\n",
@@ -228,15 +231,14 @@ static int apply_r_riscv_call_plt_rela(struct module *me, u32 *location,

hi20 = (offset + 0x800) & 0xfffff000;
lo12 = (offset - hi20) & 0xfff;
- *location = (*location & 0xfff) | hi20;
- *(location + 1) = (*(location + 1) & 0xfffff) | (lo12 << 20);
- return 0;
+ riscv_insn_rmw(location, 0xfff, hi20);
+ return riscv_insn_rmw(location + 4, 0xfffff, lo12 << 20);
}

-static int apply_r_riscv_call_rela(struct module *me, u32 *location,
+static int apply_r_riscv_call_rela(struct module *me, void *location,
Elf_Addr v)
{
- ptrdiff_t offset = (void *)v - (void *)location;
+ ptrdiff_t offset = (void *)v - location;
u32 hi20, lo12;

if (!riscv_insn_valid_32bit_offset(offset)) {
@@ -248,18 +250,17 @@ static int apply_r_riscv_call_rela(struct module *me, u32 *location,

hi20 = (offset + 0x800) & 0xfffff000;
lo12 = (offset - hi20) & 0xfff;
- *location = (*location & 0xfff) | hi20;
- *(location + 1) = (*(location + 1) & 0xfffff) | (lo12 << 20);
- return 0;
+ riscv_insn_rmw(location, 0xfff, hi20);
+ return riscv_insn_rmw(location + 4, 0xfffff, lo12 << 20);
}

-static int apply_r_riscv_relax_rela(struct module *me, u32 *location,
+static int apply_r_riscv_relax_rela(struct module *me, void *location,
Elf_Addr v)
{
return 0;
}

-static int apply_r_riscv_align_rela(struct module *me, u32 *location,
+static int apply_r_riscv_align_rela(struct module *me, void *location,
Elf_Addr v)
{
pr_err(
@@ -268,49 +269,49 @@ static int apply_r_riscv_align_rela(struct module *me, u32 *location,
return -EINVAL;
}

-static int apply_r_riscv_add16_rela(struct module *me, u32 *location,
+static int apply_r_riscv_add16_rela(struct module *me, void *location,
Elf_Addr v)
{
*(u16 *)location += (u16)v;
return 0;
}

-static int apply_r_riscv_add32_rela(struct module *me, u32 *location,
+static int apply_r_riscv_add32_rela(struct module *me, void *location,
Elf_Addr v)
{
*(u32 *)location += (u32)v;
return 0;
}

-static int apply_r_riscv_add64_rela(struct module *me, u32 *location,
+static int apply_r_riscv_add64_rela(struct module *me, void *location,
Elf_Addr v)
{
*(u64 *)location += (u64)v;
return 0;
}

-static int apply_r_riscv_sub16_rela(struct module *me, u32 *location,
+static int apply_r_riscv_sub16_rela(struct module *me, void *location,
Elf_Addr v)
{
*(u16 *)location -= (u16)v;
return 0;
}

-static int apply_r_riscv_sub32_rela(struct module *me, u32 *location,
+static int apply_r_riscv_sub32_rela(struct module *me, void *location,
Elf_Addr v)
{
*(u32 *)location -= (u32)v;
return 0;
}

-static int apply_r_riscv_sub64_rela(struct module *me, u32 *location,
+static int apply_r_riscv_sub64_rela(struct module *me, void *location,
Elf_Addr v)
{
*(u64 *)location -= (u64)v;
return 0;
}

-static int (*reloc_handlers_rela[]) (struct module *me, u32 *location,
+static int (*reloc_handlers_rela[]) (struct module *me, void *location,
Elf_Addr v) = {
[R_RISCV_32] = apply_r_riscv_32_rela,
[R_RISCV_64] = apply_r_riscv_64_rela,
@@ -342,9 +343,9 @@ int apply_relocate_add(Elf_Shdr *sechdrs, const char *strtab,
struct module *me)
{
Elf_Rela *rel = (void *) sechdrs[relsec].sh_addr;
- int (*handler)(struct module *me, u32 *location, Elf_Addr v);
+ int (*handler)(struct module *me, void *location, Elf_Addr v);
Elf_Sym *sym;
- u32 *location;
+ void *location;
unsigned int i, type;
Elf_Addr v;
int res;

--
2.34.1

2023-10-31 07:26:40

by Charlie Jenkins

[permalink] [raw]
Subject: [PATCH v7 2/3] riscv: Add remaining module relocations

Add all final module relocations and add error logs explaining the ones
that are not supported. Implement overflow checks for
ADD/SUB/SET/ULEB128 relocations.

Signed-off-by: Charlie Jenkins <[email protected]>
---
arch/riscv/include/uapi/asm/elf.h | 5 +-
arch/riscv/kernel/module.c | 534 ++++++++++++++++++++++++++++++++++++--
2 files changed, 511 insertions(+), 28 deletions(-)

diff --git a/arch/riscv/include/uapi/asm/elf.h b/arch/riscv/include/uapi/asm/elf.h
index d696d6610231..11a71b8533d5 100644
--- a/arch/riscv/include/uapi/asm/elf.h
+++ b/arch/riscv/include/uapi/asm/elf.h
@@ -49,6 +49,7 @@ typedef union __riscv_fp_state elf_fpregset_t;
#define R_RISCV_TLS_DTPREL64 9
#define R_RISCV_TLS_TPREL32 10
#define R_RISCV_TLS_TPREL64 11
+#define R_RISCV_IRELATIVE 58

/* Relocation types not used by the dynamic linker */
#define R_RISCV_BRANCH 16
@@ -81,7 +82,6 @@ typedef union __riscv_fp_state elf_fpregset_t;
#define R_RISCV_ALIGN 43
#define R_RISCV_RVC_BRANCH 44
#define R_RISCV_RVC_JUMP 45
-#define R_RISCV_LUI 46
#define R_RISCV_GPREL_I 47
#define R_RISCV_GPREL_S 48
#define R_RISCV_TPREL_I 49
@@ -93,6 +93,9 @@ typedef union __riscv_fp_state elf_fpregset_t;
#define R_RISCV_SET16 55
#define R_RISCV_SET32 56
#define R_RISCV_32_PCREL 57
+#define R_RISCV_PLT32 59
+#define R_RISCV_SET_ULEB128 60
+#define R_RISCV_SUB_ULEB128 61


#endif /* _UAPI_ASM_RISCV_ELF_H */
diff --git a/arch/riscv/kernel/module.c b/arch/riscv/kernel/module.c
index a9e94e939cb5..230172ecb26e 100644
--- a/arch/riscv/kernel/module.c
+++ b/arch/riscv/kernel/module.c
@@ -7,6 +7,9 @@
#include <linux/elf.h>
#include <linux/err.h>
#include <linux/errno.h>
+#include <linux/hashtable.h>
+#include <linux/kernel.h>
+#include <linux/log2.h>
#include <linux/moduleloader.h>
#include <linux/vmalloc.h>
#include <linux/sizes.h>
@@ -14,6 +17,27 @@
#include <asm/alternative.h>
#include <asm/sections.h>

+struct used_bucket {
+ struct list_head head;
+ struct hlist_head *bucket;
+};
+
+struct relocation_head {
+ struct hlist_node node;
+ struct list_head *rel_entry;
+ void *location;
+};
+
+struct relocation_entry {
+ struct list_head head;
+ Elf_Addr value;
+ unsigned int type;
+};
+
+struct hlist_head *relocation_hashtable;
+
+struct list_head used_buckets_list;
+
/*
* The auipc+jalr instruction pair can reach any PC-relative offset
* in the range [-2^31 - 2^11, 2^31 - 2^11)
@@ -269,6 +293,12 @@ static int apply_r_riscv_align_rela(struct module *me, void *location,
return -EINVAL;
}

+static int apply_r_riscv_add8_rela(struct module *me, void *location, Elf_Addr v)
+{
+ *(u8 *)location += (u8)v;
+ return 0;
+}
+
static int apply_r_riscv_add16_rela(struct module *me, void *location,
Elf_Addr v)
{
@@ -290,6 +320,12 @@ static int apply_r_riscv_add64_rela(struct module *me, void *location,
return 0;
}

+static int apply_r_riscv_sub8_rela(struct module *me, void *location, Elf_Addr v)
+{
+ *(u8 *)location -= (u8)v;
+ return 0;
+}
+
static int apply_r_riscv_sub16_rela(struct module *me, void *location,
Elf_Addr v)
{
@@ -311,33 +347,470 @@ static int apply_r_riscv_sub64_rela(struct module *me, void *location,
return 0;
}

-static int (*reloc_handlers_rela[]) (struct module *me, void *location,
- Elf_Addr v) = {
- [R_RISCV_32] = apply_r_riscv_32_rela,
- [R_RISCV_64] = apply_r_riscv_64_rela,
- [R_RISCV_BRANCH] = apply_r_riscv_branch_rela,
- [R_RISCV_JAL] = apply_r_riscv_jal_rela,
- [R_RISCV_RVC_BRANCH] = apply_r_riscv_rvc_branch_rela,
- [R_RISCV_RVC_JUMP] = apply_r_riscv_rvc_jump_rela,
- [R_RISCV_PCREL_HI20] = apply_r_riscv_pcrel_hi20_rela,
- [R_RISCV_PCREL_LO12_I] = apply_r_riscv_pcrel_lo12_i_rela,
- [R_RISCV_PCREL_LO12_S] = apply_r_riscv_pcrel_lo12_s_rela,
- [R_RISCV_HI20] = apply_r_riscv_hi20_rela,
- [R_RISCV_LO12_I] = apply_r_riscv_lo12_i_rela,
- [R_RISCV_LO12_S] = apply_r_riscv_lo12_s_rela,
- [R_RISCV_GOT_HI20] = apply_r_riscv_got_hi20_rela,
- [R_RISCV_CALL_PLT] = apply_r_riscv_call_plt_rela,
- [R_RISCV_CALL] = apply_r_riscv_call_rela,
- [R_RISCV_RELAX] = apply_r_riscv_relax_rela,
- [R_RISCV_ALIGN] = apply_r_riscv_align_rela,
- [R_RISCV_ADD16] = apply_r_riscv_add16_rela,
- [R_RISCV_ADD32] = apply_r_riscv_add32_rela,
- [R_RISCV_ADD64] = apply_r_riscv_add64_rela,
- [R_RISCV_SUB16] = apply_r_riscv_sub16_rela,
- [R_RISCV_SUB32] = apply_r_riscv_sub32_rela,
- [R_RISCV_SUB64] = apply_r_riscv_sub64_rela,
+static int dynamic_linking_not_supported(struct module *me, void *location,
+ Elf_Addr v)
+{
+ pr_err("%s: Dynamic linking not supported in kernel modules PC = %p\n",
+ me->name, location);
+ return -EINVAL;
+}
+
+static int tls_not_supported(struct module *me, void *location, Elf_Addr v)
+{
+ pr_err("%s: Thread local storage not supported in kernel modules PC = %p\n",
+ me->name, location);
+ return -EINVAL;
+}
+
+static int apply_r_riscv_sub6_rela(struct module *me, void *location, Elf_Addr v)
+{
+ *(u8 *)location = (*(u8 *)location - ((u8)v & 0x3F)) & 0x3F;
+ return 0;
+}
+
+static int apply_r_riscv_set6_rela(struct module *me, void *location, Elf_Addr v)
+{
+ *(u8 *)location = ((*(u8 *)location & 0xc0) | ((u8)v & 0x3F));
+ return 0;
+}
+
+static int apply_r_riscv_set8_rela(struct module *me, void *location, Elf_Addr v)
+{
+ *(u8 *)location = (u8)v;
+ return 0;
+}
+
+static int apply_r_riscv_set16_rela(struct module *me, void *location,
+ Elf_Addr v)
+{
+ *(u16 *)location = (u16)v;
+ return 0;
+}
+
+static int apply_r_riscv_set32_rela(struct module *me, void *location,
+ Elf_Addr v)
+{
+ *(u32 *)location = (u32)v;
+ return 0;
+}
+
+static int apply_r_riscv_32_pcrel_rela(struct module *me, void *location,
+ Elf_Addr v)
+{
+ *(u32 *)location = v - (unsigned long)location;
+ return 0;
+}
+
+static int apply_r_riscv_plt32_rela(struct module *me, void *location,
+ Elf_Addr v)
+{
+ ptrdiff_t offset = (void *)v - location;
+
+ if (!riscv_insn_valid_32bit_offset(offset)) {
+ /* Only emit the plt entry if offset over 32-bit range */
+ if (IS_ENABLED(CONFIG_MODULE_SECTIONS)) {
+ offset = (void *)module_emit_plt_entry(me, v) - location;
+ } else {
+ pr_err("%s: target %016llx can not be addressed by the 32-bit offset from PC = %p\n",
+ me->name, (long long)v, location);
+ return -EINVAL;
+ }
+ }
+
+ *(u32 *)location = (u32)offset;
+ return 0;
+}
+
+static int apply_r_riscv_set_uleb128(struct module *me, void *location, Elf_Addr v)
+{
+ *(long *)location = v;
+ return 0;
+}
+
+static int apply_r_riscv_sub_uleb128(struct module *me, void *location, Elf_Addr v)
+{
+ *(long *)location -= v;
+ return 0;
+}
+
+/*
+ * Relocations defined in the riscv-elf-psabi-doc.
+ * This handles static linking only.
+ */
+static int (*reloc_handlers_rela[])(struct module *me, void *location,
+ Elf_Addr v) = {
+ [R_RISCV_32] = apply_r_riscv_32_rela,
+ [R_RISCV_64] = apply_r_riscv_64_rela,
+ [R_RISCV_RELATIVE] = dynamic_linking_not_supported,
+ [R_RISCV_COPY] = dynamic_linking_not_supported,
+ [R_RISCV_JUMP_SLOT] = dynamic_linking_not_supported,
+ [R_RISCV_TLS_DTPMOD32] = dynamic_linking_not_supported,
+ [R_RISCV_TLS_DTPMOD64] = dynamic_linking_not_supported,
+ [R_RISCV_TLS_DTPREL32] = dynamic_linking_not_supported,
+ [R_RISCV_TLS_DTPREL64] = dynamic_linking_not_supported,
+ [R_RISCV_TLS_TPREL32] = dynamic_linking_not_supported,
+ [R_RISCV_TLS_TPREL64] = dynamic_linking_not_supported,
+ /* 12-15 undefined */
+ [R_RISCV_BRANCH] = apply_r_riscv_branch_rela,
+ [R_RISCV_JAL] = apply_r_riscv_jal_rela,
+ [R_RISCV_CALL] = apply_r_riscv_call_rela,
+ [R_RISCV_CALL_PLT] = apply_r_riscv_call_plt_rela,
+ [R_RISCV_GOT_HI20] = apply_r_riscv_got_hi20_rela,
+ [R_RISCV_TLS_GOT_HI20] = tls_not_supported,
+ [R_RISCV_TLS_GD_HI20] = tls_not_supported,
+ [R_RISCV_PCREL_HI20] = apply_r_riscv_pcrel_hi20_rela,
+ [R_RISCV_PCREL_LO12_I] = apply_r_riscv_pcrel_lo12_i_rela,
+ [R_RISCV_PCREL_LO12_S] = apply_r_riscv_pcrel_lo12_s_rela,
+ [R_RISCV_HI20] = apply_r_riscv_hi20_rela,
+ [R_RISCV_LO12_I] = apply_r_riscv_lo12_i_rela,
+ [R_RISCV_LO12_S] = apply_r_riscv_lo12_s_rela,
+ [R_RISCV_TPREL_HI20] = tls_not_supported,
+ [R_RISCV_TPREL_LO12_I] = tls_not_supported,
+ [R_RISCV_TPREL_LO12_S] = tls_not_supported,
+ [R_RISCV_TPREL_ADD] = tls_not_supported,
+ [R_RISCV_ADD8] = apply_r_riscv_add8_rela,
+ [R_RISCV_ADD16] = apply_r_riscv_add16_rela,
+ [R_RISCV_ADD32] = apply_r_riscv_add32_rela,
+ [R_RISCV_ADD64] = apply_r_riscv_add64_rela,
+ [R_RISCV_SUB8] = apply_r_riscv_sub8_rela,
+ [R_RISCV_SUB16] = apply_r_riscv_sub16_rela,
+ [R_RISCV_SUB32] = apply_r_riscv_sub32_rela,
+ [R_RISCV_SUB64] = apply_r_riscv_sub64_rela,
+ /* 41-42 reserved for future standard use */
+ [R_RISCV_ALIGN] = apply_r_riscv_align_rela,
+ [R_RISCV_RVC_BRANCH] = apply_r_riscv_rvc_branch_rela,
+ [R_RISCV_RVC_JUMP] = apply_r_riscv_rvc_jump_rela,
+ /* 46-50 reserved for future standard use */
+ [R_RISCV_RELAX] = apply_r_riscv_relax_rela,
+ [R_RISCV_SUB6] = apply_r_riscv_sub6_rela,
+ [R_RISCV_SET6] = apply_r_riscv_set6_rela,
+ [R_RISCV_SET8] = apply_r_riscv_set8_rela,
+ [R_RISCV_SET16] = apply_r_riscv_set16_rela,
+ [R_RISCV_SET32] = apply_r_riscv_set32_rela,
+ [R_RISCV_32_PCREL] = apply_r_riscv_32_pcrel_rela,
+ [R_RISCV_IRELATIVE] = dynamic_linking_not_supported,
+ [R_RISCV_PLT32] = apply_r_riscv_plt32_rela,
+ [R_RISCV_SET_ULEB128] = apply_r_riscv_set_uleb128,
+ [R_RISCV_SUB_ULEB128] = apply_r_riscv_sub_uleb128,
+ /* 62-191 reserved for future standard use */
+ /* 192-255 nonstandard ABI extensions */
+};
+
+static bool accumulate_relocations[] = {
+ [R_RISCV_32] = false,
+ [R_RISCV_64] = false,
+ [R_RISCV_RELATIVE] = false,
+ [R_RISCV_COPY] = false,
+ [R_RISCV_JUMP_SLOT] = false,
+ [R_RISCV_TLS_DTPMOD32] = false,
+ [R_RISCV_TLS_DTPMOD64] = false,
+ [R_RISCV_TLS_DTPREL32] = false,
+ [R_RISCV_TLS_DTPREL64] = false,
+ [R_RISCV_TLS_TPREL32] = false,
+ [R_RISCV_TLS_TPREL64] = false,
+ /* 12-15 undefined */
+ [R_RISCV_BRANCH] = false,
+ [R_RISCV_JAL] = false,
+ [R_RISCV_CALL] = false,
+ [R_RISCV_CALL_PLT] = false,
+ [R_RISCV_GOT_HI20] = false,
+ [R_RISCV_TLS_GOT_HI20] = false,
+ [R_RISCV_TLS_GD_HI20] = false,
+ [R_RISCV_PCREL_HI20] = false,
+ [R_RISCV_PCREL_LO12_I] = false,
+ [R_RISCV_PCREL_LO12_S] = false,
+ [R_RISCV_HI20] = false,
+ [R_RISCV_LO12_I] = false,
+ [R_RISCV_LO12_S] = false,
+ [R_RISCV_TPREL_HI20] = false,
+ [R_RISCV_TPREL_LO12_I] = false,
+ [R_RISCV_TPREL_LO12_S] = false,
+ [R_RISCV_TPREL_ADD] = false,
+ [R_RISCV_ADD8] = true,
+ [R_RISCV_ADD16] = true,
+ [R_RISCV_ADD32] = true,
+ [R_RISCV_ADD64] = true,
+ [R_RISCV_SUB8] = true,
+ [R_RISCV_SUB16] = true,
+ [R_RISCV_SUB32] = true,
+ [R_RISCV_SUB64] = true,
+ /* 41-42 reserved for future standard use */
+ [R_RISCV_ALIGN] = false,
+ [R_RISCV_RVC_BRANCH] = false,
+ [R_RISCV_RVC_JUMP] = false,
+ /* 46-50 reserved for future standard use */
+ [R_RISCV_RELAX] = false,
+ [R_RISCV_SUB6] = true,
+ [R_RISCV_SET6] = true,
+ [R_RISCV_SET8] = true,
+ [R_RISCV_SET16] = true,
+ [R_RISCV_SET32] = true,
+ [R_RISCV_32_PCREL] = false,
+ [R_RISCV_IRELATIVE] = false,
+ [R_RISCV_PLT32] = false,
+ [R_RISCV_SET_ULEB128] = true,
+ [R_RISCV_SUB_ULEB128] = true,
+ /* 62-191 reserved for future standard use */
+ /* 192-255 nonstandard ABI extensions */
+};
+
+static int accumulation_not_supported(struct module *me, void *location, long buffer)
+{
+ pr_err("%s: Internal error. Only ADD/SUB/SET/ULEB128 should be accumulated.", me->name);
+ return -EINVAL;
+}
+
+static int apply_6_bit_accumulation(struct module *me, void *location, long buffer)
+{
+ if (buffer != (buffer & 0x3F)) {
+ pr_err("%s: value %ld out of range for 6-bit relocation.\n",
+ me->name, buffer);
+ return -EINVAL;
+ }
+ *(u8 *)location = ((*(u8 *)location & 0xc0) | ((u8)buffer & 0x3F));
+ return 0;
+}
+
+static int apply_8_bit_accumulation(struct module *me, void *location, long buffer)
+{
+ if (buffer != (u8)buffer) {
+ pr_err("%s: value %ld out of range for 8-bit relocation.\n",
+ me->name, buffer);
+ return -EINVAL;
+ }
+ *(u8 *)location = (u8)buffer;
+ return 0;
+}
+
+static int apply_16_bit_accumulation(struct module *me, void *location, long buffer)
+{
+ if (buffer != (u16)buffer) {
+ pr_err("%s: value %ld out of range for 16-bit relocation.\n",
+ me->name, buffer);
+ return -EINVAL;
+ }
+ *(u16 *)location = (u16)buffer;
+ return 0;
+}
+
+static int apply_32_bit_accumulation(struct module *me, void *location, long buffer)
+{
+ if (buffer != (u32)buffer) {
+ pr_err("%s: value %ld out of range for 32-bit relocation.\n",
+ me->name, buffer);
+ return -EINVAL;
+ }
+ *(u32 *)location = (u32)buffer;
+ return 0;
+}
+
+static int apply_64_bit_accumulation(struct module *me, void *location, long buffer)
+{
+ *(u64 *)location = (u64)buffer;
+ return 0;
+}
+
+static int apply_uleb128_accumulation(struct module *me, void *location, long buffer)
+{
+ /*
+ * ULEB128 is a variable length encoding. Encode the buffer into
+ * the ULEB128 data format.
+ */
+ while (buffer != 0) {
+ *(u8 *)location = (u8)buffer & 0x7F;
+ buffer >>= 7;
+ *(u8 *)location |= (buffer != 0) << 7;
+ location = (u8 *)location + 1;
+ }
+ return 0;
+}
+
+/*
+ * Need to duplicate this a third time to capture the handlers for accumulation.
+ */
+static int (*accumulate_handlers[])(struct module *me, void *location, long buffer) = {
+ [R_RISCV_32] = accumulation_not_supported,
+ [R_RISCV_64] = accumulation_not_supported,
+ [R_RISCV_RELATIVE] = accumulation_not_supported,
+ [R_RISCV_COPY] = accumulation_not_supported,
+ [R_RISCV_JUMP_SLOT] = accumulation_not_supported,
+ [R_RISCV_TLS_DTPMOD32] = accumulation_not_supported,
+ [R_RISCV_TLS_DTPMOD64] = accumulation_not_supported,
+ [R_RISCV_TLS_DTPREL32] = accumulation_not_supported,
+ [R_RISCV_TLS_DTPREL64] = accumulation_not_supported,
+ [R_RISCV_TLS_TPREL32] = accumulation_not_supported,
+ [R_RISCV_TLS_TPREL64] = accumulation_not_supported,
+ /* 12-15 undefined */
+ [R_RISCV_BRANCH] = accumulation_not_supported,
+ [R_RISCV_JAL] = accumulation_not_supported,
+ [R_RISCV_CALL] = accumulation_not_supported,
+ [R_RISCV_CALL_PLT] = accumulation_not_supported,
+ [R_RISCV_GOT_HI20] = accumulation_not_supported,
+ [R_RISCV_TLS_GOT_HI20] = accumulation_not_supported,
+ [R_RISCV_TLS_GD_HI20] = accumulation_not_supported,
+ [R_RISCV_PCREL_HI20] = accumulation_not_supported,
+ [R_RISCV_PCREL_LO12_I] = accumulation_not_supported,
+ [R_RISCV_PCREL_LO12_S] = accumulation_not_supported,
+ [R_RISCV_HI20] = accumulation_not_supported,
+ [R_RISCV_LO12_I] = accumulation_not_supported,
+ [R_RISCV_LO12_S] = accumulation_not_supported,
+ [R_RISCV_TPREL_HI20] = accumulation_not_supported,
+ [R_RISCV_TPREL_LO12_I] = accumulation_not_supported,
+ [R_RISCV_TPREL_LO12_S] = accumulation_not_supported,
+ [R_RISCV_TPREL_ADD] = accumulation_not_supported,
+ [R_RISCV_ADD8] = apply_8_bit_accumulation,
+ [R_RISCV_ADD16] = apply_16_bit_accumulation,
+ [R_RISCV_ADD32] = apply_32_bit_accumulation,
+ [R_RISCV_ADD64] = apply_64_bit_accumulation,
+ [R_RISCV_SUB8] = apply_8_bit_accumulation,
+ [R_RISCV_SUB16] = apply_16_bit_accumulation,
+ [R_RISCV_SUB32] = apply_32_bit_accumulation,
+ [R_RISCV_SUB64] = apply_64_bit_accumulation,
+ /* 41-42 reserved for future standard use */
+ [R_RISCV_ALIGN] = accumulation_not_supported,
+ [R_RISCV_RVC_BRANCH] = accumulation_not_supported,
+ [R_RISCV_RVC_JUMP] = accumulation_not_supported,
+ /* 46-50 reserved for future standard use */
+ [R_RISCV_RELAX] = accumulation_not_supported,
+ [R_RISCV_SUB6] = apply_6_bit_accumulation,
+ [R_RISCV_SET6] = apply_6_bit_accumulation,
+ [R_RISCV_SET8] = apply_8_bit_accumulation,
+ [R_RISCV_SET16] = apply_16_bit_accumulation,
+ [R_RISCV_SET32] = apply_32_bit_accumulation,
+ [R_RISCV_32_PCREL] = accumulation_not_supported,
+ [R_RISCV_IRELATIVE] = accumulation_not_supported,
+ [R_RISCV_PLT32] = accumulation_not_supported,
+ [R_RISCV_SET_ULEB128] = apply_uleb128_accumulation,
+ [R_RISCV_SUB_ULEB128] = apply_uleb128_accumulation,
+ /* 62-191 reserved for future standard use */
+ /* 192-255 nonstandard ABI extensions */
};

+void process_accumulated_relocations(struct module *me)
+{
+ /*
+ * Only ADD/SUB/SET/ULEB128 should end up here.
+ *
+ * Each bucket may have more than one relocation location. All
+ * relocations for a location are stored in a list in a bucket.
+ *
+ * Relocations are applied to a temp variable before being stored to the
+ * provided location to check for overflow. This also allows ULEB128 to
+ * properly decide how many entries are needed before storing to
+ * location. The final value is stored into location using the handler
+ * for the last relocation to an address.
+ *
+ * Three layers of indexing:
+ * - Each of the buckets in use
+ * - Groups of relocations in each bucket by location address
+ * - Each relocation entry for a location address
+ */
+ struct used_bucket *bucket_iter;
+ struct relocation_head *rel_head_iter;
+ struct relocation_entry *rel_entry_iter;
+ int curr_type;
+ void *location;
+ long buffer;
+
+ list_for_each_entry(bucket_iter, &used_buckets_list, head) {
+ hlist_for_each_entry(rel_head_iter, bucket_iter->bucket, node) {
+ buffer = 0;
+ location = rel_head_iter->location;
+ list_for_each_entry(rel_entry_iter, rel_head_iter->rel_entry, head) {
+ curr_type = rel_entry_iter->type;
+ reloc_handlers_rela[curr_type](me, &buffer, rel_entry_iter->value);
+ kfree(rel_entry_iter);
+ }
+ accumulate_handlers[curr_type](me, location, buffer);
+ kfree(rel_head_iter);
+ }
+ kfree(bucket_iter);
+ }
+
+ kfree(relocation_hashtable);
+}
+
+int add_relocation_to_accumulate(struct module *me, int type, void *location,
+ unsigned int hashtable_bits, Elf_Addr v)
+{
+ struct relocation_entry *entry;
+ struct relocation_head *rel_head;
+ struct hlist_head *current_head;
+ struct hlist_node *first;
+ struct used_bucket *bucket;
+ unsigned long hash;
+
+ entry = kmalloc(sizeof(*entry), GFP_KERNEL);
+ INIT_LIST_HEAD(&entry->head);
+ entry->type = type;
+ entry->value = v;
+
+ hash = hash_min((unsigned long)location, hashtable_bits);
+
+ current_head = &relocation_hashtable[hash];
+ first = current_head->first;
+
+ /* Find matching location (if any) */
+ bool found = false;
+ struct relocation_head *rel_head_iter;
+
+ hlist_for_each_entry(rel_head_iter, current_head, node) {
+ if (rel_head_iter->location == location) {
+ found = true;
+ rel_head = rel_head_iter;
+ break;
+ }
+ }
+
+ if (!found) {
+ rel_head = kmalloc(sizeof(*rel_head), GFP_KERNEL);
+ rel_head->rel_entry =
+ kmalloc(sizeof(struct list_head), GFP_KERNEL);
+ INIT_LIST_HEAD(rel_head->rel_entry);
+ rel_head->location = location;
+ INIT_HLIST_NODE(&rel_head->node);
+ if (!current_head->first) {
+ bucket =
+ kmalloc(sizeof(struct used_bucket), GFP_KERNEL);
+ INIT_LIST_HEAD(&bucket->head);
+ bucket->bucket = current_head;
+ list_add(&bucket->head, &used_buckets_list);
+ }
+ hlist_add_head(&rel_head->node, current_head);
+ }
+
+ /* Add relocation to head of discovered rel_head */
+ list_add_tail(&entry->head, rel_head->rel_entry);
+
+ return 0;
+}
+
+unsigned int initialize_relocation_hashtable(unsigned int num_relocations)
+{
+ /* Can safely assume that bits is not greater than sizeof(long) */
+ unsigned long hashtable_size = roundup_pow_of_two(num_relocations);
+ unsigned int hashtable_bits = ilog2(hashtable_size);
+
+ /*
+ * Double size of hashtable if num_relocations * 1.25 is greater than
+ * hashtable_size.
+ */
+ int should_double_size = ((num_relocations + (num_relocations >> 2)) > (hashtable_size));
+
+ hashtable_bits += should_double_size;
+
+ hashtable_size <<= should_double_size;
+
+ relocation_hashtable = kmalloc_array(hashtable_size,
+ sizeof(*relocation_hashtable),
+ GFP_KERNEL);
+ __hash_init(relocation_hashtable, hashtable_size);
+
+ INIT_LIST_HEAD(&used_buckets_list);
+
+ return hashtable_bits;
+}
+
int apply_relocate_add(Elf_Shdr *sechdrs, const char *strtab,
unsigned int symindex, unsigned int relsec,
struct module *me)
@@ -349,11 +822,13 @@ int apply_relocate_add(Elf_Shdr *sechdrs, const char *strtab,
unsigned int i, type;
Elf_Addr v;
int res;
+ unsigned int num_relocations = sechdrs[relsec].sh_size / sizeof(*rel);
+ unsigned int hashtable_bits = initialize_relocation_hashtable(num_relocations);

pr_debug("Applying relocate section %u to %u\n", relsec,
sechdrs[relsec].sh_info);

- for (i = 0; i < sechdrs[relsec].sh_size / sizeof(*rel); i++) {
+ for (i = 0; i < num_relocations; i++) {
/* This is where to make the change */
location = (void *)sechdrs[sechdrs[relsec].sh_info].sh_addr
+ rel[i].r_offset;
@@ -428,11 +903,16 @@ int apply_relocate_add(Elf_Shdr *sechdrs, const char *strtab,
}
}

- res = handler(me, location, v);
+ if (accumulate_relocations[type])
+ res = add_relocation_to_accumulate(me, type, location, hashtable_bits, v);
+ else
+ res = handler(me, location, v);
if (res)
return res;
}

+ process_accumulated_relocations(me);
+
return 0;
}


--
2.34.1

2023-10-31 07:26:42

by Charlie Jenkins

[permalink] [raw]
Subject: [PATCH v7 3/3] riscv: Add tests for riscv module loading

Add test cases for the two main groups of relocations added: SUB and
SET, along with uleb128.

Signed-off-by: Charlie Jenkins <[email protected]>
---
arch/riscv/Kconfig.debug | 1 +
arch/riscv/kernel/Makefile | 1 +
arch/riscv/kernel/tests/Kconfig.debug | 35 +++++++++
arch/riscv/kernel/tests/Makefile | 1 +
arch/riscv/kernel/tests/module_test/Makefile | 15 ++++
.../tests/module_test/test_module_linking_main.c | 88 ++++++++++++++++++++++
arch/riscv/kernel/tests/module_test/test_set16.S | 23 ++++++
arch/riscv/kernel/tests/module_test/test_set32.S | 20 +++++
arch/riscv/kernel/tests/module_test/test_set6.S | 23 ++++++
arch/riscv/kernel/tests/module_test/test_set8.S | 23 ++++++
arch/riscv/kernel/tests/module_test/test_sub16.S | 22 ++++++
arch/riscv/kernel/tests/module_test/test_sub32.S | 22 ++++++
arch/riscv/kernel/tests/module_test/test_sub6.S | 22 ++++++
arch/riscv/kernel/tests/module_test/test_sub64.S | 27 +++++++
arch/riscv/kernel/tests/module_test/test_sub8.S | 22 ++++++
arch/riscv/kernel/tests/module_test/test_uleb128.S | 31 ++++++++
16 files changed, 376 insertions(+)

diff --git a/arch/riscv/Kconfig.debug b/arch/riscv/Kconfig.debug
index e69de29bb2d1..eafe17ebf710 100644
--- a/arch/riscv/Kconfig.debug
+++ b/arch/riscv/Kconfig.debug
@@ -0,0 +1 @@
+source "arch/riscv/kernel/tests/Kconfig.debug"
diff --git a/arch/riscv/kernel/Makefile b/arch/riscv/kernel/Makefile
index 95cf25d48405..bb99657252f4 100644
--- a/arch/riscv/kernel/Makefile
+++ b/arch/riscv/kernel/Makefile
@@ -57,6 +57,7 @@ obj-y += stacktrace.o
obj-y += cacheinfo.o
obj-y += patch.o
obj-y += probes/
+obj-y += tests/
obj-$(CONFIG_MMU) += vdso.o vdso/

obj-$(CONFIG_RISCV_M_MODE) += traps_misaligned.o
diff --git a/arch/riscv/kernel/tests/Kconfig.debug b/arch/riscv/kernel/tests/Kconfig.debug
new file mode 100644
index 000000000000..5dba64e8e977
--- /dev/null
+++ b/arch/riscv/kernel/tests/Kconfig.debug
@@ -0,0 +1,35 @@
+# SPDX-License-Identifier: GPL-2.0-only
+menu "arch/riscv/kernel Testing and Coverage"
+
+config AS_HAS_ULEB128
+ def_bool $(as-instr,.reloc label$(comma) R_RISCV_SET_ULEB128$(comma) 127\n.reloc label$(comma) R_RISCV_SUB_ULEB128$(comma) 127\nlabel:\n.word 0)
+
+menuconfig RUNTIME_KERNEL_TESTING_MENU
+ bool "arch/riscv/kernel runtime Testing"
+ def_bool y
+ help
+ Enable riscv kernel runtime testing.
+
+if RUNTIME_KERNEL_TESTING_MENU
+
+config RISCV_MODULE_LINKING_KUNIT
+ bool "KUnit test riscv module linking at runtime" if !KUNIT_ALL_TESTS
+ depends on KUNIT
+ default KUNIT_ALL_TESTS
+ help
+ Enable this option to test riscv module linking at boot. This will
+ enable a module called "test_module_linking".
+
+ KUnit tests run during boot and output the results to the debug log
+ in TAP format (http://testanything.org/). Only useful for kernel devs
+ running the KUnit test harness, and not intended for inclusion into a
+ production build.
+
+ For more information on KUnit and unit tests in general please refer
+ to the KUnit documentation in Documentation/dev-tools/kunit/.
+
+ If unsure, say N.
+
+endif # RUNTIME_TESTING_MENU
+
+endmenu # "arch/riscv/kernel runtime Testing"
diff --git a/arch/riscv/kernel/tests/Makefile b/arch/riscv/kernel/tests/Makefile
new file mode 100644
index 000000000000..7d6c76cffe20
--- /dev/null
+++ b/arch/riscv/kernel/tests/Makefile
@@ -0,0 +1 @@
+obj-$(CONFIG_RISCV_MODULE_LINKING_KUNIT) += module_test/
diff --git a/arch/riscv/kernel/tests/module_test/Makefile b/arch/riscv/kernel/tests/module_test/Makefile
new file mode 100644
index 000000000000..d7a6fd8943de
--- /dev/null
+++ b/arch/riscv/kernel/tests/module_test/Makefile
@@ -0,0 +1,15 @@
+obj-m += test_module_linking.o
+
+test_sub := test_sub6.o test_sub8.o test_sub16.o test_sub32.o test_sub64.o
+
+test_set := test_set6.o test_set8.o test_set16.o test_set32.o
+
+test_module_linking-objs += $(test_sub)
+
+test_module_linking-objs += $(test_set)
+
+ifeq ($(CONFIG_AS_HAS_ULEB128),y)
+test_module_linking-objs += test_uleb128.o
+endif
+
+test_module_linking-objs += test_module_linking_main.o
diff --git a/arch/riscv/kernel/tests/module_test/test_module_linking_main.c b/arch/riscv/kernel/tests/module_test/test_module_linking_main.c
new file mode 100644
index 000000000000..8df5fa5b834e
--- /dev/null
+++ b/arch/riscv/kernel/tests/module_test/test_module_linking_main.c
@@ -0,0 +1,88 @@
+// SPDX-License-Identifier: GPL-2.0
+/*
+ * Copyright (C) 2023 Rivos Inc.
+ */
+
+#include <linux/module.h>
+#include <linux/kernel.h>
+#include <linux/init.h>
+#include <kunit/test.h>
+
+MODULE_LICENSE("GPL");
+MODULE_DESCRIPTION("Test module linking");
+
+extern int test_set32(void);
+extern int test_set16(void);
+extern int test_set8(void);
+extern int test_set6(void);
+extern long test_sub64(void);
+extern int test_sub32(void);
+extern int test_sub16(void);
+extern int test_sub8(void);
+extern int test_sub6(void);
+
+#ifdef CONFIG_AS_HAS_ULEB128
+extern int test_uleb_basic(void);
+extern int test_uleb_large(void);
+#endif
+
+#define CHECK_EQ(lhs, rhs) KUNIT_ASSERT_EQ(test, lhs, rhs)
+
+void run_test_set(struct kunit *test);
+void run_test_sub(struct kunit *test);
+void run_test_uleb(struct kunit *test);
+
+void run_test_set(struct kunit *test)
+{
+ int val32 = test_set32();
+ int val16 = test_set16();
+ int val8 = test_set8();
+ int val6 = test_set6();
+
+ CHECK_EQ(val32, 0);
+ CHECK_EQ(val16, 0);
+ CHECK_EQ(val8, 0);
+ CHECK_EQ(val6, 0);
+}
+
+void run_test_sub(struct kunit *test)
+{
+ int val64 = test_sub64();
+ int val32 = test_sub32();
+ int val16 = test_sub16();
+ int val8 = test_sub8();
+ int val6 = test_sub6();
+
+ CHECK_EQ(val64, 0);
+ CHECK_EQ(val32, 0);
+ CHECK_EQ(val16, 0);
+ CHECK_EQ(val8, 0);
+ CHECK_EQ(val6, 0);
+}
+
+#ifdef CONFIG_AS_HAS_ULEB128
+void run_test_uleb(struct kunit *test)
+{
+ int val_uleb = test_uleb_basic();
+ int val_uleb2 = test_uleb_large();
+
+ CHECK_EQ(val_uleb, 0);
+ CHECK_EQ(val_uleb2, 0);
+}
+#endif
+
+static struct kunit_case __refdata riscv_module_linking_test_cases[] = {
+ KUNIT_CASE(run_test_set),
+ KUNIT_CASE(run_test_sub),
+#ifdef CONFIG_AS_HAS_ULEB128
+ KUNIT_CASE(run_test_uleb),
+#endif
+ {}
+};
+
+static struct kunit_suite riscv_module_linking_test_suite = {
+ .name = "riscv_checksum",
+ .test_cases = riscv_module_linking_test_cases,
+};
+
+kunit_test_suites(&riscv_module_linking_test_suite);
diff --git a/arch/riscv/kernel/tests/module_test/test_set16.S b/arch/riscv/kernel/tests/module_test/test_set16.S
new file mode 100644
index 000000000000..2be0e441a12e
--- /dev/null
+++ b/arch/riscv/kernel/tests/module_test/test_set16.S
@@ -0,0 +1,23 @@
+/* SPDX-License-Identifier: GPL-2.0 */
+/*
+ * Copyright (C) 2023 Rivos Inc.
+ */
+
+.text
+.global test_set16
+test_set16:
+ lw a0, set16
+ la t0, set16
+#ifdef CONFIG_32BIT
+ slli t0, t0, 16
+ srli t0, t0, 16
+#else
+ slli t0, t0, 48
+ srli t0, t0, 48
+#endif
+ sub a0, a0, t0
+ ret
+.data
+set16:
+ .reloc set16, R_RISCV_SET16, set16
+ .word 0
diff --git a/arch/riscv/kernel/tests/module_test/test_set32.S b/arch/riscv/kernel/tests/module_test/test_set32.S
new file mode 100644
index 000000000000..de0444537e67
--- /dev/null
+++ b/arch/riscv/kernel/tests/module_test/test_set32.S
@@ -0,0 +1,20 @@
+/* SPDX-License-Identifier: GPL-2.0 */
+/*
+ * Copyright (C) 2023 Rivos Inc.
+ */
+
+.text
+.global test_set32
+test_set32:
+ lw a0, set32
+ la t0, set32
+#ifndef CONFIG_32BIT
+ slli t0, t0, 32
+ srli t0, t0, 32
+#endif
+ sub a0, a0, t0
+ ret
+.data
+set32:
+ .reloc set32, R_RISCV_SET32, set32
+ .word 0
diff --git a/arch/riscv/kernel/tests/module_test/test_set6.S b/arch/riscv/kernel/tests/module_test/test_set6.S
new file mode 100644
index 000000000000..c39ce4c219eb
--- /dev/null
+++ b/arch/riscv/kernel/tests/module_test/test_set6.S
@@ -0,0 +1,23 @@
+/* SPDX-License-Identifier: GPL-2.0 */
+/*
+ * Copyright (C) 2023 Rivos Inc.
+ */
+
+.text
+.global test_set6
+test_set6:
+ lw a0, set6
+ la t0, set6
+#ifdef CONFIG_32BIT
+ slli t0, t0, 26
+ srli t0, t0, 26
+#else
+ slli t0, t0, 58
+ srli t0, t0, 58
+#endif
+ sub a0, a0, t0
+ ret
+.data
+set6:
+ .reloc set6, R_RISCV_SET6, set6
+ .word 0
diff --git a/arch/riscv/kernel/tests/module_test/test_set8.S b/arch/riscv/kernel/tests/module_test/test_set8.S
new file mode 100644
index 000000000000..a656173f6f99
--- /dev/null
+++ b/arch/riscv/kernel/tests/module_test/test_set8.S
@@ -0,0 +1,23 @@
+/* SPDX-License-Identifier: GPL-2.0 */
+/*
+ * Copyright (C) 2023 Rivos Inc.
+ */
+
+.text
+.global test_set8
+test_set8:
+ lw a0, set8
+ la t0, set8
+#ifdef CONFIG_32BIT
+ slli t0, t0, 24
+ srli t0, t0, 24
+#else
+ slli t0, t0, 56
+ srli t0, t0, 56
+#endif
+ sub a0, a0, t0
+ ret
+.data
+set8:
+ .reloc set8, R_RISCV_SET8, set8
+ .word 0
diff --git a/arch/riscv/kernel/tests/module_test/test_sub16.S b/arch/riscv/kernel/tests/module_test/test_sub16.S
new file mode 100644
index 000000000000..c561e155d1db
--- /dev/null
+++ b/arch/riscv/kernel/tests/module_test/test_sub16.S
@@ -0,0 +1,22 @@
+/* SPDX-License-Identifier: GPL-2.0 */
+/*
+ * Copyright (C) 2023 Rivos Inc.
+ */
+
+.text
+.global test_sub16
+test_sub16:
+ lh a0, sub16
+ addi a0, a0, -32
+ ret
+first:
+ .rept 8
+ .word 0
+ .endr
+second:
+
+.data
+sub16:
+ .reloc sub16, R_RISCV_ADD16, second
+ .reloc sub16, R_RISCV_SUB16, first
+ .half 0
diff --git a/arch/riscv/kernel/tests/module_test/test_sub32.S b/arch/riscv/kernel/tests/module_test/test_sub32.S
new file mode 100644
index 000000000000..93232c70cae6
--- /dev/null
+++ b/arch/riscv/kernel/tests/module_test/test_sub32.S
@@ -0,0 +1,22 @@
+/* SPDX-License-Identifier: GPL-2.0 */
+/*
+ * Copyright (C) 2023 Rivos Inc.
+ */
+
+.text
+.global test_sub32
+test_sub32:
+ lw a0, sub32
+ addi a0, a0, -32
+ ret
+first:
+ .rept 8
+ .word 0
+ .endr
+second:
+
+.data
+sub32:
+ .reloc sub32, R_RISCV_ADD32, second
+ .reloc sub32, R_RISCV_SUB32, first
+ .word 0
diff --git a/arch/riscv/kernel/tests/module_test/test_sub6.S b/arch/riscv/kernel/tests/module_test/test_sub6.S
new file mode 100644
index 000000000000..d9c9526ceb62
--- /dev/null
+++ b/arch/riscv/kernel/tests/module_test/test_sub6.S
@@ -0,0 +1,22 @@
+/* SPDX-License-Identifier: GPL-2.0 */
+/*
+ * Copyright (C) 2023 Rivos Inc.
+ */
+
+.text
+.global test_sub6
+test_sub6:
+ lb a0, sub6
+ addi a0, a0, -32
+ ret
+first:
+ .rept 8
+ .word 0
+ .endr
+second:
+
+.data
+sub6:
+ .reloc sub6, R_RISCV_SET6, second
+ .reloc sub6, R_RISCV_SUB6, first
+ .byte 0
diff --git a/arch/riscv/kernel/tests/module_test/test_sub64.S b/arch/riscv/kernel/tests/module_test/test_sub64.S
new file mode 100644
index 000000000000..6d260e2a5d98
--- /dev/null
+++ b/arch/riscv/kernel/tests/module_test/test_sub64.S
@@ -0,0 +1,27 @@
+/* SPDX-License-Identifier: GPL-2.0 */
+/*
+ * Copyright (C) 2023 Rivos Inc.
+ */
+
+.text
+.global test_sub64
+test_sub64:
+#ifdef CONFIG_32BIT
+ lw a0, sub64
+#else
+ ld a0, sub64
+#endif
+ addi a0, a0, -32
+ ret
+first:
+ .rept 8
+ .word 0
+ .endr
+second:
+
+.data
+sub64:
+ .reloc sub64, R_RISCV_ADD64, second
+ .reloc sub64, R_RISCV_SUB64, first
+ .word 0
+ .word 0
diff --git a/arch/riscv/kernel/tests/module_test/test_sub8.S b/arch/riscv/kernel/tests/module_test/test_sub8.S
new file mode 100644
index 000000000000..af7849115d4d
--- /dev/null
+++ b/arch/riscv/kernel/tests/module_test/test_sub8.S
@@ -0,0 +1,22 @@
+/* SPDX-License-Identifier: GPL-2.0 */
+/*
+ * Copyright (C) 2023 Rivos Inc.
+ */
+
+.text
+.global test_sub8
+test_sub8:
+ lb a0, sub8
+ addi a0, a0, -32
+ ret
+first:
+ .rept 8
+ .word 0
+ .endr
+second:
+
+.data
+sub8:
+ .reloc sub8, R_RISCV_ADD8, second
+ .reloc sub8, R_RISCV_SUB8, first
+ .byte 0
diff --git a/arch/riscv/kernel/tests/module_test/test_uleb128.S b/arch/riscv/kernel/tests/module_test/test_uleb128.S
new file mode 100644
index 000000000000..51e23808136c
--- /dev/null
+++ b/arch/riscv/kernel/tests/module_test/test_uleb128.S
@@ -0,0 +1,31 @@
+/* SPDX-License-Identifier: GPL-2.0 */
+/*
+ * Copyright (C) 2023 Rivos Inc.
+ */
+
+.text
+.global test_uleb
+test_uleb:
+ ld a0, second
+ addi a0, a0, -127
+ ret
+
+.global test_uleb_large
+test_uleb_large:
+ ld a0, fourth
+ addi a0, a0, 0x07e8
+ ret
+
+.data
+first:
+ .rept 127
+ .byte 0
+ .endr
+second:
+ .reloc second, R_RISCV_SET_ULEB128, second
+ .reloc second, R_RISCV_SUB_ULEB128, first
+ .dword 0
+third:
+ .space 1000
+fourth:
+ .uleb128 fourth - third

--
2.34.1

2023-10-31 13:12:14

by Emil Renner Berthing

[permalink] [raw]
Subject: Re: [PATCH v7 1/3] riscv: Avoid unaligned access when relocating modules

Charlie Jenkins wrote:
> From: Emil Renner Berthing <[email protected]>
>
> With the C-extension regular 32bit instructions are not
> necessarily aligned on 4-byte boundaries. RISC-V instructions
> are in fact an ordered list of 16bit little-endian
> "parcels", so access the instruction as such.
>
> This should also make the code work in case someone builds
> a big-endian RISC-V machine.
>
> Signed-off-by: Emil Renner Berthing <[email protected]>
> Signed-off-by: Charlie Jenkins <[email protected]>
> ---
> arch/riscv/kernel/module.c | 153 +++++++++++++++++++++++----------------------
> 1 file changed, 77 insertions(+), 76 deletions(-)
>
> diff --git a/arch/riscv/kernel/module.c b/arch/riscv/kernel/module.c
> index 7c651d55fcbd..a9e94e939cb5 100644
> --- a/arch/riscv/kernel/module.c
> +++ b/arch/riscv/kernel/module.c
> @@ -27,68 +27,86 @@ static bool riscv_insn_valid_32bit_offset(ptrdiff_t val)
> #endif
> }
>
> -static int apply_r_riscv_32_rela(struct module *me, u32 *location, Elf_Addr v)
> +static int riscv_insn_rmw(void *location, u32 keep, u32 set)
> +{
> + u16 *parcel = location;
> + u32 insn = (u32)le16_to_cpu(parcel[0]) | (u32)le16_to_cpu(parcel[1]) << 16;
> +
> + insn &= keep;
> + insn |= set;
> +
> + parcel[0] = cpu_to_le32(insn);

Why cpu_to_le32(insn)? Unless I've misunderstood something downcasting unsigned
to unsigned values in C (eg. from u32 to u16) is defined to always discard the
most signifcant bits, so cpu_to_le16(insn) should be fine.

> + parcel[1] = cpu_to_le16(insn >> 16);
> + return 0;
> +}
> +
> +static int riscv_insn_rvc_rmw(void *location, u16 keep, u16 set)
> +{
> + u16 *parcel = location;
> +
> + *parcel = cpu_to_le16((le16_to_cpu(*parcel) & keep) | set);

In this case, maybe consider writing it out like above just so it's easy to see
that the two functions does the same just for differently sized instructions.
The compiler should generate the same code.

> + return 0;
> +}
> +
> ...

2023-10-31 13:43:49

by Emil Renner Berthing

[permalink] [raw]
Subject: Re: [PATCH v7 2/3] riscv: Add remaining module relocations

Charlie Jenkins wrote:
> Add all final module relocations and add error logs explaining the ones
> that are not supported. Implement overflow checks for
> ADD/SUB/SET/ULEB128 relocations.
>
> Signed-off-by: Charlie Jenkins <[email protected]>
> ---
> arch/riscv/include/uapi/asm/elf.h | 5 +-
> arch/riscv/kernel/module.c | 534 ++++++++++++++++++++++++++++++++++++--
> 2 files changed, 511 insertions(+), 28 deletions(-)
>
> diff --git a/arch/riscv/include/uapi/asm/elf.h b/arch/riscv/include/uapi/asm/elf.h
> index d696d6610231..11a71b8533d5 100644
> --- a/arch/riscv/include/uapi/asm/elf.h
> +++ b/arch/riscv/include/uapi/asm/elf.h
> @@ -49,6 +49,7 @@ typedef union __riscv_fp_state elf_fpregset_t;
> #define R_RISCV_TLS_DTPREL64 9
> #define R_RISCV_TLS_TPREL32 10
> #define R_RISCV_TLS_TPREL64 11
> +#define R_RISCV_IRELATIVE 58
>
> /* Relocation types not used by the dynamic linker */
> #define R_RISCV_BRANCH 16
> @@ -81,7 +82,6 @@ typedef union __riscv_fp_state elf_fpregset_t;
> #define R_RISCV_ALIGN 43
> #define R_RISCV_RVC_BRANCH 44
> #define R_RISCV_RVC_JUMP 45
> -#define R_RISCV_LUI 46
> #define R_RISCV_GPREL_I 47
> #define R_RISCV_GPREL_S 48
> #define R_RISCV_TPREL_I 49
> @@ -93,6 +93,9 @@ typedef union __riscv_fp_state elf_fpregset_t;
> #define R_RISCV_SET16 55
> #define R_RISCV_SET32 56
> #define R_RISCV_32_PCREL 57
> +#define R_RISCV_PLT32 59
> +#define R_RISCV_SET_ULEB128 60
> +#define R_RISCV_SUB_ULEB128 61
>
>
> #endif /* _UAPI_ASM_RISCV_ELF_H */
> diff --git a/arch/riscv/kernel/module.c b/arch/riscv/kernel/module.c
> index a9e94e939cb5..230172ecb26e 100644
> --- a/arch/riscv/kernel/module.c
> +++ b/arch/riscv/kernel/module.c
> @@ -7,6 +7,9 @@
> #include <linux/elf.h>
> #include <linux/err.h>
> #include <linux/errno.h>
> +#include <linux/hashtable.h>
> +#include <linux/kernel.h>
> +#include <linux/log2.h>
> #include <linux/moduleloader.h>
> #include <linux/vmalloc.h>
> #include <linux/sizes.h>
> @@ -14,6 +17,27 @@
> #include <asm/alternative.h>
> #include <asm/sections.h>
>
> +struct used_bucket {
> + struct list_head head;
> + struct hlist_head *bucket;
> +};
> +
> +struct relocation_head {
> + struct hlist_node node;
> + struct list_head *rel_entry;
> + void *location;
> +};
> +
> +struct relocation_entry {
> + struct list_head head;
> + Elf_Addr value;
> + unsigned int type;
> +};
> +
> +struct hlist_head *relocation_hashtable;
> +
> +struct list_head used_buckets_list;
> +
> /*
> * The auipc+jalr instruction pair can reach any PC-relative offset
> * in the range [-2^31 - 2^11, 2^31 - 2^11)
> @@ -269,6 +293,12 @@ static int apply_r_riscv_align_rela(struct module *me, void *location,
> return -EINVAL;
> }
>
> +static int apply_r_riscv_add8_rela(struct module *me, void *location, Elf_Addr v)
> +{
> + *(u8 *)location += (u8)v;
> + return 0;
> +}
> +
> static int apply_r_riscv_add16_rela(struct module *me, void *location,
> Elf_Addr v)
> {
> @@ -290,6 +320,12 @@ static int apply_r_riscv_add64_rela(struct module *me, void *location,
> return 0;
> }
>
> +static int apply_r_riscv_sub8_rela(struct module *me, void *location, Elf_Addr v)
> +{
> + *(u8 *)location -= (u8)v;
> + return 0;
> +}
> +
> static int apply_r_riscv_sub16_rela(struct module *me, void *location,
> Elf_Addr v)
> {
> @@ -311,33 +347,470 @@ static int apply_r_riscv_sub64_rela(struct module *me, void *location,
> return 0;
> }
>
> -static int (*reloc_handlers_rela[]) (struct module *me, void *location,
> - Elf_Addr v) = {
> - [R_RISCV_32] = apply_r_riscv_32_rela,
> - [R_RISCV_64] = apply_r_riscv_64_rela,
> - [R_RISCV_BRANCH] = apply_r_riscv_branch_rela,
> - [R_RISCV_JAL] = apply_r_riscv_jal_rela,
> - [R_RISCV_RVC_BRANCH] = apply_r_riscv_rvc_branch_rela,
> - [R_RISCV_RVC_JUMP] = apply_r_riscv_rvc_jump_rela,
> - [R_RISCV_PCREL_HI20] = apply_r_riscv_pcrel_hi20_rela,
> - [R_RISCV_PCREL_LO12_I] = apply_r_riscv_pcrel_lo12_i_rela,
> - [R_RISCV_PCREL_LO12_S] = apply_r_riscv_pcrel_lo12_s_rela,
> - [R_RISCV_HI20] = apply_r_riscv_hi20_rela,
> - [R_RISCV_LO12_I] = apply_r_riscv_lo12_i_rela,
> - [R_RISCV_LO12_S] = apply_r_riscv_lo12_s_rela,
> - [R_RISCV_GOT_HI20] = apply_r_riscv_got_hi20_rela,
> - [R_RISCV_CALL_PLT] = apply_r_riscv_call_plt_rela,
> - [R_RISCV_CALL] = apply_r_riscv_call_rela,
> - [R_RISCV_RELAX] = apply_r_riscv_relax_rela,
> - [R_RISCV_ALIGN] = apply_r_riscv_align_rela,
> - [R_RISCV_ADD16] = apply_r_riscv_add16_rela,
> - [R_RISCV_ADD32] = apply_r_riscv_add32_rela,
> - [R_RISCV_ADD64] = apply_r_riscv_add64_rela,
> - [R_RISCV_SUB16] = apply_r_riscv_sub16_rela,
> - [R_RISCV_SUB32] = apply_r_riscv_sub32_rela,
> - [R_RISCV_SUB64] = apply_r_riscv_sub64_rela,
> +static int dynamic_linking_not_supported(struct module *me, void *location,
> + Elf_Addr v)
> +{
> + pr_err("%s: Dynamic linking not supported in kernel modules PC = %p\n",
> + me->name, location);
> + return -EINVAL;
> +}
> +
> +static int tls_not_supported(struct module *me, void *location, Elf_Addr v)
> +{
> + pr_err("%s: Thread local storage not supported in kernel modules PC = %p\n",
> + me->name, location);
> + return -EINVAL;
> +}
> +
> +static int apply_r_riscv_sub6_rela(struct module *me, void *location, Elf_Addr v)
> +{
> + *(u8 *)location = (*(u8 *)location - ((u8)v & 0x3F)) & 0x3F;
> + return 0;
> +}
> +
> +static int apply_r_riscv_set6_rela(struct module *me, void *location, Elf_Addr v)
> +{
> + *(u8 *)location = ((*(u8 *)location & 0xc0) | ((u8)v & 0x3F));

Most of these casts are simple enough for here and above I'd consider
something like this for readability:

u8 *byte = location;
u8 value = v;

*byte = (*byte & 0xc0) | (value & 0x3f)

The compiler should generate the same code.

> + return 0;
> +}
> +
> +static int apply_r_riscv_set8_rela(struct module *me, void *location, Elf_Addr v)
> +{
> + *(u8 *)location = (u8)v;
> + return 0;
> +}
> +
> +static int apply_r_riscv_set16_rela(struct module *me, void *location,
> + Elf_Addr v)
> +{
> + *(u16 *)location = (u16)v;
> + return 0;
> +}
> +
> +static int apply_r_riscv_set32_rela(struct module *me, void *location,
> + Elf_Addr v)
> +{
> + *(u32 *)location = (u32)v;
> + return 0;
> +}
> +
> +static int apply_r_riscv_32_pcrel_rela(struct module *me, void *location,
> + Elf_Addr v)
> +{
> + *(u32 *)location = v - (unsigned long)location;
> + return 0;
> +}
> +
> +static int apply_r_riscv_plt32_rela(struct module *me, void *location,
> + Elf_Addr v)
> +{
> + ptrdiff_t offset = (void *)v - location;
> +
> + if (!riscv_insn_valid_32bit_offset(offset)) {
> + /* Only emit the plt entry if offset over 32-bit range */
> + if (IS_ENABLED(CONFIG_MODULE_SECTIONS)) {
> + offset = (void *)module_emit_plt_entry(me, v) - location;
> + } else {
> + pr_err("%s: target %016llx can not be addressed by the 32-bit offset from PC = %p\n",
> + me->name, (long long)v, location);
> + return -EINVAL;
> + }
> + }
> +
> + *(u32 *)location = (u32)offset;
> + return 0;
> +}
> +
> +static int apply_r_riscv_set_uleb128(struct module *me, void *location, Elf_Addr v)
> +{
> + *(long *)location = v;
> + return 0;
> +}
> +
> +static int apply_r_riscv_sub_uleb128(struct module *me, void *location, Elf_Addr v)
> +{
> + *(long *)location -= v;
> + return 0;
> +}
> +
> +/*
> + * Relocations defined in the riscv-elf-psabi-doc.
> + * This handles static linking only.
> + */
> +static int (*reloc_handlers_rela[])(struct module *me, void *location,
> + Elf_Addr v) = {
> + [R_RISCV_32] = apply_r_riscv_32_rela,
> + [R_RISCV_64] = apply_r_riscv_64_rela,
> + [R_RISCV_RELATIVE] = dynamic_linking_not_supported,
> + [R_RISCV_COPY] = dynamic_linking_not_supported,
> + [R_RISCV_JUMP_SLOT] = dynamic_linking_not_supported,
> + [R_RISCV_TLS_DTPMOD32] = dynamic_linking_not_supported,
> + [R_RISCV_TLS_DTPMOD64] = dynamic_linking_not_supported,
> + [R_RISCV_TLS_DTPREL32] = dynamic_linking_not_supported,
> + [R_RISCV_TLS_DTPREL64] = dynamic_linking_not_supported,
> + [R_RISCV_TLS_TPREL32] = dynamic_linking_not_supported,
> + [R_RISCV_TLS_TPREL64] = dynamic_linking_not_supported,
> + /* 12-15 undefined */
> + [R_RISCV_BRANCH] = apply_r_riscv_branch_rela,
> + [R_RISCV_JAL] = apply_r_riscv_jal_rela,
> + [R_RISCV_CALL] = apply_r_riscv_call_rela,
> + [R_RISCV_CALL_PLT] = apply_r_riscv_call_plt_rela,
> + [R_RISCV_GOT_HI20] = apply_r_riscv_got_hi20_rela,
> + [R_RISCV_TLS_GOT_HI20] = tls_not_supported,
> + [R_RISCV_TLS_GD_HI20] = tls_not_supported,
> + [R_RISCV_PCREL_HI20] = apply_r_riscv_pcrel_hi20_rela,
> + [R_RISCV_PCREL_LO12_I] = apply_r_riscv_pcrel_lo12_i_rela,
> + [R_RISCV_PCREL_LO12_S] = apply_r_riscv_pcrel_lo12_s_rela,
> + [R_RISCV_HI20] = apply_r_riscv_hi20_rela,
> + [R_RISCV_LO12_I] = apply_r_riscv_lo12_i_rela,
> + [R_RISCV_LO12_S] = apply_r_riscv_lo12_s_rela,
> + [R_RISCV_TPREL_HI20] = tls_not_supported,
> + [R_RISCV_TPREL_LO12_I] = tls_not_supported,
> + [R_RISCV_TPREL_LO12_S] = tls_not_supported,
> + [R_RISCV_TPREL_ADD] = tls_not_supported,
> + [R_RISCV_ADD8] = apply_r_riscv_add8_rela,
> + [R_RISCV_ADD16] = apply_r_riscv_add16_rela,
> + [R_RISCV_ADD32] = apply_r_riscv_add32_rela,
> + [R_RISCV_ADD64] = apply_r_riscv_add64_rela,
> + [R_RISCV_SUB8] = apply_r_riscv_sub8_rela,
> + [R_RISCV_SUB16] = apply_r_riscv_sub16_rela,
> + [R_RISCV_SUB32] = apply_r_riscv_sub32_rela,
> + [R_RISCV_SUB64] = apply_r_riscv_sub64_rela,
> + /* 41-42 reserved for future standard use */
> + [R_RISCV_ALIGN] = apply_r_riscv_align_rela,
> + [R_RISCV_RVC_BRANCH] = apply_r_riscv_rvc_branch_rela,
> + [R_RISCV_RVC_JUMP] = apply_r_riscv_rvc_jump_rela,
> + /* 46-50 reserved for future standard use */
> + [R_RISCV_RELAX] = apply_r_riscv_relax_rela,
> + [R_RISCV_SUB6] = apply_r_riscv_sub6_rela,
> + [R_RISCV_SET6] = apply_r_riscv_set6_rela,
> + [R_RISCV_SET8] = apply_r_riscv_set8_rela,
> + [R_RISCV_SET16] = apply_r_riscv_set16_rela,
> + [R_RISCV_SET32] = apply_r_riscv_set32_rela,
> + [R_RISCV_32_PCREL] = apply_r_riscv_32_pcrel_rela,
> + [R_RISCV_IRELATIVE] = dynamic_linking_not_supported,
> + [R_RISCV_PLT32] = apply_r_riscv_plt32_rela,
> + [R_RISCV_SET_ULEB128] = apply_r_riscv_set_uleb128,
> + [R_RISCV_SUB_ULEB128] = apply_r_riscv_sub_uleb128,
> + /* 62-191 reserved for future standard use */
> + /* 192-255 nonstandard ABI extensions */
> +};
> +
> +static bool accumulate_relocations[] = {
> + [R_RISCV_32] = false,
> + [R_RISCV_64] = false,
> + [R_RISCV_RELATIVE] = false,
> + [R_RISCV_COPY] = false,
> + [R_RISCV_JUMP_SLOT] = false,
> + [R_RISCV_TLS_DTPMOD32] = false,
> + [R_RISCV_TLS_DTPMOD64] = false,
> + [R_RISCV_TLS_DTPREL32] = false,
> + [R_RISCV_TLS_DTPREL64] = false,
> + [R_RISCV_TLS_TPREL32] = false,
> + [R_RISCV_TLS_TPREL64] = false,
> + /* 12-15 undefined */
> + [R_RISCV_BRANCH] = false,
> + [R_RISCV_JAL] = false,
> + [R_RISCV_CALL] = false,
> + [R_RISCV_CALL_PLT] = false,
> + [R_RISCV_GOT_HI20] = false,
> + [R_RISCV_TLS_GOT_HI20] = false,
> + [R_RISCV_TLS_GD_HI20] = false,
> + [R_RISCV_PCREL_HI20] = false,
> + [R_RISCV_PCREL_LO12_I] = false,
> + [R_RISCV_PCREL_LO12_S] = false,
> + [R_RISCV_HI20] = false,
> + [R_RISCV_LO12_I] = false,
> + [R_RISCV_LO12_S] = false,
> + [R_RISCV_TPREL_HI20] = false,
> + [R_RISCV_TPREL_LO12_I] = false,
> + [R_RISCV_TPREL_LO12_S] = false,
> + [R_RISCV_TPREL_ADD] = false,
> + [R_RISCV_ADD8] = true,
> + [R_RISCV_ADD16] = true,
> + [R_RISCV_ADD32] = true,
> + [R_RISCV_ADD64] = true,
> + [R_RISCV_SUB8] = true,
> + [R_RISCV_SUB16] = true,
> + [R_RISCV_SUB32] = true,
> + [R_RISCV_SUB64] = true,
> + /* 41-42 reserved for future standard use */
> + [R_RISCV_ALIGN] = false,
> + [R_RISCV_RVC_BRANCH] = false,
> + [R_RISCV_RVC_JUMP] = false,
> + /* 46-50 reserved for future standard use */
> + [R_RISCV_RELAX] = false,
> + [R_RISCV_SUB6] = true,
> + [R_RISCV_SET6] = true,
> + [R_RISCV_SET8] = true,
> + [R_RISCV_SET16] = true,
> + [R_RISCV_SET32] = true,
> + [R_RISCV_32_PCREL] = false,
> + [R_RISCV_IRELATIVE] = false,
> + [R_RISCV_PLT32] = false,
> + [R_RISCV_SET_ULEB128] = true,
> + [R_RISCV_SUB_ULEB128] = true,
> + /* 62-191 reserved for future standard use */
> + /* 192-255 nonstandard ABI extensions */
> +};
> +
> +static int accumulation_not_supported(struct module *me, void *location, long buffer)
> +{
> + pr_err("%s: Internal error. Only ADD/SUB/SET/ULEB128 should be accumulated.", me->name);
> + return -EINVAL;
> +}
> +
> +static int apply_6_bit_accumulation(struct module *me, void *location, long buffer)
> +{
> + if (buffer != (buffer & 0x3F)) {

It may be a matter of taste, but I'd find something like this easier to read:

if (buffer > 0x3f) {

To me that more directly conveys that you're just checking if any if the higher
bits are set.

> + pr_err("%s: value %ld out of range for 6-bit relocation.\n",
> + me->name, buffer);
> + return -EINVAL;
> + }
> + *(u8 *)location = ((*(u8 *)location & 0xc0) | ((u8)buffer & 0x3F));

Here the casts also get a bit much.

> + return 0;
> +}
> +
> +static int apply_8_bit_accumulation(struct module *me, void *location, long buffer)
> +{
> + if (buffer != (u8)buffer) {
> + pr_err("%s: value %ld out of range for 8-bit relocation.\n",
> + me->name, buffer);
> + return -EINVAL;
> + }
> + *(u8 *)location = (u8)buffer;
> + return 0;
> +}
> +
> +static int apply_16_bit_accumulation(struct module *me, void *location, long buffer)
> +{
> + if (buffer != (u16)buffer) {

if (buffer > U16_MAX) {

> + pr_err("%s: value %ld out of range for 16-bit relocation.\n",
> + me->name, buffer);
> + return -EINVAL;
> + }
> + *(u16 *)location = (u16)buffer;
> + return 0;
> +}
> +
> +static int apply_32_bit_accumulation(struct module *me, void *location, long buffer)
> +{
> + if (buffer != (u32)buffer) {

if (buffer > U32_MAX) {

> + pr_err("%s: value %ld out of range for 32-bit relocation.\n",
> + me->name, buffer);
> + return -EINVAL;
> + }
> + *(u32 *)location = (u32)buffer;
> + return 0;
> +}
> +
> +static int apply_64_bit_accumulation(struct module *me, void *location, long buffer)
> +{
> + *(u64 *)location = (u64)buffer;
> + return 0;
> +}
> +
> +static int apply_uleb128_accumulation(struct module *me, void *location, long buffer)
> +{
> + /*
> + * ULEB128 is a variable length encoding. Encode the buffer into
> + * the ULEB128 data format.
> + */
> + while (buffer != 0) {
> + *(u8 *)location = (u8)buffer & 0x7F;
> + buffer >>= 7;
> + *(u8 *)location |= (buffer != 0) << 7;
> + location = (u8 *)location + 1;
> + }

How about something like

u8 *p = location;

while (buffer) {
u8 value = buffer & 0x7f;

buffer >>= 7;
value |= (!!buffer) << 7;

*p++ = value;
}


This sould do the same as above, but if buffer == 0 it doesn't write anything.
Is that the right thing to do?

> + return 0;
> +}
> +
> +/*
> + * Need to duplicate this a third time to capture the handlers for accumulation.
> + */
> +static int (*accumulate_handlers[])(struct module *me, void *location, long buffer) = {
> + [R_RISCV_32] = accumulation_not_supported,
> + [R_RISCV_64] = accumulation_not_supported,
> + [R_RISCV_RELATIVE] = accumulation_not_supported,
> + [R_RISCV_COPY] = accumulation_not_supported,
> + [R_RISCV_JUMP_SLOT] = accumulation_not_supported,
> + [R_RISCV_TLS_DTPMOD32] = accumulation_not_supported,
> + [R_RISCV_TLS_DTPMOD64] = accumulation_not_supported,
> + [R_RISCV_TLS_DTPREL32] = accumulation_not_supported,
> + [R_RISCV_TLS_DTPREL64] = accumulation_not_supported,
> + [R_RISCV_TLS_TPREL32] = accumulation_not_supported,
> + [R_RISCV_TLS_TPREL64] = accumulation_not_supported,
> + /* 12-15 undefined */
> + [R_RISCV_BRANCH] = accumulation_not_supported,
> + [R_RISCV_JAL] = accumulation_not_supported,
> + [R_RISCV_CALL] = accumulation_not_supported,
> + [R_RISCV_CALL_PLT] = accumulation_not_supported,
> + [R_RISCV_GOT_HI20] = accumulation_not_supported,
> + [R_RISCV_TLS_GOT_HI20] = accumulation_not_supported,
> + [R_RISCV_TLS_GD_HI20] = accumulation_not_supported,
> + [R_RISCV_PCREL_HI20] = accumulation_not_supported,
> + [R_RISCV_PCREL_LO12_I] = accumulation_not_supported,
> + [R_RISCV_PCREL_LO12_S] = accumulation_not_supported,
> + [R_RISCV_HI20] = accumulation_not_supported,
> + [R_RISCV_LO12_I] = accumulation_not_supported,
> + [R_RISCV_LO12_S] = accumulation_not_supported,
> + [R_RISCV_TPREL_HI20] = accumulation_not_supported,
> + [R_RISCV_TPREL_LO12_I] = accumulation_not_supported,
> + [R_RISCV_TPREL_LO12_S] = accumulation_not_supported,
> + [R_RISCV_TPREL_ADD] = accumulation_not_supported,
> + [R_RISCV_ADD8] = apply_8_bit_accumulation,
> + [R_RISCV_ADD16] = apply_16_bit_accumulation,
> + [R_RISCV_ADD32] = apply_32_bit_accumulation,
> + [R_RISCV_ADD64] = apply_64_bit_accumulation,
> + [R_RISCV_SUB8] = apply_8_bit_accumulation,
> + [R_RISCV_SUB16] = apply_16_bit_accumulation,
> + [R_RISCV_SUB32] = apply_32_bit_accumulation,
> + [R_RISCV_SUB64] = apply_64_bit_accumulation,
> + /* 41-42 reserved for future standard use */
> + [R_RISCV_ALIGN] = accumulation_not_supported,
> + [R_RISCV_RVC_BRANCH] = accumulation_not_supported,
> + [R_RISCV_RVC_JUMP] = accumulation_not_supported,
> + /* 46-50 reserved for future standard use */
> + [R_RISCV_RELAX] = accumulation_not_supported,
> + [R_RISCV_SUB6] = apply_6_bit_accumulation,
> + [R_RISCV_SET6] = apply_6_bit_accumulation,
> + [R_RISCV_SET8] = apply_8_bit_accumulation,
> + [R_RISCV_SET16] = apply_16_bit_accumulation,
> + [R_RISCV_SET32] = apply_32_bit_accumulation,
> + [R_RISCV_32_PCREL] = accumulation_not_supported,
> + [R_RISCV_IRELATIVE] = accumulation_not_supported,
> + [R_RISCV_PLT32] = accumulation_not_supported,
> + [R_RISCV_SET_ULEB128] = apply_uleb128_accumulation,
> + [R_RISCV_SUB_ULEB128] = apply_uleb128_accumulation,
> + /* 62-191 reserved for future standard use */
> + /* 192-255 nonstandard ABI extensions */
> };
>
> +void process_accumulated_relocations(struct module *me)
> +{
> + /*
> + * Only ADD/SUB/SET/ULEB128 should end up here.
> + *
> + * Each bucket may have more than one relocation location. All
> + * relocations for a location are stored in a list in a bucket.
> + *
> + * Relocations are applied to a temp variable before being stored to the
> + * provided location to check for overflow. This also allows ULEB128 to
> + * properly decide how many entries are needed before storing to
> + * location. The final value is stored into location using the handler
> + * for the last relocation to an address.
> + *
> + * Three layers of indexing:
> + * - Each of the buckets in use
> + * - Groups of relocations in each bucket by location address
> + * - Each relocation entry for a location address
> + */
> + struct used_bucket *bucket_iter;
> + struct relocation_head *rel_head_iter;
> + struct relocation_entry *rel_entry_iter;
> + int curr_type;
> + void *location;
> + long buffer;
> +
> + list_for_each_entry(bucket_iter, &used_buckets_list, head) {
> + hlist_for_each_entry(rel_head_iter, bucket_iter->bucket, node) {
> + buffer = 0;
> + location = rel_head_iter->location;
> + list_for_each_entry(rel_entry_iter, rel_head_iter->rel_entry, head) {
> + curr_type = rel_entry_iter->type;
> + reloc_handlers_rela[curr_type](me, &buffer, rel_entry_iter->value);
> + kfree(rel_entry_iter);
> + }
> + accumulate_handlers[curr_type](me, location, buffer);
> + kfree(rel_head_iter);
> + }
> + kfree(bucket_iter);
> + }
> +
> + kfree(relocation_hashtable);
> +}
> +
> +int add_relocation_to_accumulate(struct module *me, int type, void *location,
> + unsigned int hashtable_bits, Elf_Addr v)
> +{
> + struct relocation_entry *entry;
> + struct relocation_head *rel_head;
> + struct hlist_head *current_head;
> + struct hlist_node *first;
> + struct used_bucket *bucket;
> + unsigned long hash;
> +
> + entry = kmalloc(sizeof(*entry), GFP_KERNEL);
> + INIT_LIST_HEAD(&entry->head);
> + entry->type = type;
> + entry->value = v;
> +
> + hash = hash_min((unsigned long)location, hashtable_bits);
> +
> + current_head = &relocation_hashtable[hash];
> + first = current_head->first;
> +
> + /* Find matching location (if any) */
> + bool found = false;
> + struct relocation_head *rel_head_iter;
> +
> + hlist_for_each_entry(rel_head_iter, current_head, node) {
> + if (rel_head_iter->location == location) {
> + found = true;
> + rel_head = rel_head_iter;
> + break;
> + }
> + }
> +
> + if (!found) {
> + rel_head = kmalloc(sizeof(*rel_head), GFP_KERNEL);
> + rel_head->rel_entry =
> + kmalloc(sizeof(struct list_head), GFP_KERNEL);
> + INIT_LIST_HEAD(rel_head->rel_entry);
> + rel_head->location = location;
> + INIT_HLIST_NODE(&rel_head->node);
> + if (!current_head->first) {
> + bucket =
> + kmalloc(sizeof(struct used_bucket), GFP_KERNEL);
> + INIT_LIST_HEAD(&bucket->head);
> + bucket->bucket = current_head;
> + list_add(&bucket->head, &used_buckets_list);
> + }
> + hlist_add_head(&rel_head->node, current_head);
> + }
> +
> + /* Add relocation to head of discovered rel_head */
> + list_add_tail(&entry->head, rel_head->rel_entry);
> +
> + return 0;
> +}
> +
> +unsigned int initialize_relocation_hashtable(unsigned int num_relocations)
> +{
> + /* Can safely assume that bits is not greater than sizeof(long) */
> + unsigned long hashtable_size = roundup_pow_of_two(num_relocations);
> + unsigned int hashtable_bits = ilog2(hashtable_size);
> +
> + /*
> + * Double size of hashtable if num_relocations * 1.25 is greater than
> + * hashtable_size.
> + */
> + int should_double_size = ((num_relocations + (num_relocations >> 2)) > (hashtable_size));
> +
> + hashtable_bits += should_double_size;
> +
> + hashtable_size <<= should_double_size;
> +
> + relocation_hashtable = kmalloc_array(hashtable_size,
> + sizeof(*relocation_hashtable),
> + GFP_KERNEL);
> + __hash_init(relocation_hashtable, hashtable_size);
> +
> + INIT_LIST_HEAD(&used_buckets_list);
> +
> + return hashtable_bits;
> +}
> +
> int apply_relocate_add(Elf_Shdr *sechdrs, const char *strtab,
> unsigned int symindex, unsigned int relsec,
> struct module *me)
> @@ -349,11 +822,13 @@ int apply_relocate_add(Elf_Shdr *sechdrs, const char *strtab,
> unsigned int i, type;
> Elf_Addr v;
> int res;
> + unsigned int num_relocations = sechdrs[relsec].sh_size / sizeof(*rel);
> + unsigned int hashtable_bits = initialize_relocation_hashtable(num_relocations);
>
> pr_debug("Applying relocate section %u to %u\n", relsec,
> sechdrs[relsec].sh_info);
>
> - for (i = 0; i < sechdrs[relsec].sh_size / sizeof(*rel); i++) {
> + for (i = 0; i < num_relocations; i++) {
> /* This is where to make the change */
> location = (void *)sechdrs[sechdrs[relsec].sh_info].sh_addr
> + rel[i].r_offset;
> @@ -428,11 +903,16 @@ int apply_relocate_add(Elf_Shdr *sechdrs, const char *strtab,
> }
> }
>
> - res = handler(me, location, v);
> + if (accumulate_relocations[type])
> + res = add_relocation_to_accumulate(me, type, location, hashtable_bits, v);
> + else
> + res = handler(me, location, v);
> if (res)
> return res;
> }
>
> + process_accumulated_relocations(me);
> +
> return 0;
> }
>
>
> --
> 2.34.1
>
>
> _______________________________________________
> linux-riscv mailing list
> [email protected]
> http://lists.infradead.org/mailman/listinfo/linux-riscv

2023-10-31 16:46:20

by Andreas Schwab

[permalink] [raw]
Subject: Re: [PATCH v7 1/3] riscv: Avoid unaligned access when relocating modules

On Okt 31 2023, Emil Renner Berthing wrote:

>> +static int riscv_insn_rmw(void *location, u32 keep, u32 set)
>> +{
>> + u16 *parcel = location;
>> + u32 insn = (u32)le16_to_cpu(parcel[0]) | (u32)le16_to_cpu(parcel[1]) << 16;
>> +
>> + insn &= keep;
>> + insn |= set;
>> +
>> + parcel[0] = cpu_to_le32(insn);
>
> Why cpu_to_le32(insn)? Unless I've misunderstood something downcasting unsigned
> to unsigned values in C (eg. from u32 to u16) is defined to always discard the
> most signifcant bits, so cpu_to_le16(insn) should be fine.

cpu_to_le32(insn) can't be right here anyway, since it also swaps the
two u16 halves and would be the same as cpu_to_le16(insn >> 16) on big
endian.

--
Andreas Schwab, [email protected]
GPG Key fingerprint = 7578 EB47 D4E5 4D69 2510 2552 DF73 E780 A9DA AEC1
"And now for something completely different."

2023-10-31 18:19:10

by Charlie Jenkins

[permalink] [raw]
Subject: Re: [PATCH v7 2/3] riscv: Add remaining module relocations

On Tue, Oct 31, 2023 at 06:43:19AM -0700, Emil Renner Berthing wrote:
> Charlie Jenkins wrote:
> > Add all final module relocations and add error logs explaining the ones
> > that are not supported. Implement overflow checks for
> > ADD/SUB/SET/ULEB128 relocations.
> >
> > Signed-off-by: Charlie Jenkins <[email protected]>
> > ---
> > arch/riscv/include/uapi/asm/elf.h | 5 +-
> > arch/riscv/kernel/module.c | 534 ++++++++++++++++++++++++++++++++++++--
> > 2 files changed, 511 insertions(+), 28 deletions(-)
> >
> > diff --git a/arch/riscv/include/uapi/asm/elf.h b/arch/riscv/include/uapi/asm/elf.h
> > index d696d6610231..11a71b8533d5 100644
> > --- a/arch/riscv/include/uapi/asm/elf.h
> > +++ b/arch/riscv/include/uapi/asm/elf.h
> > @@ -49,6 +49,7 @@ typedef union __riscv_fp_state elf_fpregset_t;
> > #define R_RISCV_TLS_DTPREL64 9
> > #define R_RISCV_TLS_TPREL32 10
> > #define R_RISCV_TLS_TPREL64 11
> > +#define R_RISCV_IRELATIVE 58
> >
> > /* Relocation types not used by the dynamic linker */
> > #define R_RISCV_BRANCH 16
> > @@ -81,7 +82,6 @@ typedef union __riscv_fp_state elf_fpregset_t;
> > #define R_RISCV_ALIGN 43
> > #define R_RISCV_RVC_BRANCH 44
> > #define R_RISCV_RVC_JUMP 45
> > -#define R_RISCV_LUI 46
> > #define R_RISCV_GPREL_I 47
> > #define R_RISCV_GPREL_S 48
> > #define R_RISCV_TPREL_I 49
> > @@ -93,6 +93,9 @@ typedef union __riscv_fp_state elf_fpregset_t;
> > #define R_RISCV_SET16 55
> > #define R_RISCV_SET32 56
> > #define R_RISCV_32_PCREL 57
> > +#define R_RISCV_PLT32 59
> > +#define R_RISCV_SET_ULEB128 60
> > +#define R_RISCV_SUB_ULEB128 61
> >
> >
> > #endif /* _UAPI_ASM_RISCV_ELF_H */
> > diff --git a/arch/riscv/kernel/module.c b/arch/riscv/kernel/module.c
> > index a9e94e939cb5..230172ecb26e 100644
> > --- a/arch/riscv/kernel/module.c
> > +++ b/arch/riscv/kernel/module.c
> > @@ -7,6 +7,9 @@
> > #include <linux/elf.h>
> > #include <linux/err.h>
> > #include <linux/errno.h>
> > +#include <linux/hashtable.h>
> > +#include <linux/kernel.h>
> > +#include <linux/log2.h>
> > #include <linux/moduleloader.h>
> > #include <linux/vmalloc.h>
> > #include <linux/sizes.h>
> > @@ -14,6 +17,27 @@
> > #include <asm/alternative.h>
> > #include <asm/sections.h>
> >
> > +struct used_bucket {
> > + struct list_head head;
> > + struct hlist_head *bucket;
> > +};
> > +
> > +struct relocation_head {
> > + struct hlist_node node;
> > + struct list_head *rel_entry;
> > + void *location;
> > +};
> > +
> > +struct relocation_entry {
> > + struct list_head head;
> > + Elf_Addr value;
> > + unsigned int type;
> > +};
> > +
> > +struct hlist_head *relocation_hashtable;
> > +
> > +struct list_head used_buckets_list;
> > +
> > /*
> > * The auipc+jalr instruction pair can reach any PC-relative offset
> > * in the range [-2^31 - 2^11, 2^31 - 2^11)
> > @@ -269,6 +293,12 @@ static int apply_r_riscv_align_rela(struct module *me, void *location,
> > return -EINVAL;
> > }
> >
> > +static int apply_r_riscv_add8_rela(struct module *me, void *location, Elf_Addr v)
> > +{
> > + *(u8 *)location += (u8)v;
> > + return 0;
> > +}
> > +
> > static int apply_r_riscv_add16_rela(struct module *me, void *location,
> > Elf_Addr v)
> > {
> > @@ -290,6 +320,12 @@ static int apply_r_riscv_add64_rela(struct module *me, void *location,
> > return 0;
> > }
> >
> > +static int apply_r_riscv_sub8_rela(struct module *me, void *location, Elf_Addr v)
> > +{
> > + *(u8 *)location -= (u8)v;
> > + return 0;
> > +}
> > +
> > static int apply_r_riscv_sub16_rela(struct module *me, void *location,
> > Elf_Addr v)
> > {
> > @@ -311,33 +347,470 @@ static int apply_r_riscv_sub64_rela(struct module *me, void *location,
> > return 0;
> > }
> >
> > -static int (*reloc_handlers_rela[]) (struct module *me, void *location,
> > - Elf_Addr v) = {
> > - [R_RISCV_32] = apply_r_riscv_32_rela,
> > - [R_RISCV_64] = apply_r_riscv_64_rela,
> > - [R_RISCV_BRANCH] = apply_r_riscv_branch_rela,
> > - [R_RISCV_JAL] = apply_r_riscv_jal_rela,
> > - [R_RISCV_RVC_BRANCH] = apply_r_riscv_rvc_branch_rela,
> > - [R_RISCV_RVC_JUMP] = apply_r_riscv_rvc_jump_rela,
> > - [R_RISCV_PCREL_HI20] = apply_r_riscv_pcrel_hi20_rela,
> > - [R_RISCV_PCREL_LO12_I] = apply_r_riscv_pcrel_lo12_i_rela,
> > - [R_RISCV_PCREL_LO12_S] = apply_r_riscv_pcrel_lo12_s_rela,
> > - [R_RISCV_HI20] = apply_r_riscv_hi20_rela,
> > - [R_RISCV_LO12_I] = apply_r_riscv_lo12_i_rela,
> > - [R_RISCV_LO12_S] = apply_r_riscv_lo12_s_rela,
> > - [R_RISCV_GOT_HI20] = apply_r_riscv_got_hi20_rela,
> > - [R_RISCV_CALL_PLT] = apply_r_riscv_call_plt_rela,
> > - [R_RISCV_CALL] = apply_r_riscv_call_rela,
> > - [R_RISCV_RELAX] = apply_r_riscv_relax_rela,
> > - [R_RISCV_ALIGN] = apply_r_riscv_align_rela,
> > - [R_RISCV_ADD16] = apply_r_riscv_add16_rela,
> > - [R_RISCV_ADD32] = apply_r_riscv_add32_rela,
> > - [R_RISCV_ADD64] = apply_r_riscv_add64_rela,
> > - [R_RISCV_SUB16] = apply_r_riscv_sub16_rela,
> > - [R_RISCV_SUB32] = apply_r_riscv_sub32_rela,
> > - [R_RISCV_SUB64] = apply_r_riscv_sub64_rela,
> > +static int dynamic_linking_not_supported(struct module *me, void *location,
> > + Elf_Addr v)
> > +{
> > + pr_err("%s: Dynamic linking not supported in kernel modules PC = %p\n",
> > + me->name, location);
> > + return -EINVAL;
> > +}
> > +
> > +static int tls_not_supported(struct module *me, void *location, Elf_Addr v)
> > +{
> > + pr_err("%s: Thread local storage not supported in kernel modules PC = %p\n",
> > + me->name, location);
> > + return -EINVAL;
> > +}
> > +
> > +static int apply_r_riscv_sub6_rela(struct module *me, void *location, Elf_Addr v)
> > +{
> > + *(u8 *)location = (*(u8 *)location - ((u8)v & 0x3F)) & 0x3F;
> > + return 0;
> > +}
> > +
> > +static int apply_r_riscv_set6_rela(struct module *me, void *location, Elf_Addr v)
> > +{
> > + *(u8 *)location = ((*(u8 *)location & 0xc0) | ((u8)v & 0x3F));
>
> Most of these casts are simple enough for here and above I'd consider
> something like this for readability:
>
> u8 *byte = location;
> u8 value = v;
>
> *byte = (*byte & 0xc0) | (value & 0x3f)
>
> The compiler should generate the same code.
>
> > + return 0;
> > +}
> > +
> > +static int apply_r_riscv_set8_rela(struct module *me, void *location, Elf_Addr v)
> > +{
> > + *(u8 *)location = (u8)v;
> > + return 0;
> > +}
> > +
> > +static int apply_r_riscv_set16_rela(struct module *me, void *location,
> > + Elf_Addr v)
> > +{
> > + *(u16 *)location = (u16)v;
> > + return 0;
> > +}
> > +
> > +static int apply_r_riscv_set32_rela(struct module *me, void *location,
> > + Elf_Addr v)
> > +{
> > + *(u32 *)location = (u32)v;
> > + return 0;
> > +}
> > +
> > +static int apply_r_riscv_32_pcrel_rela(struct module *me, void *location,
> > + Elf_Addr v)
> > +{
> > + *(u32 *)location = v - (unsigned long)location;
> > + return 0;
> > +}
> > +
> > +static int apply_r_riscv_plt32_rela(struct module *me, void *location,
> > + Elf_Addr v)
> > +{
> > + ptrdiff_t offset = (void *)v - location;
> > +
> > + if (!riscv_insn_valid_32bit_offset(offset)) {
> > + /* Only emit the plt entry if offset over 32-bit range */
> > + if (IS_ENABLED(CONFIG_MODULE_SECTIONS)) {
> > + offset = (void *)module_emit_plt_entry(me, v) - location;
> > + } else {
> > + pr_err("%s: target %016llx can not be addressed by the 32-bit offset from PC = %p\n",
> > + me->name, (long long)v, location);
> > + return -EINVAL;
> > + }
> > + }
> > +
> > + *(u32 *)location = (u32)offset;
> > + return 0;
> > +}
> > +
> > +static int apply_r_riscv_set_uleb128(struct module *me, void *location, Elf_Addr v)
> > +{
> > + *(long *)location = v;
> > + return 0;
> > +}
> > +
> > +static int apply_r_riscv_sub_uleb128(struct module *me, void *location, Elf_Addr v)
> > +{
> > + *(long *)location -= v;
> > + return 0;
> > +}
> > +
> > +/*
> > + * Relocations defined in the riscv-elf-psabi-doc.
> > + * This handles static linking only.
> > + */
> > +static int (*reloc_handlers_rela[])(struct module *me, void *location,
> > + Elf_Addr v) = {
> > + [R_RISCV_32] = apply_r_riscv_32_rela,
> > + [R_RISCV_64] = apply_r_riscv_64_rela,
> > + [R_RISCV_RELATIVE] = dynamic_linking_not_supported,
> > + [R_RISCV_COPY] = dynamic_linking_not_supported,
> > + [R_RISCV_JUMP_SLOT] = dynamic_linking_not_supported,
> > + [R_RISCV_TLS_DTPMOD32] = dynamic_linking_not_supported,
> > + [R_RISCV_TLS_DTPMOD64] = dynamic_linking_not_supported,
> > + [R_RISCV_TLS_DTPREL32] = dynamic_linking_not_supported,
> > + [R_RISCV_TLS_DTPREL64] = dynamic_linking_not_supported,
> > + [R_RISCV_TLS_TPREL32] = dynamic_linking_not_supported,
> > + [R_RISCV_TLS_TPREL64] = dynamic_linking_not_supported,
> > + /* 12-15 undefined */
> > + [R_RISCV_BRANCH] = apply_r_riscv_branch_rela,
> > + [R_RISCV_JAL] = apply_r_riscv_jal_rela,
> > + [R_RISCV_CALL] = apply_r_riscv_call_rela,
> > + [R_RISCV_CALL_PLT] = apply_r_riscv_call_plt_rela,
> > + [R_RISCV_GOT_HI20] = apply_r_riscv_got_hi20_rela,
> > + [R_RISCV_TLS_GOT_HI20] = tls_not_supported,
> > + [R_RISCV_TLS_GD_HI20] = tls_not_supported,
> > + [R_RISCV_PCREL_HI20] = apply_r_riscv_pcrel_hi20_rela,
> > + [R_RISCV_PCREL_LO12_I] = apply_r_riscv_pcrel_lo12_i_rela,
> > + [R_RISCV_PCREL_LO12_S] = apply_r_riscv_pcrel_lo12_s_rela,
> > + [R_RISCV_HI20] = apply_r_riscv_hi20_rela,
> > + [R_RISCV_LO12_I] = apply_r_riscv_lo12_i_rela,
> > + [R_RISCV_LO12_S] = apply_r_riscv_lo12_s_rela,
> > + [R_RISCV_TPREL_HI20] = tls_not_supported,
> > + [R_RISCV_TPREL_LO12_I] = tls_not_supported,
> > + [R_RISCV_TPREL_LO12_S] = tls_not_supported,
> > + [R_RISCV_TPREL_ADD] = tls_not_supported,
> > + [R_RISCV_ADD8] = apply_r_riscv_add8_rela,
> > + [R_RISCV_ADD16] = apply_r_riscv_add16_rela,
> > + [R_RISCV_ADD32] = apply_r_riscv_add32_rela,
> > + [R_RISCV_ADD64] = apply_r_riscv_add64_rela,
> > + [R_RISCV_SUB8] = apply_r_riscv_sub8_rela,
> > + [R_RISCV_SUB16] = apply_r_riscv_sub16_rela,
> > + [R_RISCV_SUB32] = apply_r_riscv_sub32_rela,
> > + [R_RISCV_SUB64] = apply_r_riscv_sub64_rela,
> > + /* 41-42 reserved for future standard use */
> > + [R_RISCV_ALIGN] = apply_r_riscv_align_rela,
> > + [R_RISCV_RVC_BRANCH] = apply_r_riscv_rvc_branch_rela,
> > + [R_RISCV_RVC_JUMP] = apply_r_riscv_rvc_jump_rela,
> > + /* 46-50 reserved for future standard use */
> > + [R_RISCV_RELAX] = apply_r_riscv_relax_rela,
> > + [R_RISCV_SUB6] = apply_r_riscv_sub6_rela,
> > + [R_RISCV_SET6] = apply_r_riscv_set6_rela,
> > + [R_RISCV_SET8] = apply_r_riscv_set8_rela,
> > + [R_RISCV_SET16] = apply_r_riscv_set16_rela,
> > + [R_RISCV_SET32] = apply_r_riscv_set32_rela,
> > + [R_RISCV_32_PCREL] = apply_r_riscv_32_pcrel_rela,
> > + [R_RISCV_IRELATIVE] = dynamic_linking_not_supported,
> > + [R_RISCV_PLT32] = apply_r_riscv_plt32_rela,
> > + [R_RISCV_SET_ULEB128] = apply_r_riscv_set_uleb128,
> > + [R_RISCV_SUB_ULEB128] = apply_r_riscv_sub_uleb128,
> > + /* 62-191 reserved for future standard use */
> > + /* 192-255 nonstandard ABI extensions */
> > +};
> > +
> > +static bool accumulate_relocations[] = {
> > + [R_RISCV_32] = false,
> > + [R_RISCV_64] = false,
> > + [R_RISCV_RELATIVE] = false,
> > + [R_RISCV_COPY] = false,
> > + [R_RISCV_JUMP_SLOT] = false,
> > + [R_RISCV_TLS_DTPMOD32] = false,
> > + [R_RISCV_TLS_DTPMOD64] = false,
> > + [R_RISCV_TLS_DTPREL32] = false,
> > + [R_RISCV_TLS_DTPREL64] = false,
> > + [R_RISCV_TLS_TPREL32] = false,
> > + [R_RISCV_TLS_TPREL64] = false,
> > + /* 12-15 undefined */
> > + [R_RISCV_BRANCH] = false,
> > + [R_RISCV_JAL] = false,
> > + [R_RISCV_CALL] = false,
> > + [R_RISCV_CALL_PLT] = false,
> > + [R_RISCV_GOT_HI20] = false,
> > + [R_RISCV_TLS_GOT_HI20] = false,
> > + [R_RISCV_TLS_GD_HI20] = false,
> > + [R_RISCV_PCREL_HI20] = false,
> > + [R_RISCV_PCREL_LO12_I] = false,
> > + [R_RISCV_PCREL_LO12_S] = false,
> > + [R_RISCV_HI20] = false,
> > + [R_RISCV_LO12_I] = false,
> > + [R_RISCV_LO12_S] = false,
> > + [R_RISCV_TPREL_HI20] = false,
> > + [R_RISCV_TPREL_LO12_I] = false,
> > + [R_RISCV_TPREL_LO12_S] = false,
> > + [R_RISCV_TPREL_ADD] = false,
> > + [R_RISCV_ADD8] = true,
> > + [R_RISCV_ADD16] = true,
> > + [R_RISCV_ADD32] = true,
> > + [R_RISCV_ADD64] = true,
> > + [R_RISCV_SUB8] = true,
> > + [R_RISCV_SUB16] = true,
> > + [R_RISCV_SUB32] = true,
> > + [R_RISCV_SUB64] = true,
> > + /* 41-42 reserved for future standard use */
> > + [R_RISCV_ALIGN] = false,
> > + [R_RISCV_RVC_BRANCH] = false,
> > + [R_RISCV_RVC_JUMP] = false,
> > + /* 46-50 reserved for future standard use */
> > + [R_RISCV_RELAX] = false,
> > + [R_RISCV_SUB6] = true,
> > + [R_RISCV_SET6] = true,
> > + [R_RISCV_SET8] = true,
> > + [R_RISCV_SET16] = true,
> > + [R_RISCV_SET32] = true,
> > + [R_RISCV_32_PCREL] = false,
> > + [R_RISCV_IRELATIVE] = false,
> > + [R_RISCV_PLT32] = false,
> > + [R_RISCV_SET_ULEB128] = true,
> > + [R_RISCV_SUB_ULEB128] = true,
> > + /* 62-191 reserved for future standard use */
> > + /* 192-255 nonstandard ABI extensions */
> > +};
> > +
> > +static int accumulation_not_supported(struct module *me, void *location, long buffer)
> > +{
> > + pr_err("%s: Internal error. Only ADD/SUB/SET/ULEB128 should be accumulated.", me->name);
> > + return -EINVAL;
> > +}
> > +
> > +static int apply_6_bit_accumulation(struct module *me, void *location, long buffer)
> > +{
> > + if (buffer != (buffer & 0x3F)) {
>
> It may be a matter of taste, but I'd find something like this easier to read:
>
> if (buffer > 0x3f) {
>
> To me that more directly conveys that you're just checking if any if the higher
> bits are set.
>
> > + pr_err("%s: value %ld out of range for 6-bit relocation.\n",
> > + me->name, buffer);
> > + return -EINVAL;
> > + }
> > + *(u8 *)location = ((*(u8 *)location & 0xc0) | ((u8)buffer & 0x3F));
>
> Here the casts also get a bit much.
>
> > + return 0;
> > +}
> > +
> > +static int apply_8_bit_accumulation(struct module *me, void *location, long buffer)
> > +{
> > + if (buffer != (u8)buffer) {
> > + pr_err("%s: value %ld out of range for 8-bit relocation.\n",
> > + me->name, buffer);
> > + return -EINVAL;
> > + }
> > + *(u8 *)location = (u8)buffer;
> > + return 0;
> > +}
> > +
> > +static int apply_16_bit_accumulation(struct module *me, void *location, long buffer)
> > +{
> > + if (buffer != (u16)buffer) {
>
> if (buffer > U16_MAX) {
>
> > + pr_err("%s: value %ld out of range for 16-bit relocation.\n",
> > + me->name, buffer);
> > + return -EINVAL;
> > + }
> > + *(u16 *)location = (u16)buffer;
> > + return 0;
> > +}
> > +
> > +static int apply_32_bit_accumulation(struct module *me, void *location, long buffer)
> > +{
> > + if (buffer != (u32)buffer) {
>
> if (buffer > U32_MAX) {
>
> > + pr_err("%s: value %ld out of range for 32-bit relocation.\n",
> > + me->name, buffer);
> > + return -EINVAL;
> > + }
> > + *(u32 *)location = (u32)buffer;
> > + return 0;
> > +}
> > +
> > +static int apply_64_bit_accumulation(struct module *me, void *location, long buffer)
> > +{
> > + *(u64 *)location = (u64)buffer;
> > + return 0;
> > +}
> > +
> > +static int apply_uleb128_accumulation(struct module *me, void *location, long buffer)
> > +{
> > + /*
> > + * ULEB128 is a variable length encoding. Encode the buffer into
> > + * the ULEB128 data format.
> > + */
> > + while (buffer != 0) {
> > + *(u8 *)location = (u8)buffer & 0x7F;
> > + buffer >>= 7;
> > + *(u8 *)location |= (buffer != 0) << 7;
> > + location = (u8 *)location + 1;
> > + }
>
> How about something like
>
> u8 *p = location;
>
> while (buffer) {
> u8 value = buffer & 0x7f;
>
> buffer >>= 7;
> value |= (!!buffer) << 7;
>
> *p++ = value;
> }
>
>
> This sould do the same as above, but if buffer == 0 it doesn't write anything.
> Is that the right thing to do?
>

All great suggestions, thank you. Yes, that is the right thing to do
when buffer == 0.

- Charlie

> > + return 0;
> > +}
> > +
> > +/*
> > + * Need to duplicate this a third time to capture the handlers for accumulation.
> > + */
> > +static int (*accumulate_handlers[])(struct module *me, void *location, long buffer) = {
> > + [R_RISCV_32] = accumulation_not_supported,
> > + [R_RISCV_64] = accumulation_not_supported,
> > + [R_RISCV_RELATIVE] = accumulation_not_supported,
> > + [R_RISCV_COPY] = accumulation_not_supported,
> > + [R_RISCV_JUMP_SLOT] = accumulation_not_supported,
> > + [R_RISCV_TLS_DTPMOD32] = accumulation_not_supported,
> > + [R_RISCV_TLS_DTPMOD64] = accumulation_not_supported,
> > + [R_RISCV_TLS_DTPREL32] = accumulation_not_supported,
> > + [R_RISCV_TLS_DTPREL64] = accumulation_not_supported,
> > + [R_RISCV_TLS_TPREL32] = accumulation_not_supported,
> > + [R_RISCV_TLS_TPREL64] = accumulation_not_supported,
> > + /* 12-15 undefined */
> > + [R_RISCV_BRANCH] = accumulation_not_supported,
> > + [R_RISCV_JAL] = accumulation_not_supported,
> > + [R_RISCV_CALL] = accumulation_not_supported,
> > + [R_RISCV_CALL_PLT] = accumulation_not_supported,
> > + [R_RISCV_GOT_HI20] = accumulation_not_supported,
> > + [R_RISCV_TLS_GOT_HI20] = accumulation_not_supported,
> > + [R_RISCV_TLS_GD_HI20] = accumulation_not_supported,
> > + [R_RISCV_PCREL_HI20] = accumulation_not_supported,
> > + [R_RISCV_PCREL_LO12_I] = accumulation_not_supported,
> > + [R_RISCV_PCREL_LO12_S] = accumulation_not_supported,
> > + [R_RISCV_HI20] = accumulation_not_supported,
> > + [R_RISCV_LO12_I] = accumulation_not_supported,
> > + [R_RISCV_LO12_S] = accumulation_not_supported,
> > + [R_RISCV_TPREL_HI20] = accumulation_not_supported,
> > + [R_RISCV_TPREL_LO12_I] = accumulation_not_supported,
> > + [R_RISCV_TPREL_LO12_S] = accumulation_not_supported,
> > + [R_RISCV_TPREL_ADD] = accumulation_not_supported,
> > + [R_RISCV_ADD8] = apply_8_bit_accumulation,
> > + [R_RISCV_ADD16] = apply_16_bit_accumulation,
> > + [R_RISCV_ADD32] = apply_32_bit_accumulation,
> > + [R_RISCV_ADD64] = apply_64_bit_accumulation,
> > + [R_RISCV_SUB8] = apply_8_bit_accumulation,
> > + [R_RISCV_SUB16] = apply_16_bit_accumulation,
> > + [R_RISCV_SUB32] = apply_32_bit_accumulation,
> > + [R_RISCV_SUB64] = apply_64_bit_accumulation,
> > + /* 41-42 reserved for future standard use */
> > + [R_RISCV_ALIGN] = accumulation_not_supported,
> > + [R_RISCV_RVC_BRANCH] = accumulation_not_supported,
> > + [R_RISCV_RVC_JUMP] = accumulation_not_supported,
> > + /* 46-50 reserved for future standard use */
> > + [R_RISCV_RELAX] = accumulation_not_supported,
> > + [R_RISCV_SUB6] = apply_6_bit_accumulation,
> > + [R_RISCV_SET6] = apply_6_bit_accumulation,
> > + [R_RISCV_SET8] = apply_8_bit_accumulation,
> > + [R_RISCV_SET16] = apply_16_bit_accumulation,
> > + [R_RISCV_SET32] = apply_32_bit_accumulation,
> > + [R_RISCV_32_PCREL] = accumulation_not_supported,
> > + [R_RISCV_IRELATIVE] = accumulation_not_supported,
> > + [R_RISCV_PLT32] = accumulation_not_supported,
> > + [R_RISCV_SET_ULEB128] = apply_uleb128_accumulation,
> > + [R_RISCV_SUB_ULEB128] = apply_uleb128_accumulation,
> > + /* 62-191 reserved for future standard use */
> > + /* 192-255 nonstandard ABI extensions */
> > };
> >
> > +void process_accumulated_relocations(struct module *me)
> > +{
> > + /*
> > + * Only ADD/SUB/SET/ULEB128 should end up here.
> > + *
> > + * Each bucket may have more than one relocation location. All
> > + * relocations for a location are stored in a list in a bucket.
> > + *
> > + * Relocations are applied to a temp variable before being stored to the
> > + * provided location to check for overflow. This also allows ULEB128 to
> > + * properly decide how many entries are needed before storing to
> > + * location. The final value is stored into location using the handler
> > + * for the last relocation to an address.
> > + *
> > + * Three layers of indexing:
> > + * - Each of the buckets in use
> > + * - Groups of relocations in each bucket by location address
> > + * - Each relocation entry for a location address
> > + */
> > + struct used_bucket *bucket_iter;
> > + struct relocation_head *rel_head_iter;
> > + struct relocation_entry *rel_entry_iter;
> > + int curr_type;
> > + void *location;
> > + long buffer;
> > +
> > + list_for_each_entry(bucket_iter, &used_buckets_list, head) {
> > + hlist_for_each_entry(rel_head_iter, bucket_iter->bucket, node) {
> > + buffer = 0;
> > + location = rel_head_iter->location;
> > + list_for_each_entry(rel_entry_iter, rel_head_iter->rel_entry, head) {
> > + curr_type = rel_entry_iter->type;
> > + reloc_handlers_rela[curr_type](me, &buffer, rel_entry_iter->value);
> > + kfree(rel_entry_iter);
> > + }
> > + accumulate_handlers[curr_type](me, location, buffer);
> > + kfree(rel_head_iter);
> > + }
> > + kfree(bucket_iter);
> > + }
> > +
> > + kfree(relocation_hashtable);
> > +}
> > +
> > +int add_relocation_to_accumulate(struct module *me, int type, void *location,
> > + unsigned int hashtable_bits, Elf_Addr v)
> > +{
> > + struct relocation_entry *entry;
> > + struct relocation_head *rel_head;
> > + struct hlist_head *current_head;
> > + struct hlist_node *first;
> > + struct used_bucket *bucket;
> > + unsigned long hash;
> > +
> > + entry = kmalloc(sizeof(*entry), GFP_KERNEL);
> > + INIT_LIST_HEAD(&entry->head);
> > + entry->type = type;
> > + entry->value = v;
> > +
> > + hash = hash_min((unsigned long)location, hashtable_bits);
> > +
> > + current_head = &relocation_hashtable[hash];
> > + first = current_head->first;
> > +
> > + /* Find matching location (if any) */
> > + bool found = false;
> > + struct relocation_head *rel_head_iter;
> > +
> > + hlist_for_each_entry(rel_head_iter, current_head, node) {
> > + if (rel_head_iter->location == location) {
> > + found = true;
> > + rel_head = rel_head_iter;
> > + break;
> > + }
> > + }
> > +
> > + if (!found) {
> > + rel_head = kmalloc(sizeof(*rel_head), GFP_KERNEL);
> > + rel_head->rel_entry =
> > + kmalloc(sizeof(struct list_head), GFP_KERNEL);
> > + INIT_LIST_HEAD(rel_head->rel_entry);
> > + rel_head->location = location;
> > + INIT_HLIST_NODE(&rel_head->node);
> > + if (!current_head->first) {
> > + bucket =
> > + kmalloc(sizeof(struct used_bucket), GFP_KERNEL);
> > + INIT_LIST_HEAD(&bucket->head);
> > + bucket->bucket = current_head;
> > + list_add(&bucket->head, &used_buckets_list);
> > + }
> > + hlist_add_head(&rel_head->node, current_head);
> > + }
> > +
> > + /* Add relocation to head of discovered rel_head */
> > + list_add_tail(&entry->head, rel_head->rel_entry);
> > +
> > + return 0;
> > +}
> > +
> > +unsigned int initialize_relocation_hashtable(unsigned int num_relocations)
> > +{
> > + /* Can safely assume that bits is not greater than sizeof(long) */
> > + unsigned long hashtable_size = roundup_pow_of_two(num_relocations);
> > + unsigned int hashtable_bits = ilog2(hashtable_size);
> > +
> > + /*
> > + * Double size of hashtable if num_relocations * 1.25 is greater than
> > + * hashtable_size.
> > + */
> > + int should_double_size = ((num_relocations + (num_relocations >> 2)) > (hashtable_size));
> > +
> > + hashtable_bits += should_double_size;
> > +
> > + hashtable_size <<= should_double_size;
> > +
> > + relocation_hashtable = kmalloc_array(hashtable_size,
> > + sizeof(*relocation_hashtable),
> > + GFP_KERNEL);
> > + __hash_init(relocation_hashtable, hashtable_size);
> > +
> > + INIT_LIST_HEAD(&used_buckets_list);
> > +
> > + return hashtable_bits;
> > +}
> > +
> > int apply_relocate_add(Elf_Shdr *sechdrs, const char *strtab,
> > unsigned int symindex, unsigned int relsec,
> > struct module *me)
> > @@ -349,11 +822,13 @@ int apply_relocate_add(Elf_Shdr *sechdrs, const char *strtab,
> > unsigned int i, type;
> > Elf_Addr v;
> > int res;
> > + unsigned int num_relocations = sechdrs[relsec].sh_size / sizeof(*rel);
> > + unsigned int hashtable_bits = initialize_relocation_hashtable(num_relocations);
> >
> > pr_debug("Applying relocate section %u to %u\n", relsec,
> > sechdrs[relsec].sh_info);
> >
> > - for (i = 0; i < sechdrs[relsec].sh_size / sizeof(*rel); i++) {
> > + for (i = 0; i < num_relocations; i++) {
> > /* This is where to make the change */
> > location = (void *)sechdrs[sechdrs[relsec].sh_info].sh_addr
> > + rel[i].r_offset;
> > @@ -428,11 +903,16 @@ int apply_relocate_add(Elf_Shdr *sechdrs, const char *strtab,
> > }
> > }
> >
> > - res = handler(me, location, v);
> > + if (accumulate_relocations[type])
> > + res = add_relocation_to_accumulate(me, type, location, hashtable_bits, v);
> > + else
> > + res = handler(me, location, v);
> > if (res)
> > return res;
> > }
> >
> > + process_accumulated_relocations(me);
> > +
> > return 0;
> > }
> >
> >
> > --
> > 2.34.1
> >
> >
> > _______________________________________________
> > linux-riscv mailing list
> > [email protected]
> > http://lists.infradead.org/mailman/listinfo/linux-riscv

2023-10-31 20:07:02

by Charlie Jenkins

[permalink] [raw]
Subject: Re: [PATCH v7 1/3] riscv: Avoid unaligned access when relocating modules

On Tue, Oct 31, 2023 at 05:35:57PM +0100, Andreas Schwab wrote:
> On Okt 31 2023, Emil Renner Berthing wrote:
>
> >> +static int riscv_insn_rmw(void *location, u32 keep, u32 set)
> >> +{
> >> + u16 *parcel = location;
> >> + u32 insn = (u32)le16_to_cpu(parcel[0]) | (u32)le16_to_cpu(parcel[1]) << 16;
> >> +
> >> + insn &= keep;
> >> + insn |= set;
> >> +
> >> + parcel[0] = cpu_to_le32(insn);
> >
> > Why cpu_to_le32(insn)? Unless I've misunderstood something downcasting unsigned
> > to unsigned values in C (eg. from u32 to u16) is defined to always discard the
> > most signifcant bits, so cpu_to_le16(insn) should be fine.
>
> cpu_to_le32(insn) can't be right here anyway, since it also swaps the
> two u16 halves and would be the same as cpu_to_le16(insn >> 16) on big
> endian.

Yes, not sure why I did that... I will fix that up.

- Charlie

>
> --
> Andreas Schwab, [email protected]
> GPG Key fingerprint = 7578 EB47 D4E5 4D69 2510 2552 DF73 E780 A9DA AEC1
> "And now for something completely different."