2023-11-27 22:05:46

by Charlie Jenkins

[permalink] [raw]
Subject: [PATCH v4 0/2] riscv: Fix issues with module loading

Module loading did not account for multiple threads concurrently loading
modules. This patch fixes that issue. There is also a small patch to fix
the type of a __le16 variable.

Signed-off-by: Charlie Jenkins <[email protected]>
---
Changes in v4:
- Make functions only used internally static
- Free data structures on kmalloc failure (Andreas)
- Link to v3: https://lore.kernel.org/r/[email protected]

Changes in v3:
- Cleanup pointer passing (Samuel)
- Correct indentation (Samuel)
- Check for kmalloc failures (Samuel)
- Link to v2: https://lore.kernel.org/r/[email protected]

Changes in v2:
- Support linking modules concurrently across threads.
- Link to v1: https://lore.kernel.org/r/[email protected]

---
Charlie Jenkins (2):
riscv: Safely remove entries from relocation list
riscv: Correct type casting in module loading

arch/riscv/kernel/module.c | 114 +++++++++++++++++++++++++++++++++------------
1 file changed, 84 insertions(+), 30 deletions(-)
---
base-commit: 98b1cc82c4affc16f5598d4fa14b1858671b2263
change-id: 20231120-module_linking_freeing-2b5a3b255b5e
--
- Charlie


2023-11-27 22:06:11

by Charlie Jenkins

[permalink] [raw]
Subject: [PATCH v4 1/2] riscv: Safely remove entries from relocation list

Use the safe versions of list and hlist iteration to safely remove
entries from the module relocation lists. To allow mutliple threads to
load modules concurrently, move relocation list pointers onto the stack
rather than using global variables.

Fixes: 8fd6c5142395 ("riscv: Add remaining module relocations")
Reported-by: Ron Economos <[email protected]>
Closes: https://lore.kernel.org/linux-riscv/[email protected]
Signed-off-by: Charlie Jenkins <[email protected]>
---
arch/riscv/kernel/module.c | 110 +++++++++++++++++++++++++++++++++------------
1 file changed, 82 insertions(+), 28 deletions(-)

diff --git a/arch/riscv/kernel/module.c b/arch/riscv/kernel/module.c
index 56a8c78e9e21..53593fe58cd8 100644
--- a/arch/riscv/kernel/module.c
+++ b/arch/riscv/kernel/module.c
@@ -40,15 +40,6 @@ struct relocation_handlers {
long buffer);
};

-unsigned int initialize_relocation_hashtable(unsigned int num_relocations);
-void process_accumulated_relocations(struct module *me);
-int add_relocation_to_accumulate(struct module *me, int type, void *location,
- unsigned int hashtable_bits, Elf_Addr v);
-
-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)
@@ -604,7 +595,10 @@ static const struct relocation_handlers reloc_handlers[] = {
/* 192-255 nonstandard ABI extensions */
};

