Re: [PATCH] arm64/module-plts: Consider the special case where plt_max_entries is 0

Re: [PATCH] arm64/module-plts: Consider the special case where plt_max_entries is 0

[Will Deacon]

On Tue, Jul 14, 2020 at 08:48:11AM +0000, 彭浩(Richard) wrote:
> On Thu, Jul 09, 2020 at 07:18:01AM +0000,Peng Hao(Richard) wrote:
> > On Thu, 9 Jul 2020 at 09:50, Peng Hao(Richard) <richard.peng@oppo.com> wrote:
> > >> >Apparently, you are hitting a R_AARCH64_JUMP26 or R_AARCH64_CALL26
> > >> >relocation that operates on a b or bl instruction that is more than
> > >> >128 megabytes away from its target.
> > >> >
> > >> My understanding is that a module that calls functions that are not part of the module will use PLT.
> > >> Plt_max_entries =0 May occur if a module does not depend on other module functions.
> > >>
> > >
> > >A PLT slot is allocated for each b or bl instruction that refers to a
> > >symbol that lives in a different section, either of the same module
> > > (e.g., bl in .init calling into .text), of another module, or of the
> > >core kernel.
> > >
> > >I don't see how you end up with plt_max_entries in this case, though.
> > if a module does not depend on other module functions, PLT entries in the module is equal to 0.
> 
> >This brings me back to my earlier question: if there are no PLT entries in
> >the module, then count_plts() will not find any R_AARCH64_JUMP26 or
> >R_AARCH64_CALL26 relocations that require PLTs and will therefore return 0.
> >The absence of these relocations means that module_emit_plt_entry() will not
> >be called by apply_relocate_add(), and so your patch should have no effect.
> 1.The module in question is the calling function from core kernel.( Ib_core.ko triggered the warning multiple times).
> 2. There are multiple threads loading IB_core.ko
> [   73.388931]  ###cpu=33, name=ib_core, core_plts=0, init_plts=0  
> [   73.402102]  #### cpu=33,pid=2297,name=ib_core, module_emit_plt_entry:plt_num_entries=1, plt_max_entries=0 (warning)
> [   73.439391]  ###cpu=24, name=ib_core, core_plts=0, init_plts=0  
> [   73.448617]  ###cpu=4, name=ib_core, core_plts=0, init_plts=0  
> [   73.547535]  ###cpu=221, name=ib_core, core_plts=0, init_plts=0  
> [   75.198075]  #### cpu=24,pid=2336,name=ib_core, module_emit_plt_entry:plt_num_entries=1, plt_max_entries=0 (warning)
> [   75.489496]  #### cpu=4,pid=2344,name=ib_core, module_emit_plt_entry:plt_num_entries=1, plt_max_entries=0(warning)
> 
> I don't understand why count_plts returns 0 when CONFIG_RANDOMIZE_BASE=n for R_AARCH64_JUMP26 and R_AARCH64_CALL26.
> 
> 3. Set CONFIG_ARM64_MODULE_PLTS=y and restart the server several times without triggering this warning.

Can you provide a means for us to reproduce this failure with an upstream
kernel, please? I really can't tell what's going on from the report. If I
can reproduce the problem locally, then I'm happy to take a look.

Thanks,

Will

_______________________________________________
linux-arm-kernel mailing list
linux-arm-kernel@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-arm-kernel

Re: [PATCH] arm64/module-plts: Consider the special case where plt_max_entries is 0

[Will Deacon]

On Thu, Jul 09, 2020 at 07:18:01AM +0000, 彭浩(Richard) wrote:
> On Thu, 9 Jul 2020 at 09:50, 彭浩(Richard) <richard.peng@oppo.com> wrote:
> >> >Apparently, you are hitting a R_AARCH64_JUMP26 or R_AARCH64_CALL26
> >> >relocation that operates on a b or bl instruction that is more than
> >> >128 megabytes away from its target.
> >> >
> >> My understanding is that a module that calls functions that are not part of the module will use PLT.
> >> Plt_max_entries =0 May occur if a module does not depend on other module functions.
> >>
> >
> >A PLT slot is allocated for each b or bl instruction that refers to a
> >symbol that lives in a different section, either of the same module
> > (e.g., bl in .init calling into .text), of another module, or of the
> >core kernel.
> >
> >I don't see how you end up with plt_max_entries in this case, though.
> if a module does not depend on other module functions, PLT entries in the module is equal to 0.

