From: Sebastian Andrzej Siewior <bigeasy@linutronix.de>
To: Petr Mladek <pmladek@suse.com>, Thomas Gleixner <tglx@kernel.org>,
Peter Zijlstra <peterz@infradead.org>
Cc: linux-kernel@vger.kernel.org, linux-hardening@vger.kernel.org,
Andrew Morton <akpm@linux-foundation.org>,
Andy Shevchenko <andriy.shevchenko@linux.intel.com>,
Kees Cook <kees@kernel.org>,
Rasmus Villemoes <linux@rasmusvillemoes.dk>,
Sergey Senozhatsky <senozhatsky@chromium.org>,
Steven Rostedt <rostedt@goodmis.org>,
Tycho Andersen <tycho@tycho.pizza>,
Linus Torvalds <torvalds@linuxfoundation.org>
Subject: Re: [PATCH v2 2/2] kallsyms: Document why unresolved symbols are revealed
Date: Mon, 31 Aug 2026 15:37:29 +0200 [thread overview]
Message-ID: <20260831133729.p5ylO7O6@linutronix.de> (raw)
In-Reply-To: <20260827102411.NsziRXyJ@linutronix.de>
Could someone yell here, please? I convinced myself that hiding
unresolved pointers is the good thing to do here since a "wrong" pointer
would be seen in backtrace anyway.
Thread starts at
https://lore.kernel.org/all/20260821152614.2202196-1-bigeasy@linutronix.de/
On 2026-08-27 12:24:13 [+0200], To Petr Mladek wrote:
> On 2026-08-26 18:21:12 [+0200], Petr Mladek wrote:
> > Adding Linus into Cc.
> >
> > On Fri 2026-08-21 17:26:14, Sebastian Andrzej Siewior wrote:
> > > __sprint_symbol() is supposed to resolve the passed address to a symbol
> > > name. If the symbol can not be resolved it will print the actual pointer
> > > that was passed. The pointer policy is to not reveal actual pointer
> > > values. However for post-mortem analysis of crashes it is helpful to see
> > > the raw pointer if it is a corrupted pointer.
> > >
> > > Document why raw unresolved pointers are printed.
> >
> > > --- a/kernel/kallsyms.c
> > > +++ b/kernel/kallsyms.c
> > > @@ -482,8 +482,13 @@ static int __sprint_symbol(char *buffer, unsigned long address,
> > > address += symbol_offset;
> > > len = kallsyms_lookup_buildid(address, &size, &offset, &modname, &buildid,
> > > buffer);
> > > - if (!len)
> > > + if (!len) {
> > > + /*
> > > + * Print the raw pointer to allow post-mortem analysis of corrupted
> > > + * pointer in backtraces.
> >
> > This might be acceptable when the system is going to panic().
> > But is this formatting used only during panic?
>
> panic, bug, warning, stack backtrace, … everything using %p[sSB] where
> the symbol can not be resolved to an actual function.
>
> > > + */
> > > return sprintf(buffer, "0x%lx", address - symbol_offset);
> >
> > I expected that we would replace this by "%p" so that the pointer got
> > hashed by default. After all, we suggest to use %ps because it should
> > not leak pointers.
> >
> > Hmm, I see %ps or %pS used by many interfaces, like procfs, sysfs,
> > ftrace. Many of them are accessible only by root. Maybe, people expect
> > to see the valid pointers.
>
> For %p[sS] it should something in .text area either kernel or module. If
> it can not be resolved then something went south. This can be either a
> legitime case like in commit b86280aa48b67 ("kernel/kallsyms.c: fix %pB
> when there's no symbol at the address") or the IP gets invalid due to
> $reason and the system panics. Making this %p would mean you have to use
> unhashed pointers just to see the unresolved pointer in the crash case.
>
> > But we do not want to repeate the %pK eperience here. We could not
> > reliably check the access rights of the vsprintf() caller.
> > So, we should agree on the default behavior which does not
> > depend on the caller.
>
> We don't want that, yes.
>
> > And I think that we want to reduce the risk of leaking.
> > So, I would use %p here.
> >
> > If some callers really want to always print the real pointer when
> > the symbol is not resolved then we might add some modifier for
> > this, e.g. %p[SsB][R][p], where p would mean plain. But I am
> > not sure if we really want it.
>
> That sounds like too much. The general policy should be what we want. So
> if the IP gets to be entire nonsense I get
>
> | BUG: unable to handle page fault for address: 0000000012345678 11:52 [2/1960]
> | #PF: supervisor instruction fetch in kernel mode
> | #PF: error_code(0x0010) - not-present page
> | PGD 0 P4D 0
> | Oops: Oops: 0010 [#1] SMP NOPTI
> | CPU: 6 UID: 0 PID: 1915 Comm: kworker/6:2 Tainted: G W 7.2.0+ #72 PREEMPT_{RT,(lazy)}
> | Tainted: [W]=WARN
> | Hardware name: QEMU Standard PC (Q35 + ICH9, 2009), BIOS 2026.05-2 08/06/2026
> | Workqueue: events inv_func
> | RIP: 0010:unresolved
>
> The %pS that did get resolved (instead 0000000012345678).
>
> | Code: Unable to access opcode bytes at 0x1234564e.
> | RSP: 0018:ffffc900024c7e78 EFLAGS: 00010286
> | RAX: ffffffff812691a0 RBX: ffff8881080f2780 RCX: 0000000000000006
> | RDX: 0000000000000000 RSI: 000073746e657665 RDI: 0000000012345678
> | RBP: ffff8881000ada00 R08: 8080808080808080 R09: ffff8881080f2808
> | R10: ffff8881000964c0 R11: fefefefefefefeff R12: ffff88817bdaa500
> | R13: ffff8881000ada05 R14: 0000000000000000 R15: ffffffff8280e000
> | FS: 0000000000000000(0000) GS:ffff8881f8c09000(0000) knlGS:0000000000000000
> | CS: 0010 DS: 0000 ES: 0000 CR0: 0000000080050033
> | CR2: 0000000012345678 CR3: 000000010858d000 CR4: 00000000003506f0
>
> and on x86 we have CR2
>
> | Call Trace:
> | <TASK>
> | ? process_one_work+0x174/0x350
> …
>
> So given that "BUG:" here would print that information, the RIP %pS line
> might now have to.
> If I use a data pointer instead:
> | kernel tried to execute NX-protected page - exploit attempt? (uid: 0)
> | BUG: unable to handle page fault for address: ffffffff8280e000
> | #PF: supervisor instruction fetch in kernel mode
> | #PF: error_code(0x0011) - permissions violation
> | PGD 3049067 P4D 3049067 PUD 304a063 PMD 80000000030001e3
> | Oops: Oops: 0011 [#1] SMP NOPTI
> | CPU: 6 UID: 0 PID: 1731 Comm: kworker/6:2 Tainted: G W 7.2.0+ #73 PREEMPT_{RT,(lazy)}
> | Tainted: [W]=WARN
> | Hardware name: QEMU Standard PC (Q35 + ICH9, 2009), BIOS 2026.05-2 08/06/2026
> | Workqueue: events inv_func
> | RIP: 0010:unresolved
> | Code: 00 00 66 2e 0f 1f 84 00 00 00 00 00 66 2e 0f 1f 84 00 00 00 00 00 66 2e 0f 1f 84 00 00 00 00 00 66 2e 0f 1f 84 00 00 00 00 00 <00> 00 40 03 00 00 00 00 08 e0 80 82 ff ff ff ff 08 e0 80 82 ff ff
> | RSP: 0018:ffffc90001fc3e78 EFLAGS: 00010286
> | RAX: ffffffff812691a0 RBX: ffff88810364e240 RCX: 0000000000000006
> | RDX: 0000000000000000 RSI: 000073746e657665 RDI: ffffffff8280e000
> | RBP: ffff8881000ada00 R08: 8080808080808080 R09: ffff88810364e2c8
> | R10: ffff8881000964c0 R11: fefefefefefefeff R12: ffff88817bdaa500
> | R13: ffff8881000ada05 R14: 0000000000000000 R15: ffffffff8280e000
> | FS: 0000000000000000(0000) GS:ffff8881f8c09000(0000) knlGS:0000000000000000
> | CS: 0010 DS: 0000 ES: 0000 CR0: 0000000080050033
> | CR2: ffffffff8280e000 CR3: 0000000100ac6000 CR4: 00000000003506f0
> | Call Trace:
> | <TASK>
> | ? process_one_work+0x174/0x350
>
> given that, we could go back to (unknown) instead in the hashed case
> instead of printing the pointer. I see the same kind of output for
> arm64.
> The only question would if that is enough for stack trace. But if so, I
> would indeed suggest to replace it with "(unknown)" as I already had in
> v1.
>
> > > + }
> > >
> > > offset -= symbol_offset;
> >
> > Best Regards,
> > Petr
Sebastian
next prev parent reply other threads:[~2026-08-31 13:37 UTC|newest]
Thread overview: 16+ messages / expand[flat|nested] mbox.gz Atom feed top
2026-08-21 15:26 [PATCH v2 0/2] vsprintf: Don't leak pointers Sebastian Andrzej Siewior
2026-08-21 15:26 ` [PATCH v2 1/2] vsprintf: Don't leak pointers for %ps without KALLSYMS enabled Sebastian Andrzej Siewior
2026-08-26 15:56 ` Petr Mladek
2026-08-27 9:14 ` Sebastian Andrzej Siewior
2026-08-27 9:51 ` Petr Mladek
2026-08-31 13:30 ` Sebastian Andrzej Siewior
2026-08-31 14:22 ` Petr Mladek
2026-08-21 15:26 ` [PATCH v2 2/2] kallsyms: Document why unresolved symbols are revealed Sebastian Andrzej Siewior
2026-08-26 16:21 ` Petr Mladek
2026-08-27 10:24 ` Sebastian Andrzej Siewior
2026-08-31 13:37 ` Sebastian Andrzej Siewior [this message]
2026-08-31 16:49 ` Steven Rostedt
2026-09-01 7:23 ` Petr Mladek
2026-09-01 8:01 ` Sebastian Andrzej Siewior
2026-09-01 13:30 ` Steven Rostedt
2026-08-26 14:51 ` [PATCH v2 0/2] vsprintf: Don't leak pointers Sebastian Andrzej Siewior
Reply instructions:
You may reply publicly to this message via plain-text email
using any one of the following methods:
* Save the following mbox file, import it into your mail client,
and reply-to-all from there: mbox
Avoid top-posting and favor interleaved quoting:
https://en.wikipedia.org/wiki/Posting_style#Interleaved_style
* Reply using the --to, --cc, and --in-reply-to
switches of git-send-email(1):
git send-email \
--in-reply-to=20260831133729.p5ylO7O6@linutronix.de \
--to=bigeasy@linutronix.de \
--cc=akpm@linux-foundation.org \
--cc=andriy.shevchenko@linux.intel.com \
--cc=kees@kernel.org \
--cc=linux-hardening@vger.kernel.org \
--cc=linux-kernel@vger.kernel.org \
--cc=linux@rasmusvillemoes.dk \
--cc=peterz@infradead.org \
--cc=pmladek@suse.com \
--cc=rostedt@goodmis.org \
--cc=senozhatsky@chromium.org \
--cc=tglx@kernel.org \
--cc=torvalds@linuxfoundation.org \
--cc=tycho@tycho.pizza \
/path/to/YOUR_REPLY
https://kernel.org/pub/software/scm/git/docs/git-send-email.html
* If your mail client supports setting the In-Reply-To header
via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line
before the message body.
This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.