* [PATCH] arm64: module: PLT allowed even !RANDOM_BASE @ 2023-10-23 7:57 Maria Yu 2023-10-23 8:08 ` Arnd Bergmann 0 siblings, 1 reply; 4+ messages in thread From: Maria Yu @ 2023-10-23 7:57 UTC (permalink / raw) To: catalin.marinas, will, arnd Cc: Maria Yu, linux-arm-kernel, linux-kernel, kernel, linux-arm-msm 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 | 3 --- 1 file changed, 3 deletions(-) diff --git a/arch/arm64/kernel/module-plts.c b/arch/arm64/kernel/module-plts.c index bd69a4e7cd60..21a67d52d7a0 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. base-commit: 05d3ef8bba77c1b5f98d941d8b2d4aeab8118ef1 -- 2.17.1 _______________________________________________ linux-arm-kernel mailing list linux-arm-kernel@lists.infradead.org http://lists.infradead.org/mailman/listinfo/linux-arm-kernel ^ permalink raw reply related [flat|nested] 4+ messages in thread
* Re: [PATCH] arm64: module: PLT allowed even !RANDOM_BASE 2023-10-23 7:57 [PATCH] arm64: module: PLT allowed even !RANDOM_BASE Maria Yu @ 2023-10-23 8:08 ` Arnd Bergmann 2023-10-23 9:02 ` Mark Rutland 0 siblings, 1 reply; 4+ messages in thread From: Arnd Bergmann @ 2023-10-23 8:08 UTC (permalink / raw) To: Maria Yu, Catalin Marinas, Will Deacon Cc: linux-arm-kernel, linux-kernel, kernel, linux-arm-msm, Ard Biesheuvel On Mon, Oct 23, 2023, at 09:57, 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. > > Signed-off-by: Maria Yu <quic_aiquny@quicinc.com> Adding Ard Biesheuvel to Cc, as he added the check in commit a257e02579e42 ("arm64/kernel: don't ban ADRP to work around Cortex-A53 erratum #843419") > arch/arm64/kernel/module-plts.c | 3 --- > 1 file changed, 3 deletions(-) > > diff --git a/arch/arm64/kernel/module-plts.c > b/arch/arm64/kernel/module-plts.c > index bd69a4e7cd60..21a67d52d7a0 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. I see there are two such checks (in partition_branch_plt_relas() and in count_plts()), can you explain in more detail how you concluded that one of them is correct but the other one is not? Arnd _______________________________________________ linux-arm-kernel mailing list linux-arm-kernel@lists.infradead.org http://lists.infradead.org/mailman/listinfo/linux-arm-kernel ^ permalink raw reply [flat|nested] 4+ messages in thread
* Re: [PATCH] arm64: module: PLT allowed even !RANDOM_BASE 2023-10-23 8:08 ` Arnd Bergmann @ 2023-10-23 9:02 ` Mark Rutland 2023-10-23 9:49 ` Aiqun(Maria) Yu 0 siblings, 1 reply; 4+ messages in thread From: Mark Rutland @ 2023-10-23 9:02 UTC (permalink / raw) To: Arnd Bergmann Cc: Maria Yu, Catalin Marinas, Will Deacon, linux-arm-kernel, linux-kernel, kernel, linux-arm-msm, Ard Biesheuvel On Mon, Oct 23, 2023 at 10:08:33AM +0200, Arnd Bergmann wrote: > On Mon, Oct 23, 2023, at 09:57, 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. Has an actual problem been seen in practice, or was this found by looking at the code? > > > > Signed-off-by: Maria Yu <quic_aiquny@quicinc.com> > > Adding Ard Biesheuvel to Cc, as he added the check in commit > a257e02579e42 ("arm64/kernel: don't ban ADRP to work around > Cortex-A53 erratum #843419") I think that the actual mistake is in commit: 3e35d303ab7d22c4 ("arm64: module: rework module VA range selection") Prior to that commit, when CONFIG_RANDOMIZE_BASE=n all modules and code had to be within 128M of each other, and so there were no PLTs necessary for B/BL. After that commit we can have a 2G module range regardless of CONFIG_RANDOMIZE_BASE, and PLTs may be necessary for B/BL. We should have removed the check for !CONFIG_RANDOMIZE_BASE as part of that. > > arch/arm64/kernel/module-plts.c | 3 --- > > 1 file changed, 3 deletions(-) > > > > diff --git a/arch/arm64/kernel/module-plts.c > > b/arch/arm64/kernel/module-plts.c > > index bd69a4e7cd60..21a67d52d7a0 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. > > I see there are two such checks (in partition_branch_plt_relas() > and in count_plts()), can you explain in more detail how you > concluded that one of them is correct but the other one is not? I believe that the one in partition_branch_plt_relas() needs to go too; that's just a minor optimization for the case where there shouldn't be any PLTs for B/BL, and it no longer holds after the module VA range rework. That was introduced in commit: d4e0340919fb9190 ("arm64/module: Optimize module load time by optimizing PLT counting") Thanks, Mark. _______________________________________________ linux-arm-kernel mailing list linux-arm-kernel@lists.infradead.org http://lists.infradead.org/mailman/listinfo/linux-arm-kernel ^ permalink raw reply [flat|nested] 4+ messages in thread
* Re: [PATCH] arm64: module: PLT allowed even !RANDOM_BASE 2023-10-23 9:02 ` Mark Rutland @ 2023-10-23 9:49 ` Aiqun(Maria) Yu 0 siblings, 0 replies; 4+ messages in thread From: Aiqun(Maria) Yu @ 2023-10-23 9:49 UTC (permalink / raw) To: Mark Rutland, Arnd Bergmann Cc: Catalin Marinas, Will Deacon, linux-arm-kernel, linux-kernel, kernel, linux-arm-msm, Ard Biesheuvel On 10/23/2023 5:02 PM, Mark Rutland wrote: > On Mon, Oct 23, 2023 at 10:08:33AM +0200, Arnd Bergmann wrote: >> On Mon, Oct 23, 2023, at 09:57, 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. > > Has an actual problem been seen in practice, or was this found by looking at > the code? I've encounter an actual problem when disalbe CONFIG_RADOM_BASE and the kernel module have the exec format error issue. > >>> >>> Signed-off-by: Maria Yu <quic_aiquny@quicinc.com> >> >> Adding Ard Biesheuvel to Cc, as he added the check in commit >> a257e02579e42 ("arm64/kernel: don't ban ADRP to work around >> Cortex-A53 erratum #843419") Thx for adding Ard. Will keep him in next patchset as well. > > I think that the actual mistake is in commit: > > 3e35d303ab7d22c4 ("arm64: module: rework module VA range selection") > > Prior to that commit, when CONFIG_RANDOMIZE_BASE=n all modules and code had to > be within 128M of each other, and so there were no PLTs necessary for B/BL. > After that commit we can have a 2G module range regardless of > CONFIG_RANDOMIZE_BASE, and PLTs may be necessary for B/BL. > > We should have removed the check for !CONFIG_RANDOMIZE_BASE as part of that. Agree with you. > >>> arch/arm64/kernel/module-plts.c | 3 --- >>> 1 file changed, 3 deletions(-) >>> >>> diff --git a/arch/arm64/kernel/module-plts.c >>> b/arch/arm64/kernel/module-plts.c >>> index bd69a4e7cd60..21a67d52d7a0 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. >> >> I see there are two such checks (in partition_branch_plt_relas() >> and in count_plts()), can you explain in more detail how you >> concluded that one of them is correct but the other one is not? > > I believe that the one in partition_branch_plt_relas() needs to go too; that's > just a minor optimization for the case where there shouldn't be any PLTs for > B/BL, and it no longer holds after the module VA range rework. > > That was introduced in commit: > > d4e0340919fb9190 ("arm64/module: Optimize module load time by optimizing PLT counting") The functionality is the same from my try with plt allocated kernel modules. While the PLT entry can be dramatically reduced from ~50000 to ~500 after fix in partition_branch_plt_relas. I will send out the second patchset with fix in partition_branch_plt_relas (remove check of CONFIG_RANDOMIZE_BASE) tomorrow if no more other comments today. > > Thanks, > Mark. -- Thx and BRs, Aiqun(Maria) Yu _______________________________________________ linux-arm-kernel mailing list linux-arm-kernel@lists.infradead.org http://lists.infradead.org/mailman/listinfo/linux-arm-kernel ^ permalink raw reply [flat|nested] 4+ messages in thread
end of thread, other threads:[~2023-10-23 9:50 UTC | newest] Thread overview: 4+ messages (download: mbox.gz follow: Atom feed -- links below jump to the message on this page -- 2023-10-23 7:57 [PATCH] arm64: module: PLT allowed even !RANDOM_BASE Maria Yu 2023-10-23 8:08 ` Arnd Bergmann 2023-10-23 9:02 ` Mark Rutland 2023-10-23 9:49 ` Aiqun(Maria) Yu
This is a public inbox, see mirroring instructions for how to clone and mirror all data and code used for this inbox; as well as URLs for NNTP newsgroup(s).