This brings me back to my earlier question: if there are no PLT entries in
the module, then count_plts() will not find any R_AARCH64_JUMP26 or
R_AARCH64_CALL26 relocations that require PLTs and will therefore return 0.
The absence of these relocations means that module_emit_plt_entry() will not
be called by apply_relocate_add(), and so your patch should have no effect.

You seem to be saying that module_emit_plt_entry() _is_ being called,
despite count_plts() returning 0. One way that can happen is if PLTs are
needed for branches within a single, very large text section, but you also
say that's not the case.

So I think we need more information from you so that we can either reproduce
this ourselves, or better understand where things are going wrong.

Finally, you said that your kernel is "5.6.0-rc3+". Are you able to
reproduce with mainline (5.8-rc4)?

Will

P.S. whenever you reply, the mail threading breaks :(

_______________________________________________
linux-arm-kernel mailing list
linux-arm-kernel@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-arm-kernel

Re: [PATCH] arm64/module-plts: Consider the special case where plt_max_entries is 0

[Ard Biesheuvel]

On Thu, 9 Jul 2020 at 09:50, 彭浩(Richard) <richard.peng@oppo.com> wrote:
>
> On Wed, 8 Jul 2020 at 13:03, 彭浩(Richard) <richard.peng@oppo.com> wrote:
> >>
> >>
> >> On Tue, Jul 07, 2020 at 07:46:08AM -0400, Peng Hao wrote:
> >> >> If plt_max_entries is 0, a warning is triggered.
> >> >> WARNING: CPU: 200 PID: 3000 at arch/arm64/kernel/module-plts.c:97 module_emit_plt_entry+0xa4/0x150
> >> >
> >> > Which kernel are you seeing this with? There is a PLT-related change in
> >> > for-next/core, and I'd like to rule if out if possible.
> >> >
> >> 5.6.0-rc3+
> >> >> Signed-off-by: Peng Hao <richard.peng@oppo.com>
> >> >> ---
> >> >>  arch/arm64/kernel/module-plts.c | 3 ++-
> >> >>  1 file changed, 2 insertions(+), 1 deletion(-)
> >> >>
> >> >> diff --git a/arch/arm64/kernel/module-plts.c b/arch/arm64/kernel/module-plts.c
> >> >> index 65b08a74aec6..1868c9ac13f2 100644
> >> >> --- a/arch/arm64/kernel/module-plts.c
> >> >> +++ b/arch/arm64/kernel/module-plts.c
> >> >> @@ -79,7 +79,8 @@ u64 module_emit_plt_entry(struct module *mod, Elf64_Shdr *sechdrs,
> >> >>      int i = pltsec->plt_num_entries;
> >> >>      int j = i - 1;
> >> >>      u64 val = sym->st_value + rela->r_addend;
> >> >> -
> >> >> +    if (pltsec->plt_max_entries == 0)
> >> >> +            return 0;
> >> >
> >> >Hmm, but if there aren't any PLTs then how do we end up here?
> >> >
> >> We also returned 0 when warning was triggered.
> >
> >That doesn't really answer the question.
> >
> >Apparently, you are hitting a R_AARCH64_JUMP26 or R_AARCH64_CALL26
> >relocation that operates on a b or bl instruction that is more than
> >128 megabytes away from its target.
> >
> My understanding is that a module that calls functions that are not part of the module will use PLT.
> Plt_max_entries =0 May occur if a module does not depend on other module functions.
>

A PLT slot is allocated for each b or bl instruction that refers to a
symbol that lives in a different section, either of the same module
(e.g., bl in .init calling into .text), of another module, or of the
core kernel.

I don't see how you end up with plt_max_entries in this case, though.
Are you sure you have CONFIG_RANDOMIZE_BASE enabled?

> >In module_frob_arch_sections(), we count all such relocations that
> >point to other sections, and allocate a PLT slot for each (and update
> >plt_max_entries) accordingly. So this means that the relocation in
> >question was disregarded, and this could happen for only two reasons:
> >- the branch instruction and its target are both in the same section,
> >in which case this section is *really* large,
> >- CONFIG_RANDOMIZE_BASE is disabled, but you are still ending up in a
> >situation where the modules are really far away from the core kernel
> >or from other modules.
> >
> >Do you have a lot of [large] modules loaded when this happens?
> I don’t think I have [large] modules.  I'll trace which module caused this warning.

Yes please.

_______________________________________________
linux-arm-kernel mailing list
linux-arm-kernel@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-arm-kernel

Re: [PATCH] arm64/module-plts: Consider the special case where plt_max_entries is 0

[Ard Biesheuvel]

On Wed, 8 Jul 2020 at 13:03, 彭浩(Richard) <richard.peng@oppo.com> wrote:
>
>
> On Tue, Jul 07, 2020 at 07:46:08AM -0400, Peng Hao wrote:
> >> If plt_max_entries is 0, a warning is triggered.
> >> WARNING: CPU: 200 PID: 3000 at arch/arm64/kernel/module-plts.c:97 module_emit_plt_entry+0xa4/0x150
> >
> > Which kernel are you seeing this with? There is a PLT-related change in
> > for-next/core, and I'd like to rule if out if possible.
> >
> 5.6.0-rc3+
> >> Signed-off-by: Peng Hao <richard.peng@oppo.com>
> >> ---
> >>  arch/arm64/kernel/module-plts.c | 3 ++-
> >>  1 file changed, 2 insertions(+), 1 deletion(-)
> >>
> >> diff --git a/arch/arm64/kernel/module-plts.c b/arch/arm64/kernel/module-plts.c
> >> index 65b08a74aec6..1868c9ac13f2 100644
> >> --- a/arch/arm64/kernel/module-plts.c
> >> +++ b/arch/arm64/kernel/module-plts.c
> >> @@ -79,7 +79,8 @@ u64 module_emit_plt_entry(struct module *mod, Elf64_Shdr *sechdrs,
> >>      int i = pltsec->plt_num_entries;
> >>      int j = i - 1;
> >>      u64 val = sym->st_value + rela->r_addend;
> >> -
> >> +    if (pltsec->plt_max_entries == 0)
> >> +            return 0;
> >
> >Hmm, but if there aren't any PLTs then how do we end up here?
> >
> We also returned 0 when warning was triggered.

That doesn't really answer the question.

Apparently, you are hitting a R_AARCH64_JUMP26 or R_AARCH64_CALL26
relocation that operates on a b or bl instruction that is more than
128 megabytes away from its target.

In module_frob_arch_sections(), we count all such relocations that
point to other sections, and allocate a PLT slot for each (and update
plt_max_entries) accordingly. So this means that the relocation in
question was disregarded, and this could happen for only two reasons:
- the branch instruction and its target are both in the same section,
in which case this section is *really* large,
- CONFIG_RANDOMIZE_BASE is disabled, but you are still ending up in a
situation where the modules are really far away from the core kernel
or from other modules.

Do you have a lot of [large] modules loaded when this happens?

_______________________________________________
linux-arm-kernel mailing list
linux-arm-kernel@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-arm-kernel

[PATCH] arm64/module-plts: Consider the special case where plt_max_entries is 0

[Peng Hao]

If plt_max_entries is 0, a warning is triggered.
WARNING: CPU: 200 PID: 3000 at arch/arm64/kernel/module-plts.c:97 module_emit_plt_entry+0xa4/0x150

Signed-off-by: Peng Hao <richard.peng@oppo.com>
---
 arch/arm64/kernel/module-plts.c | 3 ++-
 1 file changed, 2 insertions(+), 1 deletion(-)

diff --git a/arch/arm64/kernel/module-plts.c b/arch/arm64/kernel/module-plts.c
index 65b08a74aec6..1868c9ac13f2 100644
--- a/arch/arm64/kernel/module-plts.c
+++ b/arch/arm64/kernel/module-plts.c
@@ -79,7 +79,8 @@ u64 module_emit_plt_entry(struct module *mod, Elf64_Shdr *sechdrs,
 	int i = pltsec->plt_num_entries;
 	int j = i - 1;
 	u64 val = sym->st_value + rela->r_addend;
-
+	if (pltsec->plt_max_entries == 0)
+		return 0;
 	if (is_forbidden_offset_for_adrp(&plt[i].adrp))
 		i++;
 
-- 
2.18.4


_______________________________________________
linux-arm-kernel mailing list
linux-arm-kernel@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-arm-kernel

Re: [PATCH] arm64/module-plts: Consider the special case where plt_max_entries is 0

[Will Deacon]

[+Ard]

On Tue, Jul 07, 2020 at 07:46:08AM -0400, Peng Hao wrote:
> If plt_max_entries is 0, a warning is triggered.
> WARNING: CPU: 200 PID: 3000 at arch/arm64/kernel/module-plts.c:97 module_emit_plt_entry+0xa4/0x150

Which kernel are you seeing this with? There is a PLT-related change in
for-next/core, and I'd like to rule if out if possible.

> Signed-off-by: Peng Hao <richard.peng@oppo.com>
> ---
>  arch/arm64/kernel/module-plts.c | 3 ++-
>  1 file changed, 2 insertions(+), 1 deletion(-)
> 
> diff --git a/arch/arm64/kernel/module-plts.c b/arch/arm64/kernel/module-plts.c
> index 65b08a74aec6..1868c9ac13f2 100644
> --- a/arch/arm64/kernel/module-plts.c
> +++ b/arch/arm64/kernel/module-plts.c
> @@ -79,7 +79,8 @@ u64 module_emit_plt_entry(struct module *mod, Elf64_Shdr *sechdrs,
>  	int i = pltsec->plt_num_entries;
>  	int j = i - 1;
>  	u64 val = sym->st_value + rela->r_addend;
> -
> +	if (pltsec->plt_max_entries == 0)
> +		return 0;

Hmm, but if there aren't any PLTs then how do we end up here?

Will

_______________________________________________
linux-arm-kernel mailing list
linux-arm-kernel@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-arm-kernel

Re: [PATCH] arm64/module-plts: Consider the special case where plt_max_entries is 0

[Ard Biesheuvel]

On Wed, 8 Jul 2020 at 11:25, Will Deacon <will@kernel.org> wrote:
>
> [+Ard]
>
> On Tue, Jul 07, 2020 at 07:46:08AM -0400, Peng Hao wrote:
> > If plt_max_entries is 0, a warning is triggered.
> > WARNING: CPU: 200 PID: 3000 at arch/arm64/kernel/module-plts.c:97 module_emit_plt_entry+0xa4/0x150
>
> Which kernel are you seeing this with? There is a PLT-related change in
> for-next/core, and I'd like to rule if out if possible.
>
> > Signed-off-by: Peng Hao <richard.peng@oppo.com>
> > ---
> >  arch/arm64/kernel/module-plts.c | 3 ++-
> >  1 file changed, 2 insertions(+), 1 deletion(-)
> >
> > diff --git a/arch/arm64/kernel/module-plts.c b/arch/arm64/kernel/module-plts.c
> > index 65b08a74aec6..1868c9ac13f2 100644
> > --- a/arch/arm64/kernel/module-plts.c
> > +++ b/arch/arm64/kernel/module-plts.c
> > @@ -79,7 +79,8 @@ u64 module_emit_plt_entry(struct module *mod, Elf64_Shdr *sechdrs,
> >       int i = pltsec->plt_num_entries;
> >       int j = i - 1;
> >       u64 val = sym->st_value + rela->r_addend;
> > -
> > +     if (pltsec->plt_max_entries == 0)
> > +             return 0;
>
> Hmm, but if there aren't any PLTs then how do we end up here?
>

Indeed. module_emit_plt_entry() is only invoked if we encounter a call
or jump relocation that is out of range, and so we failed to allocate
enough PLT entries in module_frob_arch_sections() if you are entering
module_emit_plt_entry() with max_entries set to 0x0. The only other
way to trigger this is when the .text section of your module is so
large that branches go out of range, but PLTs won't help there.

_______________________________________________
linux-arm-kernel mailing list
linux-arm-kernel@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-arm-kernel