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

[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]

 





在 2020/3/1 6:54, Scott Wood 写道:
On Sat, 2020-02-29 at 15:27 +0800, Jason Yan wrote:

在 2020/2/29 12:28, Scott Wood 写道:
On Fri, 2020-02-28 at 14:47 +0800, Jason Yan wrote:

在 2020/2/28 13:53, Scott Wood 写道:

I don't see any debug setting for %pK (or %p) to always print the
actual
address (closest is kptr_restrict=1 but that only works in certain
contexts)... from looking at the code it seems it hashes even if kaslr
is
entirely disabled?  Or am I missing something?


Yes, %pK (or %p) always hashes whether kaslr is disabled or not. So if
we want the real value of the address, we cannot use it. But if you only
want to distinguish if two pointers are the same, it's ok.

Am I the only one that finds this a bit crazy?  If you want to lock a
system
down then fine, but why wage war on debugging even when there's no
randomization going on?  Comparing two pointers for equality is not always
adequate.


AFAIK, %p hashing is only exist because of many legacy address printings
and force who really want the raw values to switch to %px or even %lx.
It's not the opposite of debugging. Raw address printing is not
forbidden, only people need to estimate the risk of adrdress leaks.

Yes, but I don't see any format specifier to switch to that will hash in a
randomized production environment, but not in a debug or other non-randomized
environment which seems like the ideal default for most debug output.


Sorry I have no idea why there is no format specifier considered for switching of randomized or non-randomized environment. May they think that raw address should not leak in non-randomized environment too. May be Kees or Tobin can answer this question.

Kees? Tobin?


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.

-Scott



.





[Index of Archives]     [Linux Samsung SoC]     [Linux Rockchip SoC]     [Linux Actions SoC]     [Linux for Synopsys ARC Processors]     [Linux NFS]     [Linux NILFS]     [Linux USB Devel]     [Video for Linux]     [Linux Audio Users]     [Yosemite News]     [Linux Kernel]     [Linux SCSI]


  Powered by Linux