powerpc/modules: Use WARN_ON() in stub_for_addr()

Message ID 1507646852-18642-1-git-send-email-kamalesh@linux.vnet.ibm.com
State Accepted
Commit 1c0437af9fca8de6e4ba179d18cf13154da25695
Headers show
Series
  • powerpc/modules: Use WARN_ON() in stub_for_addr()
Related show

Commit Message

Kamalesh Babulal Oct. 10, 2017, 2:47 p.m.
Use WARN_ON(), while running out of stubs in stub_for_addr()
and abort loading of the module instead of BUG_ON().

Signed-off-by: Kamalesh Babulal <kamalesh@linux.vnet.ibm.com>
---
 arch/powerpc/kernel/module_64.c | 3 ++-
 1 file changed, 2 insertions(+), 1 deletion(-)

Comments

Michael Ellerman Oct. 11, 2017, 4:12 a.m. | #1
Kamalesh Babulal <kamalesh@linux.vnet.ibm.com> writes:

> Use WARN_ON(), while running out of stubs in stub_for_addr()
> and abort loading of the module instead of BUG_ON().

Thanks. This looks good in principle. Have you actually tested it to
make sure we do in fact gracefully fail the module load?

cheers
Kamalesh Babulal Oct. 11, 2017, 4:56 a.m. | #2
On Wednesday 11 October 2017 09:42 AM, Michael Ellerman wrote:
> Kamalesh Babulal <kamalesh@linux.vnet.ibm.com> writes:
> 
>> Use WARN_ON(), while running out of stubs in stub_for_addr()
>> and abort loading of the module instead of BUG_ON().
> 
> Thanks. This looks good in principle. Have you actually tested it to
> make sure we do in fact gracefully fail the module load?
> 

Thanks for the review. I tested with little hackish version of this patch:

+       if (!strncmp(me->name, "live", 4))
+               j = 100;
+       for (i = 0; stub_func_addr(stubs[i].funcdata); i+=j) {
+               if (WARN_ON(i >= num_stubs))
+                       return 0;

and it fails gracefully.

# modprobe livepatch-sample
modprobe: ERROR: could not insert 'livepatch_sample': Unknown symbol in module, or unknown parameter (see dmesg)

# echo $?
1

# dmesg 
------------[ cut here ]------------
WARNING: CPU: 2 PID: 2836 at arch/powerpc/kernel/module_64.c:526 apply_relocate_add+0x71c/0xb00
Michael Ellerman Oct. 12, 2017, 4:29 a.m. | #3
Kamalesh Babulal <kamalesh@linux.vnet.ibm.com> writes:

> On Wednesday 11 October 2017 09:42 AM, Michael Ellerman wrote:
>> Kamalesh Babulal <kamalesh@linux.vnet.ibm.com> writes:
>> 
>>> Use WARN_ON(), while running out of stubs in stub_for_addr()
>>> and abort loading of the module instead of BUG_ON().
>> 
>> Thanks. This looks good in principle. Have you actually tested it to
>> make sure we do in fact gracefully fail the module load?
>> 
>
> Thanks for the review. I tested with little hackish version of this patch:
>
> +       if (!strncmp(me->name, "live", 4))
> +               j = 100;
> +       for (i = 0; stub_func_addr(stubs[i].funcdata); i+=j) {
> +               if (WARN_ON(i >= num_stubs))
> +                       return 0;
>
> and it fails gracefully.
>
> # modprobe livepatch-sample
> modprobe: ERROR: could not insert 'livepatch_sample': Unknown symbol in module, or unknown parameter (see dmesg)
>
> # echo $?
> 1
>
> # dmesg 
> ------------[ cut here ]------------
> WARNING: CPU: 2 PID: 2836 at arch/powerpc/kernel/module_64.c:526 apply_relocate_add+0x71c/0xb00

Thanks.

cheers
Michael Ellerman Oct. 19, 2017, 4:48 a.m. | #4
On Tue, 2017-10-10 at 14:47:32 UTC, Kamalesh Babulal wrote:
> Use WARN_ON(), while running out of stubs in stub_for_addr()
> and abort loading of the module instead of BUG_ON().
> 
> Signed-off-by: Kamalesh Babulal <kamalesh@linux.vnet.ibm.com>

Applied to powerpc next, thanks.

https://git.kernel.org/powerpc/c/1c0437af9fca8de6e4ba179d18cf13

cheers

Patch

diff --git a/arch/powerpc/kernel/module_64.c b/arch/powerpc/kernel/module_64.c
index 0b0f896..759104b 100644
--- a/arch/powerpc/kernel/module_64.c
+++ b/arch/powerpc/kernel/module_64.c
@@ -429,7 +429,8 @@  static unsigned long stub_for_addr(const Elf64_Shdr *sechdrs,
 	/* Find this stub, or if that fails, the next avail. entry */
 	stubs = (void *)sechdrs[me->arch.stubs_section].sh_addr;
 	for (i = 0; stub_func_addr(stubs[i].funcdata); i++) {
-		BUG_ON(i >= num_stubs);
+		if (WARN_ON(i >= num_stubs))
+			return 0;
 
 		if (stub_func_addr(stubs[i].funcdata) == func_addr(addr))
 			return (unsigned long)&stubs[i];