Re: [PATCH v3 0/6] implement KASLR for powerpc/fsl_booke/64

在 2020/3/2 16:47, Scott Wood 写道:

On Mon, 2020-03-02 at 15:12 +0800, Jason Yan wrote:

quoted

在 2020/3/2 11:24, Scott Wood 写道:

quoted

On Mon, 2020-03-02 at 10:17 +0800, Jason Yan wrote:

quoted

在 2020/3/1 6:54, Scott Wood 写道:

quoted

On Sat, 2020-02-29 at 15:27 +0800, Jason Yan wrote:

quoted

Turnning to %p may not be a good idea in this situation. So
for the REG logs printed when dumping stack, we can disable it when
KASLR is open. For the REG logs in other places like show_regs(),
only
privileged can trigger it, and they are not combind with a symbol,
so
I think it's ok to keep them.

diff --git a/arch/powerpc/kernel/process.c

b/arch/powerpc/kernel/process.c
index fad50db9dcf2..659c51f0739a 100644

--- a/arch/powerpc/kernel/process.c
+++ b/arch/powerpc/kernel/process.c

@@ -2068,7 +2068,10 @@ void show_stack(struct task_struct *tsk,

unsigned
long *stack)
                    newsp = stack[0];
                    ip = stack[STACK_FRAME_LR_SAVE];
                    if (!firstframe || ip != lr) {
-                       printk("["REG"] ["REG"] %pS", sp, ip, (void
*)ip);
+                       if (IS_ENABLED(CONFIG_RANDOMIZE_BASE))
+                               printk("%pS", (void *)ip);
+                       else
+                               printk("["REG"] ["REG"] %pS", sp,
ip,
(void *)ip);

This doesn't deal with "nokaslr" on the kernel command line.  It also
doesn't
seem like something that every callsite should have to opencode,
versus
having
an appropriate format specifier behaves as I described above (and I
still
don't see why that format specifier should not be "%p").

Actually I still do not understand why we should print the raw value
here. When KALLSYMS is enabled we have symbol name  and  offset like
put_cred_rcu+0x108/0x110, and when KALLSYMS is disabled we have the raw
address.

I'm more concerned about the stack address for wading through a raw stack
dump
(to find function call arguments, etc).  The return address does help
confirm
that I'm on the right stack frame though, and also makes looking up a line
number slightly easier than having to look up a symbol address and then
add
the offset (at least for non-module addresses).

As a random aside, the mismatch between Linux printing a hex offset and
GDB
using decimal in disassembly is annoying...

OK, I will send a RFC patch to add a new format specifier such as "%pk"
or change the exsiting "%pK" to print raw value of addresses when KASLR
is disabled and print hash value of addresses when KASLR is enabled.
Let's see what the printk guys would say :)

I'm not sure that a new format specifier is needed versus changing the
behavior of "%p", and "%pK" definitely doesn't seem suitable given that it's
intended to be more restricted than "%p" (see commit ef0010a30935de4).  The
question is whether there is a legitimate reason to hash in the absence of
kaslr.

The problem is that if we change the behavior of "%p", we have to turn
all exsiting "%p" to "%pK". Hashing is still reasonable when there is no
kaslr because some architectures support randomize at build time such as 
arm64.

-Scott



.