-void process_accumulated_relocations(struct module *me)
+static void
+process_accumulated_relocations(struct module *me,
+ struct hlist_head **relocation_hashtable,
+ struct list_head *used_buckets_list)
{
/*
* Only ADD/SUB/SET/ULEB128 should end up here.
@@ -624,18 +618,25 @@ void process_accumulated_relocations(struct module *me)
* - Each relocation entry for a location address
*/
struct used_bucket *bucket_iter;
+ struct used_bucket *bucket_iter_tmp;
struct relocation_head *rel_head_iter;
+ struct hlist_node *rel_head_iter_tmp;
struct relocation_entry *rel_entry_iter;
+ struct relocation_entry *rel_entry_iter_tmp;
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) {
+ list_for_each_entry_safe(bucket_iter, bucket_iter_tmp,
+ used_buckets_list, head) {
+ hlist_for_each_entry_safe(rel_head_iter, rel_head_iter_tmp,
+ bucket_iter->bucket, node) {
buffer = 0;
location = rel_head_iter->location;
- list_for_each_entry(rel_entry_iter,
- rel_head_iter->rel_entry, head) {
+ list_for_each_entry_safe(rel_entry_iter,
+ rel_entry_iter_tmp,
+ rel_head_iter->rel_entry,
+ head) {
curr_type = rel_entry_iter->type;
reloc_handlers[curr_type].reloc_handler(
me, &buffer, rel_entry_iter->value);
@@ -648,11 +649,14 @@ void process_accumulated_relocations(struct module *me)
kfree(bucket_iter);
}

- kfree(relocation_hashtable);
+ kfree(*relocation_hashtable);
}

-int add_relocation_to_accumulate(struct module *me, int type, void *location,
- unsigned int hashtable_bits, Elf_Addr v)
+static int add_relocation_to_accumulate(struct module *me, int type,
+ void *location,
+ unsigned int hashtable_bits, Elf_Addr v,
+ struct hlist_head *relocation_hashtable,
+ struct list_head *used_buckets_list)
{
struct relocation_entry *entry;
struct relocation_head *rel_head;
@@ -661,6 +665,10 @@ int add_relocation_to_accumulate(struct module *me, int type, void *location,
unsigned long hash;

entry = kmalloc(sizeof(*entry), GFP_KERNEL);
+
+ if (!entry)
+ return -ENOMEM;
+
INIT_LIST_HEAD(&entry->head);
entry->type = type;
entry->value = v;
@@ -669,7 +677,10 @@ int add_relocation_to_accumulate(struct module *me, int type, void *location,

current_head = &relocation_hashtable[hash];

- /* Find matching location (if any) */
+ /*
+ * Search for the relocation_head for the relocations that happen at the
+ * provided location
+ */
bool found = false;
struct relocation_head *rel_head_iter;

@@ -681,19 +692,45 @@ int add_relocation_to_accumulate(struct module *me, int type, void *location,
}
}

+ /*
+ * If there has not yet been any relocations at the provided location,
+ * create a relocation_head for that location and populate it with this
+ * relocation_entry.
+ */
if (!found) {
rel_head = kmalloc(sizeof(*rel_head), GFP_KERNEL);
+
+ if (!rel_head) {
+ kfree(entry);
+ return -ENOMEM;
+ }
+
rel_head->rel_entry =
kmalloc(sizeof(struct list_head), GFP_KERNEL);
+
+ if (!rel_head->rel_entry) {
+ kfree(entry);
+ kfree(rel_head);
+ return -ENOMEM;
+ }
+
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);
+
+ if (!bucket) {
+ kfree(entry);
+ kfree(rel_head);
+ kfree(rel_head->rel_entry);
+ return -ENOMEM;
+ }
+
INIT_LIST_HEAD(&bucket->head);
bucket->bucket = current_head;
- list_add(&bucket->head, &used_buckets_list);
+ list_add(&bucket->head, used_buckets_list);
}
hlist_add_head(&rel_head->node, current_head);
}
@@ -704,7 +741,9 @@ int add_relocation_to_accumulate(struct module *me, int type, void *location,
return 0;
}

-unsigned int initialize_relocation_hashtable(unsigned int num_relocations)
+static unsigned int
+initialize_relocation_hashtable(unsigned int num_relocations,
+ struct hlist_head **relocation_hashtable)
{
/* Can safely assume that bits is not greater than sizeof(long) */
unsigned long hashtable_size = roundup_pow_of_two(num_relocations);
@@ -720,12 +759,13 @@ unsigned int initialize_relocation_hashtable(unsigned int num_relocations)

hashtable_size <<= should_double_size;

- relocation_hashtable = kmalloc_array(hashtable_size,
- sizeof(*relocation_hashtable),
- GFP_KERNEL);
- __hash_init(relocation_hashtable, hashtable_size);
+ *relocation_hashtable = kmalloc_array(hashtable_size,
+ sizeof(*relocation_hashtable),
+ GFP_KERNEL);
+ if (!*relocation_hashtable)
+ return -ENOMEM;

- INIT_LIST_HEAD(&used_buckets_list);
+ __hash_init(*relocation_hashtable, hashtable_size);

return hashtable_bits;
}
@@ -742,7 +782,17 @@ int apply_relocate_add(Elf_Shdr *sechdrs, const char *strtab,
Elf_Addr v;
int res;
unsigned int num_relocations = sechdrs[relsec].sh_size / sizeof(*rel);
- unsigned int hashtable_bits = initialize_relocation_hashtable(num_relocations);
+ struct hlist_head *relocation_hashtable;
+ struct list_head used_buckets_list;
+ unsigned int hashtable_bits;
+
+ hashtable_bits = initialize_relocation_hashtable(num_relocations,
+ &relocation_hashtable);
+
+ if (hashtable_bits < 0)
+ return hashtable_bits;
+
+ INIT_LIST_HEAD(&used_buckets_list);

pr_debug("Applying relocate section %u to %u\n", relsec,
sechdrs[relsec].sh_info);
@@ -823,14 +873,18 @@ int apply_relocate_add(Elf_Shdr *sechdrs, const char *strtab,
}

if (reloc_handlers[type].accumulate_handler)
- res = add_relocation_to_accumulate(me, type, location, hashtable_bits, v);
+ res = add_relocation_to_accumulate(me, type, location,
+ hashtable_bits, v,
+ relocation_hashtable,
+ &used_buckets_list);
else
res = handler(me, location, v);
if (res)
return res;
}

- process_accumulated_relocations(me);
+ process_accumulated_relocations(me, &relocation_hashtable,
+ &used_buckets_list);

return 0;
}

--
2.42.0

2023-11-27 22:06:12

by Charlie Jenkins

[permalink] [raw]
Subject: [PATCH v4 2/2] riscv: Correct type casting in module loading

Use __le16 with le16_to_cpu.

Fixes: 8fd6c5142395 ("riscv: Add remaining module relocations")
Signed-off-by: Charlie Jenkins <[email protected]>
Reviewed-by: Samuel Holland <[email protected]>
Tested-by: Samuel Holland <[email protected]>
---
arch/riscv/kernel/module.c | 4 ++--
1 file changed, 2 insertions(+), 2 deletions(-)

diff --git a/arch/riscv/kernel/module.c b/arch/riscv/kernel/module.c
index 53593fe58cd8..aac019ed63b1 100644
--- a/arch/riscv/kernel/module.c
+++ b/arch/riscv/kernel/module.c
@@ -55,7 +55,7 @@ static bool riscv_insn_valid_32bit_offset(ptrdiff_t val)

static int riscv_insn_rmw(void *location, u32 keep, u32 set)
{
- u16 *parcel = location;
+ __le16 *parcel = location;
u32 insn = (u32)le16_to_cpu(parcel[0]) | (u32)le16_to_cpu(parcel[1]) << 16;

insn &= keep;
@@ -68,7 +68,7 @@ static int riscv_insn_rmw(void *location, u32 keep, u32 set)

static int riscv_insn_rvc_rmw(void *location, u16 keep, u16 set)
{
- u16 *parcel = location;
+ __le16 *parcel = location;
u16 insn = le16_to_cpu(*parcel);

insn &= keep;

--
2.42.0

2023-11-29 07:26:20

by Björn Töpel

[permalink] [raw]
Subject: Re: [PATCH v4 0/2] riscv: Fix issues with module loading

Charlie Jenkins <[email protected]> writes:

> Module loading did not account for multiple threads concurrently loading
> modules. This patch fixes that issue. There is also a small patch to fix
> the type of a __le16 variable.
>
> Signed-off-by: Charlie Jenkins <[email protected]>
> ---
> Changes in v4:
> - Make functions only used internally static
> - Free data structures on kmalloc failure (Andreas)
> - Link to v3: https://lore.kernel.org/r/[email protected]
>
> Changes in v3:
> - Cleanup pointer passing (Samuel)
> - Correct indentation (Samuel)
> - Check for kmalloc failures (Samuel)
> - Link to v2: https://lore.kernel.org/r/[email protected]
>
> Changes in v2:
> - Support linking modules concurrently across threads.
> - Link to v1: https://lore.kernel.org/r/[email protected]
>
> ---
> Charlie Jenkins (2):
> riscv: Safely remove entries from relocation list
> riscv: Correct type casting in module loading

I got hit by this on QEMU for 6.7-rc3:
| Unable to handle kernel paging request at virtual address 6b6b6b6b6b6b6b83
| Oops [#1]
| Modules linked in:
| CPU: 3 PID: 1 Comm: systemd Not tainted 6.7.0-rc3 #1
| Hardware name: riscv-virtio,qemu (DT)
| epc : process_accumulated_relocations+0x70/0x162
| ra : process_accumulated_relocations+0x96/0x162
| epc : ffffffff8000c528 ra : ffffffff8000c54e sp : ff20000000023ab0
| gp : ffffffff8257fea0 tp : ff60000080860040 t0 : ff60000083934d68
| t1 : 0000000000000006 t2 : 0000000000000002 s0 : ff20000000023b30
| s1 : 6b6b6b6b6b6b6b6b a0 : ffffffff80ed48d8 a1 : 0000000000000000
| a2 : ff1c0000040a9610 a3 : 0000000000000000 a4 : 0000000000000000
| a5 : ff600000832d6990 a6 : ff1c0000040e4d10 a7 : 000000000bbb1d6d
| s2 : 0000000000000023 s3 : 0000000000000230 s4 : ffffffff0264dd40
| s5 : 000000000000003e s6 : ffffffff81401898 s7 : ffffffff0264d040
| s8 : ff60000083934e80 s9 : ffffffff825b7240 s10: ff60000083934de0
| s11: ff200000007a25b0 t3 : ffffffff0264d000 t4 : ffffffff0264d040
| t5 : 0000000000000017 t6 : ff60000083367eb8
| status: 0000000200000120 badaddr: 6b6b6b6b6b6b6b83 cause: 000000000000000d
| [<ffffffff8000c528>] process_accumulated_relocations+0x70/0x162
| [<ffffffff8000ca9a>] apply_relocate_add+0x142/0x37a
| [<ffffffff800e79f0>] load_module+0x1720/0x227c
| [<ffffffff800e8790>] init_module_from_file+0x82/0xba
| [<ffffffff800e8940>] __riscv_sys_finit_module+0x178/0x300
| [<ffffffff80ec9160>] do_trap_ecall_u+0xc6/0x142
| [<ffffffff80ed5cac>] ret_from_exception+0x0/0x64
| Code: 060d 3783 010d 3823 f804 3b83 018d 6384 8363 0a97 (a903) 0184
| ---[ end trace 0000000000000000 ]---

This series resolved the issue for me.

Tested-by: Björn Töpel <[email protected]>

2023-11-29 07:29:58

by Björn Töpel

[permalink] [raw]
Subject: Re: [PATCH v4 1/2] riscv: Safely remove entries from relocation list

Charlie Jenkins <[email protected]> writes:

> Use the safe versions of list and hlist iteration to safely remove
> entries from the module relocation lists. To allow mutliple threads to
> load modules concurrently, move relocation list pointers onto the stack
> rather than using global variables.
>
> Fixes: 8fd6c5142395 ("riscv: Add remaining module relocations")
> Reported-by: Ron Economos <[email protected]>
> Closes: https://lore.kernel.org/linux-riscv/[email protected]
> Signed-off-by: Charlie Jenkins <[email protected]>
> ---
> arch/riscv/kernel/module.c | 110 +++++++++++++++++++++++++++++++++------------
> 1 file changed, 82 insertions(+), 28 deletions(-)
>
> diff --git a/arch/riscv/kernel/module.c b/arch/riscv/kernel/module.c
> index 56a8c78e9e21..53593fe58cd8 100644
> --- a/arch/riscv/kernel/module.c
> +++ b/arch/riscv/kernel/module.c
> @@ -40,15 +40,6 @@ struct relocation_handlers {
> long buffer);
> };
>
> -unsigned int initialize_relocation_hashtable(unsigned int num_relocations);
> -void process_accumulated_relocations(struct module *me);
> -int add_relocation_to_accumulate(struct module *me, int type, void *location,
> - unsigned int hashtable_bits, Elf_Addr v);
> -
> -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)
> @@ -604,7 +595,10 @@ static const struct relocation_handlers reloc_handlers[] = {
> /* 192-255 nonstandard ABI extensions */
> };
>
> -void process_accumulated_relocations(struct module *me)
> +static void
> +process_accumulated_relocations(struct module *me,

Nit/breaks my workflow ;-): Don't bother if you're not respinning for
other reasons. The linebreak after return type makes it harder to grep
the code (and also is not in line with the layout with rest of this).


Björn

2023-12-04 20:31:39

by Lad, Prabhakar

[permalink] [raw]
Subject: Re: [PATCH v4 1/2] riscv: Safely remove entries from relocation list

On Mon, Nov 27, 2023 at 10:08 PM Charlie Jenkins <[email protected]> wrote:
>
> Use the safe versions of list and hlist iteration to safely remove
> entries from the module relocation lists. To allow mutliple threads to
> load modules concurrently, move relocation list pointers onto the stack
> rather than using global variables.
>
> Fixes: 8fd6c5142395 ("riscv: Add remaining module relocations")
> Reported-by: Ron Economos <[email protected]>
> Closes: https://lore.kernel.org/linux-riscv/[email protected]
> Signed-off-by: Charlie Jenkins <[email protected]>
> ---
> arch/riscv/kernel/module.c | 110 +++++++++++++++++++++++++++++++++------------
> 1 file changed, 82 insertions(+), 28 deletions(-)
>
This fixes issues seen on RZ/Five SMARC EVK.

Tested-by: Lad Prabhakar <[email protected]> #On
RZ/Five SMARC EVK

Cheers,
Prabhakar

> diff --git a/arch/riscv/kernel/module.c b/arch/riscv/kernel/module.c
> index 56a8c78e9e21..53593fe58cd8 100644
> --- a/arch/riscv/kernel/module.c
> +++ b/arch/riscv/kernel/module.c
> @@ -40,15 +40,6 @@ struct relocation_handlers {
> long buffer);
> };
>
> -unsigned int initialize_relocation_hashtable(unsigned int num_relocations);
> -void process_accumulated_relocations(struct module *me);
> -int add_relocation_to_accumulate(struct module *me, int type, void *location,
> - unsigned int hashtable_bits, Elf_Addr v);
> -
> -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)
> @@ -604,7 +595,10 @@ static const struct relocation_handlers reloc_handlers[] = {
> /* 192-255 nonstandard ABI extensions */
> };
>
> -void process_accumulated_relocations(struct module *me)
> +static void
> +process_accumulated_relocations(struct module *me,
> + struct hlist_head **relocation_hashtable,
> + struct list_head *used_buckets_list)
> {
> /*
> * Only ADD/SUB/SET/ULEB128 should end up here.
> @@ -624,18 +618,25 @@ void process_accumulated_relocations(struct module *me)
> * - Each relocation entry for a location address
> */
> struct used_bucket *bucket_iter;
> + struct used_bucket *bucket_iter_tmp;
> struct relocation_head *rel_head_iter;
> + struct hlist_node *rel_head_iter_tmp;
> struct relocation_entry *rel_entry_iter;
> + struct relocation_entry *rel_entry_iter_tmp;
> 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) {
> + list_for_each_entry_safe(bucket_iter, bucket_iter_tmp,
> + used_buckets_list, head) {
> + hlist_for_each_entry_safe(rel_head_iter, rel_head_iter_tmp,
> + bucket_iter->bucket, node) {
> buffer = 0;
> location = rel_head_iter->location;
> - list_for_each_entry(rel_entry_iter,
> - rel_head_iter->rel_entry, head) {
> + list_for_each_entry_safe(rel_entry_iter,
> + rel_entry_iter_tmp,
> + rel_head_iter->rel_entry,
> + head) {
> curr_type = rel_entry_iter->type;
> reloc_handlers[curr_type].reloc_handler(
> me, &buffer, rel_entry_iter->value);
> @@ -648,11 +649,14 @@ void process_accumulated_relocations(struct module *me)
> kfree(bucket_iter);
> }
>
> - kfree(relocation_hashtable);
> + kfree(*relocation_hashtable);
> }
>
> -int add_relocation_to_accumulate(struct module *me, int type, void *location,
> - unsigned int hashtable_bits, Elf_Addr v)
> +static int add_relocation_to_accumulate(struct module *me, int type,
> + void *location,
> + unsigned int hashtable_bits, Elf_Addr v,
> + struct hlist_head *relocation_hashtable,
> + struct list_head *used_buckets_list)
> {
> struct relocation_entry *entry;
> struct relocation_head *rel_head;
> @@ -661,6 +665,10 @@ int add_relocation_to_accumulate(struct module *me, int type, void *location,
> unsigned long hash;
>
> entry = kmalloc(sizeof(*entry), GFP_KERNEL);
> +
> + if (!entry)
> + return -ENOMEM;
> +
> INIT_LIST_HEAD(&entry->head);
> entry->type = type;
> entry->value = v;
> @@ -669,7 +677,10 @@ int add_relocation_to_accumulate(struct module *me, int type, void *location,
>
> current_head = &relocation_hashtable[hash];
>
> - /* Find matching location (if any) */
> + /*
> + * Search for the relocation_head for the relocations that happen at the
> + * provided location
> + */
> bool found = false;
> struct relocation_head *rel_head_iter;
>
> @@ -681,19 +692,45 @@ int add_relocation_to_accumulate(struct module *me, int type, void *location,
> }
> }
>
> + /*
> + * If there has not yet been any relocations at the provided location,
> + * create a relocation_head for that location and populate it with this
> + * relocation_entry.
> + */
> if (!found) {
> rel_head = kmalloc(sizeof(*rel_head), GFP_KERNEL);
> +
> + if (!rel_head) {
> + kfree(entry);
> + return -ENOMEM;
> + }
> +
> rel_head->rel_entry =
> kmalloc(sizeof(struct list_head), GFP_KERNEL);
> +
> + if (!rel_head->rel_entry) {
> + kfree(entry);
> + kfree(rel_head);
> + return -ENOMEM;
> + }
> +
> 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);
> +
> + if (!bucket) {
> + kfree(entry);
> + kfree(rel_head);
> + kfree(rel_head->rel_entry);
> + return -ENOMEM;
> + }
> +
> INIT_LIST_HEAD(&bucket->head);
> bucket->bucket = current_head;
> - list_add(&bucket->head, &used_buckets_list);
> + list_add(&bucket->head, used_buckets_list);
> }
> hlist_add_head(&rel_head->node, current_head);
> }
> @@ -704,7 +741,9 @@ int add_relocation_to_accumulate(struct module *me, int type, void *location,
> return 0;
> }
>
> -unsigned int initialize_relocation_hashtable(unsigned int num_relocations)
> +static unsigned int
> +initialize_relocation_hashtable(unsigned int num_relocations,
> + struct hlist_head **relocation_hashtable)
> {
> /* Can safely assume that bits is not greater than sizeof(long) */
> unsigned long hashtable_size = roundup_pow_of_two(num_relocations);
> @@ -720,12 +759,13 @@ unsigned int initialize_relocation_hashtable(unsigned int num_relocations)
>
> hashtable_size <<= should_double_size;
>
> - relocation_hashtable = kmalloc_array(hashtable_size,
> - sizeof(*relocation_hashtable),
> - GFP_KERNEL);
> - __hash_init(relocation_hashtable, hashtable_size);
> + *relocation_hashtable = kmalloc_array(hashtable_size,
> + sizeof(*relocation_hashtable),
> + GFP_KERNEL);
> + if (!*relocation_hashtable)
> + return -ENOMEM;
>
> - INIT_LIST_HEAD(&used_buckets_list);
> + __hash_init(*relocation_hashtable, hashtable_size);
>
> return hashtable_bits;
> }
> @@ -742,7 +782,17 @@ int apply_relocate_add(Elf_Shdr *sechdrs, const char *strtab,
> Elf_Addr v;
> int res;
> unsigned int num_relocations = sechdrs[relsec].sh_size / sizeof(*rel);
> - unsigned int hashtable_bits = initialize_relocation_hashtable(num_relocations);
> + struct hlist_head *relocation_hashtable;
> + struct list_head used_buckets_list;
> + unsigned int hashtable_bits;
> +
> + hashtable_bits = initialize_relocation_hashtable(num_relocations,
> + &relocation_hashtable);
> +
> + if (hashtable_bits < 0)
> + return hashtable_bits;
> +
> + INIT_LIST_HEAD(&used_buckets_list);
>
> pr_debug("Applying relocate section %u to %u\n", relsec,
> sechdrs[relsec].sh_info);
> @@ -823,14 +873,18 @@ int apply_relocate_add(Elf_Shdr *sechdrs, const char *strtab,
> }
>
> if (reloc_handlers[type].accumulate_handler)
> - res = add_relocation_to_accumulate(me, type, location, hashtable_bits, v);
> + res = add_relocation_to_accumulate(me, type, location,
> + hashtable_bits, v,
> + relocation_hashtable,
> + &used_buckets_list);
> else
> res = handler(me, location, v);
> if (res)
> return res;
> }
>
> - process_accumulated_relocations(me);
> + process_accumulated_relocations(me, &relocation_hashtable,
> + &used_buckets_list);
>
> return 0;
> }
>
> --
> 2.42.0
>
>
> _______________________________________________
> linux-riscv mailing list
> [email protected]
> http://lists.infradead.org/mailman/listinfo/linux-riscv

2023-12-04 20:32:49

by Lad, Prabhakar

[permalink] [raw]
Subject: Re: [PATCH v4 2/2] riscv: Correct type casting in module loading

On Mon, Nov 27, 2023 at 10:08 PM Charlie Jenkins <[email protected]> wrote:
>
> Use __le16 with le16_to_cpu.
>
> Fixes: 8fd6c5142395 ("riscv: Add remaining module relocations")
> Signed-off-by: Charlie Jenkins <[email protected]>
> Reviewed-by: Samuel Holland <[email protected]>
> Tested-by: Samuel Holland <[email protected]>
> ---
> arch/riscv/kernel/module.c | 4 ++--
> 1 file changed, 2 insertions(+), 2 deletions(-)
>
Tested-by: Lad Prabhakar <[email protected]> #On
RZ/Five SMARC EVK

Cheers,
Prabhakar

> diff --git a/arch/riscv/kernel/module.c b/arch/riscv/kernel/module.c
> index 53593fe58cd8..aac019ed63b1 100644
> --- a/arch/riscv/kernel/module.c
> +++ b/arch/riscv/kernel/module.c
> @@ -55,7 +55,7 @@ static bool riscv_insn_valid_32bit_offset(ptrdiff_t val)
>
> static int riscv_insn_rmw(void *location, u32 keep, u32 set)
> {
> - u16 *parcel = location;
> + __le16 *parcel = location;
> u32 insn = (u32)le16_to_cpu(parcel[0]) | (u32)le16_to_cpu(parcel[1]) << 16;
>
> insn &= keep;
> @@ -68,7 +68,7 @@ static int riscv_insn_rmw(void *location, u32 keep, u32 set)
>
> static int riscv_insn_rvc_rmw(void *location, u16 keep, u16 set)
> {
> - u16 *parcel = location;
> + __le16 *parcel = location;
> u16 insn = le16_to_cpu(*parcel);
>
> insn &= keep;
>
> --
> 2.42.0
>
>
> _______________________________________________
> linux-riscv mailing list
> [email protected]
> http://lists.infradead.org/mailman/listinfo/linux-riscv

Subject: Re: [PATCH v4 0/2] riscv: Fix issues with module loading

Hello:

This series was applied to riscv/linux.git (fixes)
by Palmer Dabbelt <[email protected]>:

On Mon, 27 Nov 2023 14:04:58 -0800 you wrote:
> Module loading did not account for multiple threads concurrently loading
> modules. This patch fixes that issue. There is also a small patch to fix
> the type of a __le16 variable.
>
> Signed-off-by: Charlie Jenkins <[email protected]>
> ---
> Changes in v4:
> - Make functions only used internally static
> - Free data structures on kmalloc failure (Andreas)
> - Link to v3: https://lore.kernel.org/r/[email protected]
>
> [...]

Here is the summary with links:
- [v4,1/2] riscv: Safely remove entries from relocation list
https://git.kernel.org/riscv/c/d8792a5734b0
- [v4,2/2] riscv: Correct type casting in module loading
https://git.kernel.org/riscv/c/4a92a87950c4

You are awesome, thank you!
--
Deet-doot-dot, I am a bot.
https://korg.docs.kernel.org/patchwork/pwbot.html