diff mbox series

[v2] arm64: module: PLT allowed even !RANDOM_BASE

Message ID 20231024010954.6768-1-quic_aiquny@quicinc.com (mailing list archive)
State New, archived
Headers show
Series [v2] arm64: module: PLT allowed even !RANDOM_BASE | expand

Commit Message

Aiqun Yu (Maria) Oct. 24, 2023, 1:09 a.m. UTC
Module PLT feature can be enabled even when RANDOM_BASE is disabled.
Break BLT entry counts of relocation types will make module plt entry
allocation fail and finally exec format error for even correct and plt
allocation available modules.

Signed-off-by: Maria Yu <quic_aiquny@quicinc.com>
---
 arch/arm64/kernel/module-plts.c | 6 ------
 1 file changed, 6 deletions(-)


base-commit: 05d3ef8bba77c1b5f98d941d8b2d4aeab8118ef1

Comments

Ard Biesheuvel Oct. 24, 2023, 8:25 a.m. UTC | #1
On Tue, 24 Oct 2023 at 03:10, Maria Yu <quic_aiquny@quicinc.com> wrote:
>
> Module PLT feature can be enabled even when RANDOM_BASE is disabled.
> Break BLT entry counts of relocation types will make module plt entry
> allocation fail and finally exec format error for even correct and plt
> allocation available modules.
>
> Signed-off-by: Maria Yu <quic_aiquny@quicinc.com>

Acked-by: Ard Biesheuvel <ardb@kernel.org>

