2024-01-04 19:43:22

by Charlie Jenkins

[permalink] [raw]
Subject: [PATCH v3 0/3] riscv: modules: Fix module loading error handling

When modules are loaded while there is not ample allocatable memory,
there was previously not proper error handling. This series fixes a
use-after-free error and a different issue that caused a non graceful
exit after memory was not properly allocated.

Signed-off-by: Charlie Jenkins <[email protected]>
---
Changes in v3:
- Drop patch using do-while
- Link to v2: https://lore.kernel.org/r/[email protected]

Changes in v2:
- Split changes across multiple patches
- Link to v1: https://lore.kernel.org/r/[email protected]

---
Charlie Jenkins (3):
riscv: Fix module loading free order
riscv: Correctly free relocation hashtable on error
riscv: Fix relocation_hashtable size

arch/riscv/kernel/module.c | 14 +++++++++-----
1 file changed, 9 insertions(+), 5 deletions(-)
---
base-commit: a39b6ac3781d46ba18193c9dbb2110f31e9bffe9
change-id: 20231213-module_loading_fix-3ac6d4ea8129
--
- Charlie



2024-01-04 19:43:32

by Charlie Jenkins

[permalink] [raw]
Subject: [PATCH v3 1/3] riscv: Fix module loading free order

Reverse order of kfree calls to resolve use-after-free error.

Signed-off-by: Charlie Jenkins <[email protected]>
Fixes: d8792a5734b0 ("riscv: Safely remove entries from relocation list")
Reported-by: kernel test robot <[email protected]>
Reported-by: Dan Carpenter <[email protected]>
Closes: https://lore.kernel.org/r/[email protected]/
Reported-by: kernel test robot <[email protected]>
Reported-by: Julia Lawall <[email protected]>
Closes: https://lore.kernel.org/r/[email protected]/
---
arch/riscv/kernel/module.c | 2 +-
1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/arch/riscv/kernel/module.c b/arch/riscv/kernel/module.c
index aac019ed63b1..21c7a773a8ef 100644
--- a/arch/riscv/kernel/module.c
+++ b/arch/riscv/kernel/module.c
@@ -723,8 +723,8 @@ static int add_relocation_to_accumulate(struct module *me, int type,

if (!bucket) {
kfree(entry);
- kfree(rel_head);
kfree(rel_head->rel_entry);
+ kfree(rel_head);
return -ENOMEM;
}


--
2.43.0


2024-01-04 19:44:01

by Charlie Jenkins

[permalink] [raw]
Subject: [PATCH v3 3/3] riscv: Fix relocation_hashtable size

A second dereference is needed to get the accurate size of the
relocation_hashtable.

Signed-off-by: Charlie Jenkins <[email protected]>
Fixes: d8792a5734b0 ("riscv: Safely remove entries from relocation list")
Reported-by: kernel test robot <[email protected]>
Reported-by: Julia Lawall <[email protected]>
Closes: https://lore.kernel.org/r/[email protected]/
---
arch/riscv/kernel/module.c | 2 +-
1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/arch/riscv/kernel/module.c b/arch/riscv/kernel/module.c
index 32743180e8ef..ceb0adb38715 100644
--- a/arch/riscv/kernel/module.c
+++ b/arch/riscv/kernel/module.c
@@ -764,7 +764,7 @@ initialize_relocation_hashtable(unsigned int num_relocations,
hashtable_size <<= should_double_size;

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

--
2.43.0


2024-01-04 19:44:01

by Charlie Jenkins

[permalink] [raw]
Subject: [PATCH v3 2/3] riscv: Correctly free relocation hashtable on error

When there is not enough allocatable memory for the relocation
hashtable, module loading should exit gracefully. Previously, this was
attempted to be accomplished by checking if an unsigned number is less
than zero which does not work. Instead have the caller check if the
hashtable was correctly allocated and add a comment explaining that
hashtable_bits that is 0 is valid.

Signed-off-by: Charlie Jenkins <[email protected]>
Fixes: d8792a5734b0 ("riscv: Safely remove entries from relocation list")
Reported-by: kernel test robot <[email protected]>
Reported-by: Dan Carpenter <[email protected]>
Closes: https://lore.kernel.org/r/[email protected]/
Reported-by: kernel test robot <[email protected]>
Reported-by: Julia Lawall <[email protected]>
Closes: https://lore.kernel.org/r/[email protected]/
---
arch/riscv/kernel/module.c | 10 +++++++---
1 file changed, 7 insertions(+), 3 deletions(-)

diff --git a/arch/riscv/kernel/module.c b/arch/riscv/kernel/module.c
index 21c7a773a8ef..32743180e8ef 100644
--- a/arch/riscv/kernel/module.c
+++ b/arch/riscv/kernel/module.c
@@ -747,6 +747,10 @@ 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);
+ /*
+ * When hashtable_size == 1, hashtable_bits == 0.
+ * This is valid because the hashing algorithm returns 0 in this case.
+ */
unsigned int hashtable_bits = ilog2(hashtable_size);

/*
@@ -763,7 +767,7 @@ initialize_relocation_hashtable(unsigned int num_relocations,
sizeof(*relocation_hashtable),
GFP_KERNEL);
if (!*relocation_hashtable)
- return -ENOMEM;
+ return 0;

__hash_init(*relocation_hashtable, hashtable_size);

@@ -789,8 +793,8 @@ int apply_relocate_add(Elf_Shdr *sechdrs, const char *strtab,
hashtable_bits = initialize_relocation_hashtable(num_relocations,
&relocation_hashtable);

- if (hashtable_bits < 0)
- return hashtable_bits;
+ if (!relocation_hashtable)
+ return -ENOMEM;

INIT_LIST_HEAD(&used_buckets_list);


--
2.43.0


2024-01-05 07:38:55

by Dan Carpenter

[permalink] [raw]
Subject: Re: [PATCH v3 0/3] riscv: modules: Fix module loading error handling

On Thu, Jan 04, 2024 at 11:42:46AM -0800, Charlie Jenkins wrote:
> When modules are loaded while there is not ample allocatable memory,
> there was previously not proper error handling. This series fixes a
> use-after-free error and a different issue that caused a non graceful
> exit after memory was not properly allocated.
>
> Signed-off-by: Charlie Jenkins <[email protected]>

Thanks!

Reviewed-by: Dan Carpenter <[email protected]>

regards,
dan carpenter


Subject: Re: [PATCH v3 0/3] riscv: modules: Fix module loading error handling

Hello:

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

On Thu, 04 Jan 2024 11:42:46 -0800 you wrote:
> When modules are loaded while there is not ample allocatable memory,
> there was previously not proper error handling. This series fixes a
> use-after-free error and a different issue that caused a non graceful
> exit after memory was not properly allocated.
>
> Signed-off-by: Charlie Jenkins <[email protected]>
>
> [...]

Here is the summary with links:
- [v3,1/3] riscv: Fix module loading free order
https://git.kernel.org/riscv/c/78996eee79eb
- [v3,2/3] riscv: Correctly free relocation hashtable on error
https://git.kernel.org/riscv/c/4b38b36bfbd8
- [v3,3/3] riscv: Fix relocation_hashtable size
https://git.kernel.org/riscv/c/a35551c7244d

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