print module information when KALLSYMS is disabled.
No change for %pB, as it needs to know symbol name to adjust address
value which can't be done without KALLSYMS.
(A) original output with KALLSYMS:
[8.842129] ps function_1 [crash]
[8.842735] pS function_1+0x4/0x2c [crash]
[8.842890] pSb function_1+0x4/0x2c [crash b367e79021b9f3b0172f9a36d4261c1f528ca1b3]
[8.843175] pB function_1+0x4/0x2c [crash]
[8.843362] pBb function_1+0x4/0x2c [crash b367e79021b9f3b0172f9a36d4261c1f528ca1b3]
(B) original output without KALLSYMS:
[12.487424] ps 0xffff800000eb008c
[12.487598] pS 0xffff800000eb008c
[12.487723] pSb 0xffff800000eb008c
[12.487850] pB 0xffff800000eb008c
[12.487967] pBb 0xffff800000eb008c
(C) With patched kernel
with KALLYSMS:
[41.974576] ps function_1 [crash]
[41.975173] pS function_1+0x4/0x2c [crash]
[41.975386] pSb function_1+0x4/0x2c [crash a8b20caaec9635b316cf4812f6b55598fe2b7cee]
[41.975879] pB function_1+0x4/0x2c [crash]
[41.976076] pBb function_1+0x4/0x2c [crash a8b20caaec9635b316cf4812f6b55598fe2b7cee]
without KALLSYMS:
[9.624152] ps 0xffff800001bd008c [crash] // similar to original, no changes
[9.624548] pS 0x(____ptrval____)+0x8c [crash] // base address hashed and offset is without hash
[9.624847] pSb 0x(____ptrval____)+0x8c [crash a8b20caaec9635b316cf4812f6b55598fe2b7cee]
[9.625388] pB 0x(____ptrval____)+0x8c [crash]
[9.625594] pBb 0x(____ptrval____)+0x8c [crash a8b20caaec9635b316cf4812f6b55598fe2b7cee]
with disable hashing:
[8.563916] ps 0xffff800000f2008c [crash]
[8.564574] pS 0xffff800000f20000+0x8c [crash]
[8.564749] pSb 0xffff800000f20000+0x8c [crash 3423a8993a7033fb79e5add14bf9d8d6b56330ca]
[8.565008] pB 0xffff800000f20000+0x8c [crash]
[8.565154] pBb 0xffff800000f20000+0x8c [crash 3423a8993a7033fb79e5add14bf9d8d6b56330ca]
Suggested-by: Petr Mladek <[email protected]>
Co-developed-by: Vaneet Narang <[email protected]>
Signed-off-by: Vaneet Narang <[email protected]>
Signed-off-by: Maninder Singh <[email protected]>
---
commit id 'kallsyms: print module name in %ps/S case when KALLSYMS is disabled'
needs to be removed from mm(linux-next) tree, current change is
with ignorance of this commit. I was not sure how to send patch, with 2 patches
consisting reversal commit also, or current approach is correct.
v1->v2: hash base address of module, change *fmt to fmt[0] and removed
copy paste.
v2->v3: fixed review comments from Andy Shevchenko <[email protected]>
include/linux/kallsyms.h | 2 +
include/linux/module.h | 20 ++++++++++
kernel/kallsyms.c | 27 +++++++------
kernel/module.c | 4 +-
lib/vsprintf.c | 85 ++++++++++++++++++++++++++++++++++------
5 files changed, 109 insertions(+), 29 deletions(-)
diff --git a/include/linux/kallsyms.h b/include/linux/kallsyms.h
index e5ad6e31697d..c24fa627ab6e 100644
--- a/include/linux/kallsyms.h
+++ b/include/linux/kallsyms.h
@@ -89,6 +89,8 @@ extern int sprint_symbol_build_id(char *buffer, unsigned long address);
extern int sprint_symbol_no_offset(char *buffer, unsigned long address);
extern int sprint_backtrace(char *buffer, unsigned long address);
extern int sprint_backtrace_build_id(char *buffer, unsigned long address);
+extern int sprint_kallsym_common(char *buffer, unsigned long address, int build_id,
+ int backtrace, int symbol);
int lookup_symbol_name(unsigned long addr, char *symname);
int lookup_symbol_attrs(unsigned long addr, unsigned long *size, unsigned long *offset, char *modname, char *name);
diff --git a/include/linux/module.h b/include/linux/module.h
index 1e135fd5c076..b154fa822f77 100644
--- a/include/linux/module.h
+++ b/include/linux/module.h
@@ -678,6 +678,20 @@ static inline bool is_livepatch_module(struct module *mod)
bool is_module_sig_enforced(void);
void set_module_sig_enforced(void);
+static inline int fill_name_build_id(char *buffer, char *modname,
+ int add_buildid, const unsigned char *buildid,
+ int len)
+{
+ len += sprintf(buffer + len, " [%s", modname);
+#if IS_ENABLED(CONFIG_STACKTRACE_BUILD_ID)
+ if (add_buildid && buildid) {
+ /* build ID should match length of sprintf */
+ static_assert(sizeof(typeof_member(struct module, build_id)) == 20);
+ len += sprintf(buffer + len, " %20phN", buildid);
+ }
+#endif
+ return len + sprintf(buffer + len, "]");
+}
#else /* !CONFIG_MODULES... */
static inline struct module *__module_address(unsigned long addr)
@@ -818,6 +832,12 @@ void *dereference_module_function_descriptor(struct module *mod, void *ptr)
return ptr;
}
+static inline int fill_name_build_id(char *buffer, char *modname,
+ int add_buildid, const unsigned char *buildid,
+ int len)
+{
+ return 0;
+}
#endif /* CONFIG_MODULES */
#ifdef CONFIG_SYSFS
diff --git a/kernel/kallsyms.c b/kernel/kallsyms.c
index 57213e1d2349..7762efadf166 100644
--- a/kernel/kallsyms.c
+++ b/kernel/kallsyms.c
@@ -479,19 +479,8 @@ static int __sprint_symbol(char *buffer, unsigned long address,
if (add_offset)
len += sprintf(buffer + len, "+%#lx/%#lx", offset, size);
- if (modname) {
- len += sprintf(buffer + len, " [%s", modname);
-#if IS_ENABLED(CONFIG_STACKTRACE_BUILD_ID)
- if (add_buildid && buildid) {
- /* build ID should match length of sprintf */
-#if IS_ENABLED(CONFIG_MODULES)
- static_assert(sizeof(typeof_member(struct module, build_id)) == 20);
-#endif
- len += sprintf(buffer + len, " %20phN", buildid);
- }
-#endif
- len += sprintf(buffer + len, "]");
- }
+ if (modname)
+ len += fill_name_build_id(buffer, modname, add_buildid, buildid, len);
return len;
}
@@ -586,6 +575,18 @@ int sprint_backtrace_build_id(char *buffer, unsigned long address)
return __sprint_symbol(buffer, address, -1, 1, 1);
}
+int sprint_kallsym_common(char *buffer, unsigned long address, int build_id,
+ int backtrace, int symbol)
+{
+ if (backtrace)
+ return __sprint_symbol(buffer, address, -1, 1, build_id);
+
+ if (symbol)
+ return __sprint_symbol(buffer, address, 0, 1, build_id);
+
+ return __sprint_symbol(buffer, address, 0, 0, 0);
+}
+
/* To avoid using get_symbol_offset for every symbol, we carry prefix along. */
struct kallsym_iter {
loff_t pos;
diff --git a/kernel/module.c b/kernel/module.c
index 6cea788fd965..5756d31a024b 100644
--- a/kernel/module.c
+++ b/kernel/module.c
@@ -1465,12 +1465,10 @@ resolve_symbol_wait(struct module *mod,
return ksym;
}
-#ifdef CONFIG_KALLSYMS
static inline bool sect_empty(const Elf_Shdr *sect)
{
return !(sect->sh_flags & SHF_ALLOC) || sect->sh_size == 0;
}
-#endif
/*
* /sys/module/foo/sections stuff
@@ -2799,7 +2797,7 @@ static void add_kallsyms(struct module *mod, const struct load_info *info)
}
#endif /* CONFIG_KALLSYMS */
-#if IS_ENABLED(CONFIG_KALLSYMS) && IS_ENABLED(CONFIG_STACKTRACE_BUILD_ID)
+#if IS_ENABLED(CONFIG_STACKTRACE_BUILD_ID)
static void init_build_id(struct module *mod, const struct load_info *info)
{
const Elf_Shdr *sechdr;
diff --git a/lib/vsprintf.c b/lib/vsprintf.c
index 7adb8fd4d804..86f7d24af73c 100644
--- a/lib/vsprintf.c
+++ b/lib/vsprintf.c
@@ -1000,33 +1000,92 @@ char *bdev_name(char *buf, char *end, struct block_device *bdev,
}
#endif
+#if !defined(CONFIG_KALLSYMS) && defined(CONFIG_MODULES)
+static int sprint_module_info(char *buf, unsigned long value,
+ int modbuildid, int backtrace, int symbol)
+{
+ struct module *mod;
+ unsigned long offset;
+ void *base;
+ char *modname;
+ int len;
+ const unsigned char *buildid = NULL;
+ bool add_offset;
+
+ if (is_ksym_addr(value))
+ return 0;
+
+ if (backtrace || symbol)
+ add_offset = true;
+ else
+ add_offset = false;
+
+ preempt_disable();
+ mod = __module_address(value);
+ if (mod) {
+ modname = mod->name;
+#if IS_ENABLED(CONFIG_STACKTRACE_BUILD_ID)
+ if (modbuildid)
+ buildid = mod->build_id;
+#endif
+ if (add_offset) {
+ base = mod->core_layout.base;
+ offset = value - (unsigned long)base;
+ }
+ }
+ preempt_enable();
+ if (!mod)
+ return 0;
+
+ /* address belongs to module */
+ if (add_offset)
+ len = sprintf(buf, "0x%p+0x%lx", base, offset);
+ else
+ len = sprintf(buf, "0x%lx", value);
+
+ return len + fill_name_build_id(buf, modname, modbuildid, buildid, len);
+}
+#else
+static inline int sprint_module_info(char *buf, unsigned long value,
+ int modbuildid, int backtrace, int symbol)
+{
+ return 0;
+}
+#endif
+
static noinline_for_stack
char *symbol_string(char *buf, char *end, void *ptr,
struct printf_spec spec, const char *fmt)
{
unsigned long value;
-#ifdef CONFIG_KALLSYMS
char sym[KSYM_SYMBOL_LEN];
-#endif
+ int backtrace = 0, symbol = 0, build_id = 0;
if (fmt[1] == 'R')
ptr = __builtin_extract_return_addr(ptr);
value = (unsigned long)ptr;
-#ifdef CONFIG_KALLSYMS
- if (*fmt == 'B' && fmt[1] == 'b')
- sprint_backtrace_build_id(sym, value);
- else if (*fmt == 'B')
- sprint_backtrace(sym, value);
- else if (*fmt == 'S' && (fmt[1] == 'b' || (fmt[1] == 'R' && fmt[2] == 'b')))
- sprint_symbol_build_id(sym, value);
- else if (*fmt != 's')
- sprint_symbol(sym, value);
- else
- sprint_symbol_no_offset(sym, value);
+ if (fmt[0] == 'B' && fmt[1] == 'b') {
+ backtrace = 1;
+ build_id = 1;
+ } else if (fmt[0] == 'B')
+ backtrace = 1;
+ else if (fmt[0] == 'S' && (fmt[1] == 'b' || (fmt[1] == 'R' && fmt[2] == 'b'))) {
+ symbol = 1;
+ build_id = 1;
+ } else if (fmt[0] != 's')
+ symbol = 1;
+ else {
+ /* Do Nothing, no offset */
+ }
+#ifdef CONFIG_KALLSYMS
+ sprint_kallsym_common(sym, value, build_id, backtrace, symbol);
return string_nocheck(buf, end, sym, spec);
#else
+ if (sprint_module_info(sym, value, build_id, backtrace, symbol))
+ return string_nocheck(buf, end, sym, spec);
+
return special_hex_number(buf, end, value, sizeof(void *));
#endif
}
--
2.17.1
Hi Luis
Le 15/03/2022 à 18:52, Luis Chamberlain a écrit :
> On Tue, Mar 15, 2022 at 09:21:00PM +0530, Maninder Singh wrote:
>> include/linux/kallsyms.h | 2 +
>> include/linux/module.h | 20 ++++++++++
>> kernel/kallsyms.c | 27 +++++++------
>> kernel/module.c | 4 +-
>> lib/vsprintf.c | 85 ++++++++++++++++++++++++++++++++++------
>
> Hey Maninder, thanks for your patch!
>
> Since this touches kernel/module.c and include/linux/module.h I'd prefer
> this go through modules-next [0], and as you will see that's a different
> world right now. I also have a set of at least 2 other patch sets to
> merge there before yours.
>
> Also, what is on modules-next is not intended to go to Linus for the
> next merge window as the changes there got merged only late, and I want
> at least 2 months of testing on linux-newt before any pull requiest is
> sent to Linus.
>
> Can you rebase to modules-next? I can evaluate the patches then for
> integration there once the other stuff gets merged into that tree too.
>
> [0] https://git.kernel.org/pub/scm/linux/kernel/git/mcgrof/linux.git/log/?h=modules-next
>
I can't see Aaron's series on modules-next yet, do you plan to merge it
anytime soon ?
As you say, it will be a different world by then.
I have my series rebases on top of latest Aaron's series, but I was
waiting that it lands in modules-next. How do you plan to proceed ?
Thanks
Christophe
Hi Luis,
>> ---
>> commit id 'kallsyms: print module name in %ps/S case when KALLSYMS is disabled'
>> needs to be removed from mm(linux-next) tree, current change is
>> with ignorance of this commit. I was not sure how to send patch, with 2 patches
>> consisting reversal commit also, or current approach is correct.
>>
>> v1->v2: hash base address of module, change *fmt to fmt[0] and removed
>> copy paste.
>> v2->v3: fixed review comments from Andy Shevchenko <[email protected]>
>>
>> include/linux/kallsyms.h | 2 +
>> include/linux/module.h | 20 ++++++++++
>> kernel/kallsyms.c | 27 +++++++------
>> kernel/module.c | 4 +-
>> lib/vsprintf.c | 85 ++++++++++++++++++++++++++++++++++------
>
> Hey Maninder, thanks for your patch!
>
> Since this touches kernel/module.c and include/linux/module.h I'd prefer
> this go through modules-next [0], and as you will see that's a different
> world right now. I also have a set of at least 2 other patch sets to
> merge there before yours.
>
> Also, what is on modules-next is not intended to go to Linus for the
> next merge window as the changes there got merged only late, and I want
> at least 2 months of testing on linux-newt before any pull requiest is
> sent to Linus.
>
> Can you rebase to modules-next? I can evaluate the patches then for
> integration there once the other stuff gets merged into that tree too.
>
> [0] https://git.kernel.org/pub/scm/linux/kernel/git/mcgrof/linux.git/log/?h=modules-next
>
> Luis
prepared and verified patch(KALLSYMS enabled and disabled both) on module-next rebase and sent in new mail.
[PATCH 1/1 module-next] kallsyms: enhance %pS/s/b printing when KALLSYSMS is disabled
https://lkml.org/lkml/2022/3/16/7
Thanks,
Maninder Singh
On Tue, Mar 15, 2022 at 09:21:00PM +0530, Maninder Singh wrote:
> print module information when KALLSYMS is disabled.
>
> No change for %pB, as it needs to know symbol name to adjust address
> value which can't be done without KALLSYMS.
>
> (A) original output with KALLSYMS:
> [8.842129] ps function_1 [crash]
> [8.842735] pS function_1+0x4/0x2c [crash]
> [8.842890] pSb function_1+0x4/0x2c [crash b367e79021b9f3b0172f9a36d4261c1f528ca1b3]
> [8.843175] pB function_1+0x4/0x2c [crash]
> [8.843362] pBb function_1+0x4/0x2c [crash b367e79021b9f3b0172f9a36d4261c1f528ca1b3]
>
> (B) original output without KALLSYMS:
> [12.487424] ps 0xffff800000eb008c
> [12.487598] pS 0xffff800000eb008c
> [12.487723] pSb 0xffff800000eb008c
> [12.487850] pB 0xffff800000eb008c
> [12.487967] pBb 0xffff800000eb008c
>
> (C) With patched kernel
> with KALLYSMS:
> [41.974576] ps function_1 [crash]
> [41.975173] pS function_1+0x4/0x2c [crash]
> [41.975386] pSb function_1+0x4/0x2c [crash a8b20caaec9635b316cf4812f6b55598fe2b7cee]
> [41.975879] pB function_1+0x4/0x2c [crash]
> [41.976076] pBb function_1+0x4/0x2c [crash a8b20caaec9635b316cf4812f6b55598fe2b7cee]
>
> without KALLSYMS:
> [9.624152] ps 0xffff800001bd008c [crash] // similar to original, no changes
> [9.624548] pS 0x(____ptrval____)+0x8c [crash] // base address hashed and offset is without hash
> [9.624847] pSb 0x(____ptrval____)+0x8c [crash a8b20caaec9635b316cf4812f6b55598fe2b7cee]
> [9.625388] pB 0x(____ptrval____)+0x8c [crash]
> [9.625594] pBb 0x(____ptrval____)+0x8c [crash a8b20caaec9635b316cf4812f6b55598fe2b7cee]
>
> with disable hashing:
> [8.563916] ps 0xffff800000f2008c [crash]
> [8.564574] pS 0xffff800000f20000+0x8c [crash]
> [8.564749] pSb 0xffff800000f20000+0x8c [crash 3423a8993a7033fb79e5add14bf9d8d6b56330ca]
> [8.565008] pB 0xffff800000f20000+0x8c [crash]
> [8.565154] pBb 0xffff800000f20000+0x8c [crash 3423a8993a7033fb79e5add14bf9d8d6b56330ca]
>
> Suggested-by: Petr Mladek <[email protected]>
> Co-developed-by: Vaneet Narang <[email protected]>
> Signed-off-by: Vaneet Narang <[email protected]>
> Signed-off-by: Maninder Singh <[email protected]>
> ---
> commit id 'kallsyms: print module name in %ps/S case when KALLSYMS is disabled'
> needs to be removed from mm(linux-next) tree, current change is
> with ignorance of this commit. I was not sure how to send patch, with 2 patches
> consisting reversal commit also, or current approach is correct.
>
> v1->v2: hash base address of module, change *fmt to fmt[0] and removed
> copy paste.
> v2->v3: fixed review comments from Andy Shevchenko <[email protected]>
>
> include/linux/kallsyms.h | 2 +
> include/linux/module.h | 20 ++++++++++
> kernel/kallsyms.c | 27 +++++++------
> kernel/module.c | 4 +-
> lib/vsprintf.c | 85 ++++++++++++++++++++++++++++++++++------
Hey Maninder, thanks for your patch!
Since this touches kernel/module.c and include/linux/module.h I'd prefer
this go through modules-next [0], and as you will see that's a different
world right now. I also have a set of at least 2 other patch sets to
merge there before yours.
Also, what is on modules-next is not intended to go to Linus for the
next merge window as the changes there got merged only late, and I want
at least 2 months of testing on linux-newt before any pull requiest is
sent to Linus.
Can you rebase to modules-next? I can evaluate the patches then for
integration there once the other stuff gets merged into that tree too.
[0] https://git.kernel.org/pub/scm/linux/kernel/git/mcgrof/linux.git/log/?h=modules-next
Luis
On Wed, Mar 16, 2022 at 01:25:27AM -0700, Luis Chamberlain wrote:
> Anyway, your stuff is at the end of the train after Michal's
Sorry for not being clear, this was directed towards Maninder.
Luis
On Wed, Mar 16, 2022 at 06:40:02AM +0000, Christophe Leroy wrote:
> Hi Luis
>
>
> Le 15/03/2022 ? 18:52, Luis Chamberlain a ?crit?:
> > On Tue, Mar 15, 2022 at 09:21:00PM +0530, Maninder Singh wrote:
> >> include/linux/kallsyms.h | 2 +
> >> include/linux/module.h | 20 ++++++++++
> >> kernel/kallsyms.c | 27 +++++++------
> >> kernel/module.c | 4 +-
> >> lib/vsprintf.c | 85 ++++++++++++++++++++++++++++++++++------
> >
> > Hey Maninder, thanks for your patch!
> >
> > Since this touches kernel/module.c and include/linux/module.h I'd prefer
> > this go through modules-next [0], and as you will see that's a different
> > world right now. I also have a set of at least 2 other patch sets to
> > merge there before yours.
> >
> > Also, what is on modules-next is not intended to go to Linus for the
> > next merge window as the changes there got merged only late, and I want
> > at least 2 months of testing on linux-newt before any pull requiest is
> > sent to Linus.
> >
> > Can you rebase to modules-next? I can evaluate the patches then for
> > integration there once the other stuff gets merged into that tree too.
> >
> > [0] https://git.kernel.org/pub/scm/linux/kernel/git/mcgrof/linux.git/log/?h=modules-next
> >
>
> I can't see Aaron's series on modules-next yet, do you plan to merge it
> anytime soon ?
>
> As you say, it will be a different world by then.
>
> I have my series rebases on top of latest Aaron's series, but I was
> waiting that it lands in modules-next. How do you plan to proceed ?
Yes sorry about that, modules-testing is what had Aaron's code. And
Aaron noted that from his series 13/14 and 14/14 from his series
had their Message-Id modified accidently via git-send-email. Given
*current events* and since I use b4 am to verify KSIM signatures
I asked Aaron to wait and post a v12 to aggregate further reviews
and acked-by's. The reason being that if his v11 series has issues
I rather start from a very clean patchset.
Yes I am paranoid :)
Anyway so Aaron, let's give it a few more days, and please then post a
v12 collecing all new tags, then I'll apply your changes and then try to
apply Christophe's.
There was some work by Michal Such?nek which would go after, but
its unclear if that's yet vetted by their other respective maintainers.
Michal?
Anyway, your stuff is at the end of the train after Michal's if that
stuff is really ready. So please don't be surprised if you later have
to rebase once again, or two or 3 times more. Thanks for your patience.
I know this has been quite a bit of churn, but given Aaron's series I
really hope we're goint to be in a better place for maintenance for
modules long term.
I guess I gotta go automate tests to these things somehow too.
Luis
Hi,
> Yes sorry about that, modules-testing is what had Aaron's code. And
> Aaron noted that from his series 13/14 and 14/14 from his series
> had their Message-Id modified accidently via git-send-email. Given
> *current events* and since I use b4 am to verify KSIM signatures
> I asked Aaron to wait and post a v12 to aggregate further reviews
> and acked-by's. The reason being that if his v11 series has issues
> I rather start from a very clean patchset.
>
> Yes I am paranoid :)
>
> Anyway so Aaron, let's give it a few more days, and please then post a
> v12 collecing all new tags, then I'll apply your changes and then try to
> apply Christophe's.
>
> There was some work by Michal Suchánek which would go after, but
> its unclear if that's yet vetted by their other respective maintainers.
> Michal?
>
> Anyway, your stuff is at the end of the train after Michal's if that
> stuff is really ready. So please don't be surprised if you later have
> to rebase once again, or two or 3 times more. Thanks for your patience.
No Worries :)
Let me know when it has to be done.
We will prepare and verify on new rebase.
Thanks,
Maninder Singh
On Wed 2022-03-16 01:25 -0700, Luis Chamberlain wrote:
> I know this has been quite a bit of churn, but given Aaron's series I
> really hope we're goint to be in a better place for maintenance for
> modules long term.
>
> I guess I gotta go automate tests to these things somehow too.
Hi Luis,
I can send a v12 now if you'd like?
Kind regards,
--
Aaron Tomlin
On Mon, Mar 21, 2022 at 01:39:42PM +0000, Aaron Tomlin wrote:
> On Wed 2022-03-16 01:25 -0700, Luis Chamberlain wrote:
> > I know this has been quite a bit of churn, but given Aaron's series I
> > really hope we're goint to be in a better place for maintenance for
> > modules long term.
> >
> > I guess I gotta go automate tests to these things somehow too.
>
> Hi Luis,
>
> I can send a v12 now if you'd like?
Sure, yes please, go for it, please be sure to collect all other
Reviewed-by/etc tags.
Luis