> ---
>  arch/arm64/kernel/module-plts.c | 6 ------
>  1 file changed, 6 deletions(-)
>
> diff --git a/arch/arm64/kernel/module-plts.c b/arch/arm64/kernel/module-plts.c
> index bd69a4e7cd60..79200f21e123 100644
> --- a/arch/arm64/kernel/module-plts.c
> +++ b/arch/arm64/kernel/module-plts.c
> @@ -167,9 +167,6 @@ static unsigned int count_plts(Elf64_Sym *syms, Elf64_Rela *rela, int num,
>                 switch (ELF64_R_TYPE(rela[i].r_info)) {
>                 case R_AARCH64_JUMP26:
>                 case R_AARCH64_CALL26:
> -                       if (!IS_ENABLED(CONFIG_RANDOMIZE_BASE))
> -                               break;
> -
>                         /*
>                          * We only have to consider branch targets that resolve
>                          * to symbols that are defined in a different section.
> @@ -269,9 +266,6 @@ static int partition_branch_plt_relas(Elf64_Sym *syms, Elf64_Rela *rela,
>  {
>         int i = 0, j = numrels - 1;
>
> -       if (!IS_ENABLED(CONFIG_RANDOMIZE_BASE))
> -               return 0;
> -
>         while (i < j) {
>                 if (branch_rela_needs_plt(syms, &rela[i], dstidx))
>                         i++;
>
> base-commit: 05d3ef8bba77c1b5f98d941d8b2d4aeab8118ef1
> --
> 2.17.1
>
Mark Rutland Oct. 24, 2023, 10:13 a.m. UTC | #2
On Tue, Oct 24, 2023 at 09:09:54AM +0800, Maria Yu wrote:
> Module PLT feature can be enabled even when RANDOM_BASE is disabled.
> Break BLT entry counts of relocation types will make module plt entry
> allocation fail and finally exec format error for even correct and plt
> allocation available modules.

The patch itself looks good; it would be better if we could make the commit
message explicit that this is a fix. Could we please replace that with:

| arm64: module: fix PLT counting when CONFIG_RANDOMIZE_BASE=n
| 
| The counting of module PLTs has been broken when CONFIG_RANDOMIZE_BASE=n
| since commit:
| 
|   3e35d303ab7d22c4 ("arm64: module: rework module VA range selection")
| 
| Prior to that commit, when CONFIG_RANDOMIZE_BASE=n, the kernel image and
| all modules were placed within a 128M region, and no PLTs were necessary
| for B or BL. Hence count_plts() and partition_branch_plt_relas() skipped
| handling B and BL when CONFIG_RANDOMIZE_BASE=n.
| 
| After that commit, modules can be placed anywhere within a 2G window
| regardless of CONFIG_RANDOMIZE_BASE, and hence PLTs may be necessary for
| B and BL even when CONFIG_RANDOMIZE_BASE=n. Unfortunately that commit
| failed to update count_plts() and partition_branch_plt_relas()
| accordingly.
| 
| Due to this, module_emit_plt_entry() may fail if an insufficient number
| of PLT entries have been reserved, resulting in modules failing to load
| with -ENOEXEC.
|
| Fix this by counting PLTs regardless of CONFIG_RANDOMIZE_BASE in 
| count_plts() and partition_branch_plt_relas().

... and add a fixes tag:

Fixes: 3e35d303ab7d22c4 ("arm64: module: rework module VA range selection")

With that:

Reviewed-by: Mark Rutland <mark.rutland@arm.com>

Mark.

> 
> Signed-off-by: Maria Yu <quic_aiquny@quicinc.com>
> ---
>  arch/arm64/kernel/module-plts.c | 6 ------
>  1 file changed, 6 deletions(-)
> 
> diff --git a/arch/arm64/kernel/module-plts.c b/arch/arm64/kernel/module-plts.c
> index bd69a4e7cd60..79200f21e123 100644
> --- a/arch/arm64/kernel/module-plts.c
> +++ b/arch/arm64/kernel/module-plts.c
> @@ -167,9 +167,6 @@ static unsigned int count_plts(Elf64_Sym *syms, Elf64_Rela *rela, int num,
>  		switch (ELF64_R_TYPE(rela[i].r_info)) {
>  		case R_AARCH64_JUMP26:
>  		case R_AARCH64_CALL26:
> -			if (!IS_ENABLED(CONFIG_RANDOMIZE_BASE))
> -				break;
> -
>  			/*
>  			 * We only have to consider branch targets that resolve
>  			 * to symbols that are defined in a different section.
> @@ -269,9 +266,6 @@ static int partition_branch_plt_relas(Elf64_Sym *syms, Elf64_Rela *rela,
>  {
>  	int i = 0, j = numrels - 1;
>  
> -	if (!IS_ENABLED(CONFIG_RANDOMIZE_BASE))
> -		return 0;
> -
>  	while (i < j) {
>  		if (branch_rela_needs_plt(syms, &rela[i], dstidx))
>  			i++;
> 
> base-commit: 05d3ef8bba77c1b5f98d941d8b2d4aeab8118ef1
> -- 
> 2.17.1
>
Catalin Marinas Oct. 24, 2023, 4:30 p.m. UTC | #3
On Tue, 24 Oct 2023 09:09:54 +0800, Maria Yu wrote:
> Module PLT feature can be enabled even when RANDOM_BASE is disabled.
> Break BLT entry counts of relocation types will make module plt entry
> allocation fail and finally exec format error for even correct and plt
> allocation available modules.
> 
> 

Applied to arm64 (for-next/misc) with Mark's commit log. Thanks!

[1/1] arm64: module: PLT allowed even !RANDOM_BASE
      https://git.kernel.org/arm64/c/d35686444fc8
diff mbox series

Patch

diff --git a/arch/arm64/kernel/module-plts.c b/arch/arm64/kernel/module-plts.c
index bd69a4e7cd60..79200f21e123 100644
--- a/arch/arm64/kernel/module-plts.c
+++ b/arch/arm64/kernel/module-plts.c
@@ -167,9 +167,6 @@  static unsigned int count_plts(Elf64_Sym *syms, Elf64_Rela *rela, int num,
 		switch (ELF64_R_TYPE(rela[i].r_info)) {
 		case R_AARCH64_JUMP26:
 		case R_AARCH64_CALL26:
-			if (!IS_ENABLED(CONFIG_RANDOMIZE_BASE))
-				break;
-
 			/*
 			 * We only have to consider branch targets that resolve
 			 * to symbols that are defined in a different section.
@@ -269,9 +266,6 @@  static int partition_branch_plt_relas(Elf64_Sym *syms, Elf64_Rela *rela,
 {
 	int i = 0, j = numrels - 1;
 
-	if (!IS_ENABLED(CONFIG_RANDOMIZE_BASE))
-		return 0;
-
 	while (i < j) {
 		if (branch_rela_needs_plt(syms, &rela[i], dstidx))
 			i++;