Re: [PATCH 11/14] MIPS: memblock: Print out kernel virtual mem layout

[Matt Redfearn]

Hi Serge,

On 19/01/18 14:27, Serge Semin wrote:
> On Fri, Jan 19, 2018 at 07:59:43AM +0000, Matt Redfearn <matt.redfearn@xxxxxxxx> wrote:
>
> Hello Matt,
>
> > Hi Serge,
> >
> >
> >
> > On 18/01/18 20:18, Serge Semin wrote:
> > > On Thu, Jan 18, 2018 at 12:03:03PM -0800, Florian Fainelli <f.fainelli@xxxxxxxxx> wrote:
> > > > On 01/17/2018 02:23 PM, Serge Semin wrote:
> > > > > It is useful to have the kernel virtual memory layout printed
> > > > > at boot time so to have the full information about the booted
> > > > > kernel. In some cases it might be unsafe to have virtual
> > > > > addresses freely visible in logs, so the %pK format is used if
> > > > > one want to hide them.
> > > > >
> > > > > Signed-off-by: Serge Semin <fancer.lancer@xxxxxxxxx>
> > > >
> > > > I personally like having that information because that helps debug and
> > > > have a quick reference, but there appears to be a trend to remove this
> > > > in the name of security:
> > > >
> > > > https://patchwork.kernel.org/patch/10124007/
> > > >
> > > > maybe hide this behind a configuration option?
> > >
> > > Yeah, arm code was the place I picked the function up.) But in my case
> > > I've used %pK so the pointers would disappear from logging when
> > > kptr_restrict sysctl is 1 or 2.
> > > I agree, that we might need to make the printouts optional. If there is
> > > any kernel config, which for instance increases the kernel security we
> > > could also use it or anything else to discard the printouts at compile
> > > time.
> >
> >
> > Certainly, when KASLR is active it would be preferable to hide this
> > information, so you could use CONFIG_RELOCATABLE. The existing KASLR stuff
> > additionally hides this kind of information behind CONFIG_DEBUG_KERNEL, so
> > that only people actively debugging the kernel see it:
> >
> > http://elixir.free-electrons.com/linux/v4.15-rc8/source/arch/mips/kernel/setup.c#L604
>
> Ok. I'll hide the printouts behind both of that config macros in the next patchset
> version.


Another thing to note - since ad67b74d2469d ("printk: hash addresses 
printed with %p") %pK at this time in the boot process is useless since 
the RNG is not sufficiently initialised and all prints end up being 
"(ptrval)". Hence after v4.15-rc2 we end up with output like:

[    0.000000] Kernel virtual memory layout:
[    0.000000]     lowmem  : 0x(ptrval) - 0x(ptrval)  ( 256 MB)
[    0.000000]       .text : 0x(ptrval) - 0x(ptrval)  (7374 kB)
[    0.000000]       .data : 0x(ptrval) - 0x(ptrval)  (1901 kB)
[    0.000000]       .init : 0x(ptrval) - 0x(ptrval)  (1600 kB)
[    0.000000]       .bss  : 0x(ptrval) - 0x(ptrval)  ( 415 kB)
[    0.000000]     vmalloc : 0x(ptrval) - 0x(ptrval)  (1023 MB)
[    0.000000]     fixmap  : 0x(ptrval) - 0x(ptrval)  (  68 kB)


The %px format specifier was added for cases such as this, where we 
really want to print the unmodified address. And as long as this 
function is suitably guarded to only do this when KASLR is deactivated / 
CONFIG_DEBUG_KERNEL is activated, etc, then we are not unwittingly 
leaking information - we are deliberately making it available.

Thanks,
Matt

> Regards,
> -Sergey
>
> > Thanks,
> > Matt
> >
> > > > --
> > > > Florian