| Message ID | 1507646852-18642-1-git-send-email-kamalesh@linux.vnet.ibm.com (mailing list archive) |
|---|---|
| State | Accepted |
| Commit | 1c0437af9fca8de6e4ba179d18cf13154da25695 |
| Headers | show |
| Series | powerpc/modules: Use WARN_ON() in stub_for_addr() | expand |
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
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
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
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
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];
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(-)