[PATCH] powerpc64/ftrace: fix module loading without patchable function entries

[PATCH] powerpc64/ftrace: fix module loading without patchable function entries

[Anthony Iliopoulos]

get_stubs_size assumes that there must always be at least one patchable
function entry, which is not always the case (modules that export data
but no code), otherwise it returns -ENOEXEC and thus the section header
sh_size is set to that value. During module_memory_alloc() the size is
passed to execmem_alloc() after being page-aligned and thus set to zero
which will cause it to fail the allocation (and thus module loading) as
__vmalloc_node_range() checks for zero-sized allocs and returns null:

[  115.466896] module_64: cast_common: doesn't contain __patchable_function_entries.
[  115.469189] ------------[ cut here ]------------
[  115.469496] WARNING: CPU: 0 PID: 274 at mm/vmalloc.c:3778 __vmalloc_node_range_noprof+0x8b4/0x8f0
...
[  115.478574] ---[ end trace 0000000000000000 ]---
[  115.479545] execmem: unable to allocate memory

Fix this by removing the check completely, since it is anyway not
helpful to propagate this as an error upwards.

Fixes: eec37961a56a ("powerpc64/ftrace: Move ftrace sequence out of line")
Signed-off-by: Anthony Iliopoulos <ailiop@suse.com>
---
 arch/powerpc/kernel/module_64.c | 4 ----
 1 file changed, 4 deletions(-)

diff --git a/arch/powerpc/kernel/module_64.c b/arch/powerpc/kernel/module_64.c
index 34a5aec4908f..126bf3b06ab7 100644
--- a/arch/powerpc/kernel/module_64.c
+++ b/arch/powerpc/kernel/module_64.c
@@ -258,10 +258,6 @@ static unsigned long get_stubs_size(const Elf64_Ehdr *hdr,
 			break;
 		}
 	}
-	if (i == hdr->e_shnum) {
-		pr_err("%s: doesn't contain __patchable_function_entries.\n", me->name);
-		return -ENOEXEC;
-	}
 #endif
 
 	pr_debug("Looks like a total of %lu stubs, max\n", relocs);
-- 
2.47.0

Re: [PATCH] powerpc64/ftrace: fix module loading without patchable function entries

[Naveen N Rao]

On Wed, Feb 05, 2025 at 12:18:21AM +0100, Anthony Iliopoulos wrote:
> get_stubs_size assumes that there must always be at least one patchable
> function entry, which is not always the case (modules that export data
> but no code), otherwise it returns -ENOEXEC and thus the section header
> sh_size is set to that value. During module_memory_alloc() the size is
> passed to execmem_alloc() after being page-aligned and thus set to zero
> which will cause it to fail the allocation (and thus module loading) as
> __vmalloc_node_range() checks for zero-sized allocs and returns null:
> 
> [  115.466896] module_64: cast_common: doesn't contain __patchable_function_entries.
> [  115.469189] ------------[ cut here ]------------
> [  115.469496] WARNING: CPU: 0 PID: 274 at mm/vmalloc.c:3778 __vmalloc_node_range_noprof+0x8b4/0x8f0
> ...
> [  115.478574] ---[ end trace 0000000000000000 ]---
> [  115.479545] execmem: unable to allocate memory

Ugh, that's nasty.

> 
> Fix this by removing the check completely, since it is anyway not
> helpful to propagate this as an error upwards.
> 
> Fixes: eec37961a56a ("powerpc64/ftrace: Move ftrace sequence out of line")
> Signed-off-by: Anthony Iliopoulos <ailiop@suse.com>
> ---
>  arch/powerpc/kernel/module_64.c | 4 ----
>  1 file changed, 4 deletions(-)
> 

Thanks for fixing this. It might also be good to add a check in 
setup_ftrace_ool_stubs(). Something like this:

