lkml.org 
[lkml]   [2023]   [Oct]   [23]   [last100]   RSS Feed
Views: [wrap][no wrap]   [headers]  [forward] 
 
Messages in this thread
/
Date
From
SubjectRe: [PATCH] arm64: module: PLT allowed even !RANDOM_BASE
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.

\
 
 \ /
  Last update: 2023-10-23 11:03    [W:0.046 / U:0.152 seconds]
©2003-2020 Jasper Spaans|hosted at Digital Ocean and TransIP|Read the blog|Advertise on this site