diff --git a/arch/powerpc/kernel/module_64.c b/arch/powerpc/kernel/module_64.c
index 34a5aec4908f..c10a9c66cfe3 100644
--- a/arch/powerpc/kernel/module_64.c
+++ b/arch/powerpc/kernel/module_64.c
@@ -1125,6 +1125,10 @@ static int setup_ftrace_ool_stubs(const Elf64_Shdr *sechdrs, unsigned long addr,
        unsigned int i, total_stubs, num_stubs;
        struct ppc64_stub_entry *stub;
 
+       /* Bail out early if no traceable functions */
+       if (!me->arch.ool_stub_count)
+               return 0;
+
        total_stubs = sechdrs[me->arch.stubs_section].sh_size / sizeof(*stub);
        num_stubs = roundup(me->arch.ool_stub_count * sizeof(struct ftrace_ool_stub),
                            sizeof(struct ppc64_stub_entry)) / sizeof(struct ppc64_stub_entry);


Regardless of that, for this patch:
Acked-by: Naveen N Rao (AMD) <naveen@kernel.org>


- Naveen

Re: [PATCH] powerpc64/ftrace: fix module loading without patchable function entries

[Adam Williamson]

On Thu, 2025-02-13 at 22:29 +0530, Naveen N Rao wrote:
> On Wed, Feb 05, 2025 at 12:18:21AM +0100, Anthony Iliopoulos wrote:
> > get_stubs_size assumes that there must always be at least one patchable
> > function entry, which is not always the case (modules that export data
> > but no code), otherwise it returns -ENOEXEC and thus the section header
> > sh_size is set to that value. During module_memory_alloc() the size is
> > passed to execmem_alloc() after being page-aligned and thus set to zero
> > which will cause it to fail the allocation (and thus module loading) as
> > __vmalloc_node_range() checks for zero-sized allocs and returns null:
> > 
> > [  115.466896] module_64: cast_common: doesn't contain __patchable_function_entries.
> > [  115.469189] ------------[ cut here ]------------
> > [  115.469496] WARNING: CPU: 0 PID: 274 at mm/vmalloc.c:3778 __vmalloc_node_range_noprof+0x8b4/0x8f0
> > ...
> > [  115.478574] ---[ end trace 0000000000000000 ]---
> > [  115.479545] execmem: unable to allocate memory
> 
> Ugh, that's nasty.
> 
> > 
> > Fix this by removing the check completely, since it is anyway not
> > helpful to propagate this as an error upwards.
> > 
> > Fixes: eec37961a56a ("powerpc64/ftrace: Move ftrace sequence out of line")
> > Signed-off-by: Anthony Iliopoulos <ailiop@suse.com>
> > ---
> >  arch/powerpc/kernel/module_64.c | 4 ----
> >  1 file changed, 4 deletions(-)
> > 
> 
> Thanks for fixing this. It might also be good to add a check in 
> setup_ftrace_ool_stubs(). Something like this:
> 
> diff --git a/arch/powerpc/kernel/module_64.c b/arch/powerpc/kernel/module_64.c
> index 34a5aec4908f..c10a9c66cfe3 100644
> --- a/arch/powerpc/kernel/module_64.c
> +++ b/arch/powerpc/kernel/module_64.c
> @@ -1125,6 +1125,10 @@ static int setup_ftrace_ool_stubs(const Elf64_Shdr *sechdrs, unsigned long addr,
>         unsigned int i, total_stubs, num_stubs;
>         struct ppc64_stub_entry *stub;
>  
> +       /* Bail out early if no traceable functions */
> +       if (!me->arch.ool_stub_count)
> +               return 0;
> +
>         total_stubs = sechdrs[me->arch.stubs_section].sh_size / sizeof(*stub);
>         num_stubs = roundup(me->arch.ool_stub_count * sizeof(struct ftrace_ool_stub),
>                             sizeof(struct ppc64_stub_entry)) / sizeof(struct ppc64_stub_entry);
> 
> 
> Regardless of that, for this patch:
> Acked-by: Naveen N Rao (AMD) <naveen@kernel.org>
> 
> 
> - Naveen
> 
Is there anything that can be done to get this bumped along? It
completely breaks networking on Fedora's power9 boxes, as it affects
the i40e module which they use.

Thanks!
-- 
Adam Williamson (he/him/his)
Fedora QA
Fedora Chat: @adamwill:fedora.im | Mastodon: @adamw@fosstodon.org
https://www.happyassassin.net

Re: [PATCH] powerpc64/ftrace: fix module loading without patchable function entries

[Madhavan Srinivasan]

On 3/26/25 6:46 AM, Adam Williamson wrote:
> On Thu, 2025-02-13 at 22:29 +0530, Naveen N Rao wrote:
>> On Wed, Feb 05, 2025 at 12:18:21AM +0100, Anthony Iliopoulos wrote:
>>> get_stubs_size assumes that there must always be at least one patchable
>>> function entry, which is not always the case (modules that export data
>>> but no code), otherwise it returns -ENOEXEC and thus the section header
>>> sh_size is set to that value. During module_memory_alloc() the size is
>>> passed to execmem_alloc() after being page-aligned and thus set to zero
>>> which will cause it to fail the allocation (and thus module loading) as
>>> __vmalloc_node_range() checks for zero-sized allocs and returns null:
>>>
>>> [  115.466896] module_64: cast_common: doesn't contain __patchable_function_entries.
>>> [  115.469189] ------------[ cut here ]------------
>>> [  115.469496] WARNING: CPU: 0 PID: 274 at mm/vmalloc.c:3778 __vmalloc_node_range_noprof+0x8b4/0x8f0
>>> ...
>>> [  115.478574] ---[ end trace 0000000000000000 ]---
>>> [  115.479545] execmem: unable to allocate memory
>>
>> Ugh, that's nasty.
>>
>>>
>>> Fix this by removing the check completely, since it is anyway not
>>> helpful to propagate this as an error upwards.
>>>
>>> Fixes: eec37961a56a ("powerpc64/ftrace: Move ftrace sequence out of line")
>>> Signed-off-by: Anthony Iliopoulos <ailiop@suse.com>
>>> ---
>>>  arch/powerpc/kernel/module_64.c | 4 ----
>>>  1 file changed, 4 deletions(-)
>>>
>>
>> Thanks for fixing this. It might also be good to add a check in 
>> setup_ftrace_ool_stubs(). Something like this:
>>
>> diff --git a/arch/powerpc/kernel/module_64.c b/arch/powerpc/kernel/module_64.c
>> index 34a5aec4908f..c10a9c66cfe3 100644
>> --- a/arch/powerpc/kernel/module_64.c
>> +++ b/arch/powerpc/kernel/module_64.c
>> @@ -1125,6 +1125,10 @@ static int setup_ftrace_ool_stubs(const Elf64_Shdr *sechdrs, unsigned long addr,
>>         unsigned int i, total_stubs, num_stubs;
>>         struct ppc64_stub_entry *stub;
>>  
>> +       /* Bail out early if no traceable functions */
>> +       if (!me->arch.ool_stub_count)
>> +               return 0;
>> +
>>         total_stubs = sechdrs[me->arch.stubs_section].sh_size / sizeof(*stub);
>>         num_stubs = roundup(me->arch.ool_stub_count * sizeof(struct ftrace_ool_stub),
>>                             sizeof(struct ppc64_stub_entry)) / sizeof(struct ppc64_stub_entry);
>>
>>
>> Regardless of that, for this patch:
>> Acked-by: Naveen N Rao (AMD) <naveen@kernel.org>
>>
>>
>> - Naveen
>>
> Is there anything that can be done to get this bumped along? It
> completely breaks networking on Fedora's power9 boxes, as it affects
> the i40e module which they use.
> 

Sorry missed this, Will pull it in.

Thanks.
Maddy

> Thanks!

Re: [PATCH] powerpc64/ftrace: fix module loading without patchable function entries

[Madhavan Srinivasan]

On Wed, 05 Feb 2025 00:18:21 +0100, Anthony Iliopoulos wrote:
> get_stubs_size assumes that there must always be at least one patchable
> function entry, which is not always the case (modules that export data
> but no code), otherwise it returns -ENOEXEC and thus the section header
> sh_size is set to that value. During module_memory_alloc() the size is
> passed to execmem_alloc() after being page-aligned and thus set to zero
> which will cause it to fail the allocation (and thus module loading) as
> __vmalloc_node_range() checks for zero-sized allocs and returns null:
> 
> [...]

Applied to powerpc/fixes.

[1/1] powerpc64/ftrace: fix module loading without patchable function entries
      https://git.kernel.org/powerpc/c/534f5a8ba27863141e29766467a3e1f61bcb47ac

Thanks