Re: [Crash-utility] [PATCH v2] arm64: fix kernel memory map handling for kaslr-enabled kernel

2016-05-26 Thread Pratyush Anand 
On 27/05/2016:01:03:56 PM, AKASHI Takahiro wrote:
> On Thu, May 26, 2016 at 03:09:55PM +0530, Pratyush Anand wrote:
> > On 26/05/2016:05:13:30 PM, AKASHI Takahiro wrote:
> > > On Wed, May 25, 2016 at 10:28:10AM -0400, Dave Anderson wrote:
> > 
> > [...]
> > 
> > > > > (This is the reason that I don't think we need a VMCOREINFO for
> > > > > CONFIG_RANDOMIZE_BASE.)
> > > > 
> > > > Well, as long as there's a way to avoid that unnecessary/confusing
> > > > warning message for non-KASLR new-vmemmap kernels.
> > > > 
> > > > I also wonder whether makedumpfile could use it?
> > > 
> > > -> Pratyush?
> > 
> > Since  second kernel's initrd cannot include a large file like VMLINUX, so
> > makedumpfile should also work without passing '-x vmlinux'.  Therefore,
> > makedumpfile will need some minimal symbol's values to be passed in vmcore.
> 
> I understand.
> (but I wonder why makedumpfile doesn't utilize System.map nor .config.)

So far makedumpfile has been designed to work only with vmcore as input in it's
minimal form (specially in second kernel).
+Atsushi & makedumpfile list: He will have better idea about it.

Moreover, there are few variables, whose values are needed in order to translate
phys to virt and vice versa. So, passing their symbol address would not be of
much help. For example, we need to know value of kimage_voffset for __pa(), and
so symbol address of kimage_voffset will not help.

> 
> > IIUC, then you need to pass CONFIG_RANDOMIZE_BASE in order to calculate
> > PHYS_OFFSET. If yes, then why not to pass PHYS_OFFSET itself into vmcore.
> 
> No, as I said in the discussions, I don't think that we need
> CONFIG_RANDOMIZE_BASE *if* we can read a kernel symbol's value
> from /proc/vmcore.

What I understand that, we can read only those symbols/variables from vmcore
which have been embedded using VMCOREINFO_ macros (if we do not pass
vmlinux, like in second kernel). Atsushi, please correct me if I am wrong.

> 
> > Following numbers in vmcore [1] is helping me out in implementing __pa() and
> > __va() for all page table sizes and levels.
> > 
> > VMCOREINFO_NUMBER(pgtable_levels);
> > VMCOREINFO_NUMBER(va_bits);
> > VMCOREINFO_NUMBER(page_shift);
> 
> Since We already have VMCOREINFO(PAGESIZE), we won't nedd page_shift.

Yes, agreed.

> As well, pgtable_levels can be determined by va_bits and PAGESIZE.
> See arch/arm64/Kconfig.

Yes, agreed.

> 
> > VMCOREINFO_NUMBER(phys_offset);
> > VMCOREINFO_NUMBER(config_kasan);
> 
> Let me ask some questions.
> * What kind of data in vmcore and how does makedumpfile access
>   without vmlinux nor System.map?

Well, I do not have the deep idea, again Atsushi can help here.

makedumpfile mainly compresses vmcore (ram image of panicked kernel),
additionally it also excludes unnecessary pages for analysis. It will need
symbol information to exclude unnecessary pages, where vmlinux is needed mainly.
So, normally we do not perform erase(exclude) functionality in second kernel. It
is being performed latter, on a compressed dumpfile.

> * Why do you need CONFIG_KASAN?

KASAN_SHADOW_SIZE is dependent on CONFIG_KASAN.
MODULES_VADDR is dependent on KASAN_SHADOW_SIZE.

We need MODULES_VADDR,VMALLOC_START,VMEMMAP_START and their END addresses in
order to define whether a virtual to physical translation can be obtained using
linear mapping or need to read from page table instead.

Thanks for the questions and inputs.  Those are helpful. :-)
~Pratyush
> 
> Thanks,
> -Takahiro AKASHI
> 
> > VMCOREINFO_NUMBER(kimage_voffset);
> > 
> > ~Pratyush
> > 
> > [1]
> > https://github.com/pratyushanand/linux/blob/7011e478aae3e568cc8dca15b6c128fe728416f7/arch/arm64/kernel/machine_kexec.c#L275
> > 
> > --
> > Crash-utility mailing list
> > Crash-utility@redhat.com
> > https://www.redhat.com/mailman/listinfo/crash-utility
> 
> -- 
> Thanks,
> -Takahiro AKASHI
> 
> --
> Crash-utility mailing list
> Crash-utility@redhat.com
> https://www.redhat.com/mailman/listinfo/crash-utility

--
Crash-utility mailing list
Crash-utility@redhat.com
https://www.redhat.com/mailman/listinfo/crash-utility

Re: [Crash-utility] [PATCH v1] arm64: fix kernel memory map handling for kaslr-enabled

2016-05-26 Thread AKASHI Takahiro 
On Wed, May 25, 2016 at 09:29:27AM -0400, Dave Anderson wrote:
> 
> 
> - Original Message -
> > On Tue, May 24, 2016 at 09:35:54AM -0400, Dave Anderson wrote:
> > > 
> > > 
> > > - Original Message -
> > > > > > 
> > > > > > Now that PHYS_OFFSET is defined as "memstart_addr",  we can get the 
> > > > > > value
> > > > > > if we can access this symbol (on a live system).
> > > > > 
> > > > > When PHYS_OFFSET/memstart_addr is bumped up from the actual base of 
> > > > > physical
> > > > > memory, is the physical memory below it no longer used/accessible?
> > > > 
> > > > Yes, it is! That is what Ard has managed to do in his series of
> > > > KASLR-related patches.
> > > 
> > > How is it accessed via a kernel virtual address?  Or is only accessible 
> > > when
> > > mapped into a vmalloc() or user-space address?
> > 
> > Through a linear(unity) mapping above PAGE_OFFSET.
> 
> I'm still confused...
> 
> Does PAGE_OFFSET+0 still refer to PHYS_OFFSET?

No, it doesn't if CONFIG_RANDOMIZE_RAM.
See a kernel boot message about "Virtual kernel memory layout"
That is why PHYS_OFFSET can now be negative.

Thanks,
Tahahiro AKASHI

> I'm trying to wrap my head around 
> how __virt_to_phys() would work for the memory below the bumped-up 
> PHYS_OFFSET.
> 
> Dave
> 
> 
>  
> 
> --
> Crash-utility mailing list
> Crash-utility@redhat.com
> https://www.redhat.com/mailman/listinfo/crash-utility

-- 
Thanks,
-Takahiro AKASHI

--
Crash-utility mailing list
Crash-utility@redhat.com
https://www.redhat.com/mailman/listinfo/crash-utility

Re: [Crash-utility] [PATCH v2] arm64: fix kernel memory map handling for kaslr-enabled kernel

2016-05-26 Thread AKASHI Takahiro 
On Thu, May 26, 2016 at 03:09:55PM +0530, Pratyush Anand wrote:
> On 26/05/2016:05:13:30 PM, AKASHI Takahiro wrote:
> > On Wed, May 25, 2016 at 10:28:10AM -0400, Dave Anderson wrote:
> 
> [...]
> 
> > > > (This is the reason that I don't think we need a VMCOREINFO for
> > > > CONFIG_RANDOMIZE_BASE.)
> > > 
> > > Well, as long as there's a way to avoid that unnecessary/confusing
> > > warning message for non-KASLR new-vmemmap kernels.
> > > 
> > > I also wonder whether makedumpfile could use it?
> > 
> > -> Pratyush?
> 
> Since  second kernel's initrd cannot include a large file like VMLINUX, so
> makedumpfile should also work without passing '-x vmlinux'.  Therefore,
> makedumpfile will need some minimal symbol's values to be passed in vmcore.

I understand.
(but I wonder why makedumpfile doesn't utilize System.map nor .config.)

> IIUC, then you need to pass CONFIG_RANDOMIZE_BASE in order to calculate
> PHYS_OFFSET. If yes, then why not to pass PHYS_OFFSET itself into vmcore.

No, as I said in the discussions, I don't think that we need
CONFIG_RANDOMIZE_BASE *if* we can read a kernel symbol's value
from /proc/vmcore.

> Following numbers in vmcore [1] is helping me out in implementing __pa() and
> __va() for all page table sizes and levels.
> 
> VMCOREINFO_NUMBER(pgtable_levels);
> VMCOREINFO_NUMBER(va_bits);
> VMCOREINFO_NUMBER(page_shift);

Since We already have VMCOREINFO(PAGESIZE), we won't nedd page_shift.
As well, pgtable_levels can be determined by va_bits and PAGESIZE.
See arch/arm64/Kconfig.

> VMCOREINFO_NUMBER(phys_offset);
> VMCOREINFO_NUMBER(config_kasan);

Let me ask some questions.
* What kind of data in vmcore and how does makedumpfile access
  without vmlinux nor System.map?
* Why do you need CONFIG_KASAN?

Thanks,
-Takahiro AKASHI

> VMCOREINFO_NUMBER(kimage_voffset);
> 
> ~Pratyush
> 
> [1]
> https://github.com/pratyushanand/linux/blob/7011e478aae3e568cc8dca15b6c128fe728416f7/arch/arm64/kernel/machine_kexec.c#L275
> 
> --
> Crash-utility mailing list
> Crash-utility@redhat.com
> https://www.redhat.com/mailman/listinfo/crash-utility

-- 
Thanks,
-Takahiro AKASHI

--
Crash-utility mailing list
Crash-utility@redhat.com
https://www.redhat.com/mailman/listinfo/crash-utility

Re: [Crash-utility] [PATCH v2] arm64: fix kernel memory map handling for kaslr-enabled kernel

2016-05-26 Thread AKASHI Takahiro 
On Thu, May 26, 2016 at 01:27:08PM +0530, Pratyush Anand wrote:
> On 26/05/2016:04:04:08 PM, AKASHI Takahiro wrote:
> > Pratyush,
> > 
> > Just for debug purpose.
> > Please add the following line to *your* arch_crash_save_vmcoreinfo():
> > > vmcoreinfo_append_str("NUMBER(kimage_voffset)=%llx\n", kimage_voffset);
> 
> Thanks for the pointer.
> 
> I did had VMCOREINFO_NUMBER(kimage_voffset) in arch_crash_save_vmcoreinfo().
> 
> https://github.com/pratyushanand/linux/commit/7011e478aae3e568cc8dca15b6c128fe728416f7#diff-cdf29c3b9471b9d813afc107dd154acdR291
> 
> But, I noticed that in crash code you have "ms->kimage_voffset = htol(string,
> QUIET, NULL);". So, the change you have suggested will work.

I know that. It is intentional.

> However, I think its preferable to use VMCOREINFO_NUMBER() macro instead.
> makedumpfile is able to calculate kimage_voffset correctly with that without 
> any
> issue.

I think that VMCOREINFO_NUMBER() is, at least originally, intended
to be used for a small *unsigned* integer.

I also know that PHYS_OFFSET can now be nagative in v4.6 on arm64.
Yet I'm thinking of adding "0x" as a prefix to VMCOREINOF string.

-Takahiro AKASHI

> I will suggest to take following modification in crash code:
> 
> diff --git a/arm64.c b/arm64.c
> index 6b97093..9397d6d 100644
> --- a/arm64.c
> +++ b/arm64.c
> @@ -122,7 +122,7 @@ arm64_init(int when)
> ms = machdep->machspec;
> if (!ms->kimage_voffset &&
> (string = pc->read_vmcoreinfo("NUMBER(kimage_voffset)"))) 
> {
> -   ms->kimage_voffset = htol(string, QUIET, NULL);
> +   ms->kimage_voffset = dtol(string, QUIET, NULL);
> free(string);
> }
> 
> diff --git a/tools.c b/tools.c
> index 384bebd..1383e43 100644
> --- a/tools.c
> +++ b/tools.c
> @@ -877,7 +877,7 @@ dtol(char *s, int flags, int *errptr)
>  s++;
> 
>  for (j = 0; s[j] != '\0'; j++)
> -if ((s[j] < '0' || s[j] > '9'))
> +if ( (s[j] != '-') && ((s[j] < '0' || s[j] > '9')))
>  break ;
> 
> if (s[j] != '\0') {
> 
> ~Pratyush
> 
> > 
> > I will add this to my next kdump patch.
> > 
> > Thanks,
> > -Takahiro AKASHI
> > 
> > 
> > On Thu, May 26, 2016 at 10:58:24AM +0530, Pratyush Anand wrote:
> > > On 24/05/2016:01:59:06 PM, Dave Anderson wrote:
> > > > 
> > > > 
> > > > - Original Message -
> > > > > Yet some issues, but ...
> > > > >
> > > > 
> > > > Hi Takahiro,
> > > > 
> > > > Here are my general comments on my testing of the v2 patch, followed 
> > > > by a few comments in the patch itself. 
> > > > 
> > > > First, the combination of the new memory map layout and KASLR is 
> > > > somewhat
> > > > confusing.  I am testing your patch on a 4.6.0-0.rc7.git2.1.fc25 kernel
> > > > that has this configuration:
> > > > 
> > > >   config-arm64:# CONFIG_RANDOMIZE_BASE is not set
> > > > 
> > > > So KASLR doesn't really enter into the picture.  But when bringing
> > > > up the crash session, it shows the "kaslr kernel" WARNING:
> > > > 
> > > >   # ./crash
> > > >   
> > > >   crash 7.1.5++
> > > >   Copyright (C) 2002-2016  Red Hat, Inc.
> > > >   Copyright (C) 2004, 2005, 2006, 2010  IBM Corporation
> > > >   Copyright (C) 1999-2006  Hewlett-Packard Co
> > > >   Copyright (C) 2005, 2006, 2011, 2012  Fujitsu Limited
> > > >   Copyright (C) 2006, 2007  VA Linux Systems Japan K.K.
> > > >   Copyright (C) 2005, 2011  NEC Corporation
> > > >   Copyright (C) 1999, 2002, 2007  Silicon Graphics, Inc.
> > > >   Copyright (C) 1999, 2000, 2001, 2002  Mission Critical Linux, Inc.
> > > >   This program is free software, covered by the GNU General Public 
> > > > License,
> > > >   and you are welcome to change it and/or distribute copies of it under
> > > >   certain conditions.  Enter "help copying" to see the conditions.
> > > >   This program has absolutely no warranty.  Enter "help warranty" for 
> > > > details.
> > > >
> > > >   WARNING: kimage_voffset not identified for kaslr kernel
> > > >   GNU gdb (GDB) 7.6
> > > >   Copyright (C) 2013 Free Software Foundation, Inc.
> > > >   License GPLv3+: GNU GPL version 3 or later 
> > > > 
> > > >   This is free software: you are free to change and redistribute it.
> > > >   There is NO WARRANTY, to the extent permitted by law.  Type "show 
> > > > copying"
> > > >   and "show warranty" for details.
> > > >   This GDB was configured as "aarch64-unknown-linux-gnu"...
> > > >   
> > > > KERNEL: 
> > > > /usr/lib/debug/lib/modules/4.6.0-0.rc7.git2.1.fc25.aarch64/vmlinux
> > > >   DUMPFILE: /dev/crash
> > > >   CPUS: 8
> > > >   DATE: Tue May 24 10:08:08 2016
> > > > UPTIME: 11 days, 18:32:41
> > > >   LOAD AVERAGE: 0.17, 0.09, 0.12
> > > >  TASKS: 197
> > > >   NODENAME: apm-mustang-ev3-36.khw.lab.eng.bos.redhat.com
> > > >RELEASE:

Re: [Crash-utility] [PATCH v2] arm64: fix kernel memory map handling for kaslr-enabled kernel

2016-05-26 Thread Dave Anderson 


- Original Message -

> > > > > 3) Backtracing a 'panic'ed task fails:
> > > > >  crash> bt
> > > > >  PID: 999TASK: ffe960081800  CPU: 0   COMMAND: "sh"
> > > > >  bt: WARNING: corrupt prstatus? pstate=0x8000, but no user frame 
> > > > > found
> > > > >  bt: WARNING: cannot determine starting stack frame for task 
> > > > > ffe960081800
> > > > > 
> > > > >   frame->pc indicates that it is a kernel address, so obviously 
> > > > > something
> > > > >   is wrong. I'm diggin into it.
> > > > 
> > > > Can you add a debug statement that displays frame->pc, frame->sp, and 
> > > > frame->fp?
> > > 
> > > I've already identified the cause.
> > > pt_regs->pstate was set mistakenly from "sprsr_el1" in panic().
> > > But I have a difficulty here to fix the problem as we have no direct way
> > > to retrieve a value of the *current* PSTATE.
> >  
> > Interesting.  I don't know what you mean by "sprsr_el1" in panic(), but

> > I just got a report from my github "issues", where the user injected a 
> > panic()
> > call into a kernel module and got the same error as above because of the 
> > pstate
> > value.  I asked him to put this hack into arm64.c:
> 
> spspr_el1 is a register that holds a value of PSTATE when an exception
> is trapped into EL1. Since panic() is called even in case of software
> bugs, it is not useful anyway.
> Currently I'm trying to save a "faked" current PSTATE into pt_regs->pstate
> and will add this fix in my next kdump patch series (v17).

OK thanks.  Mystery solved.  I think I'll still add the hack-around that prints
the warning message but allows it to continue if the sp, fp and pc appear 
correct.
It used to do that, but the new behavior was actually a regression that was 
introduced as part of this patch in crash-7.1.5:

Fix for the "bt" command to properly pull the stack and frame pointer
registers from the NT_PRSTATUS notes of 32-bit tasks running in
user-mode on ARM64.  Without the patch, the "bt" command utilizes
ptregs->sp and ptregs->regs[29] for 32-bit tasks instead of the
architecturally-mapped ptregs->regs[13] and ptregs->regs[11], which
yields unpredictable/invalid results, and possibly a segmentation
violation.
(drjo...@redhat.com)

Thanks,
  Dave






--
Crash-utility mailing list
Crash-utility@redhat.com
https://www.redhat.com/mailman/listinfo/crash-utility

Re: [Crash-utility] [PATCH v2] arm64: fix kernel memory map handling for kaslr-enabled kernel

2016-05-26 Thread Pratyush Anand 
On 26/05/2016:05:13:30 PM, AKASHI Takahiro wrote:
> On Wed, May 25, 2016 at 10:28:10AM -0400, Dave Anderson wrote:

[...]

> > > (This is the reason that I don't think we need a VMCOREINFO for
> > > CONFIG_RANDOMIZE_BASE.)
> > 
> > Well, as long as there's a way to avoid that unnecessary/confusing
> > warning message for non-KASLR new-vmemmap kernels.
> > 
> > I also wonder whether makedumpfile could use it?
> 
> -> Pratyush?

Since  second kernel's initrd cannot include a large file like VMLINUX, so
makedumpfile should also work without passing '-x vmlinux'.  Therefore,
makedumpfile will need some minimal symbol's values to be passed in vmcore.

IIUC, then you need to pass CONFIG_RANDOMIZE_BASE in order to calculate
PHYS_OFFSET. If yes, then why not to pass PHYS_OFFSET itself into vmcore.

Following numbers in vmcore [1] is helping me out in implementing __pa() and
__va() for all page table sizes and levels.

VMCOREINFO_NUMBER(pgtable_levels);
VMCOREINFO_NUMBER(va_bits);
VMCOREINFO_NUMBER(page_shift);
VMCOREINFO_NUMBER(phys_offset);
VMCOREINFO_NUMBER(config_kasan);
VMCOREINFO_NUMBER(kimage_voffset);

~Pratyush

[1]
https://github.com/pratyushanand/linux/blob/7011e478aae3e568cc8dca15b6c128fe728416f7/arch/arm64/kernel/machine_kexec.c#L275

--
Crash-utility mailing list
Crash-utility@redhat.com
https://www.redhat.com/mailman/listinfo/crash-utility

Re: [Crash-utility] [PATCH v2] arm64: fix kernel memory map handling for kaslr-enabled kernel

2016-05-26 Thread AKASHI Takahiro 
On Wed, May 25, 2016 at 10:28:10AM -0400, Dave Anderson wrote:
> 
> - Original Message -
> > On Tue, May 24, 2016 at 01:59:06PM -0400, Dave Anderson wrote:
> > > 
> > > - Original Message -
> > > > Yet some issues, but ...
> > > >
> > > 
> > > Hi Takahiro,
> > > 
> > > Here are my general comments on my testing of the v2 patch, followed
> > > by a few comments in the patch itself.
> > 
> > Thank you for your quick review/testing, again.
> > 
> > > First, the combination of the new memory map layout and KASLR is somewhat
> > > confusing.  I am testing your patch on a 4.6.0-0.rc7.git2.1.fc25 kernel
> > > that has this configuration:
> > 
> > Right. I use the "kaslr kernel" or "kaslr-enabled kernel" in two
> > meanings (almost interchangeably):
> >- kernel with a new memory map, that is v4.6 or later
> >- kernel that has ability of KASLR (configured with CONFIG_RANDOMIZE_RAM)
> > 
> > When we talked offline, you mentioned the possibility of backporting
> > KASLR to older kernel, so I hesitated to use "v4.6 or later".
> > But I think that we'd better use "v4.6 or later" for a clarification
> > for now.
>  
> Actually I was thinking more along the lines of the new vmemmap stuff
> being backported.  But it looks like that would be impossible without
> also bringing along "kimage_voffset".  

Yeah, a sbustantial amount of code on mm code must be changed, and it is
impractical to backport them.

> 
> > >   config-arm64:# CONFIG_RANDOMIZE_BASE is not set
> > > 
> > > So KASLR doesn't really enter into the picture.  But when bringing
> > > up the crash session, it shows the "kaslr kernel" WARNING:
> > > 
> > >   # ./crash
> > >   
> > >   crash 7.1.5++
> > >   Copyright (C) 2002-2016  Red Hat, Inc.
> > >   Copyright (C) 2004, 2005, 2006, 2010  IBM Corporation
> > >   Copyright (C) 1999-2006  Hewlett-Packard Co
> > >   Copyright (C) 2005, 2006, 2011, 2012  Fujitsu Limited
> > >   Copyright (C) 2006, 2007  VA Linux Systems Japan K.K.
> > >   Copyright (C) 2005, 2011  NEC Corporation
> > >   Copyright (C) 1999, 2002, 2007  Silicon Graphics, Inc.
> > >   Copyright (C) 1999, 2000, 2001, 2002  Mission Critical Linux, Inc.
> > >   This program is free software, covered by the GNU General Public 
> > > License,
> > >   and you are welcome to change it and/or distribute copies of it under
> > >   certain conditions.  Enter "help copying" to see the conditions.
> > >   This program has absolutely no warranty.  Enter "help warranty" for
> > >   details.
> > >
> > >   WARNING: kimage_voffset not identified for kaslr kernel
> > >   GNU gdb (GDB) 7.6
> > >   Copyright (C) 2013 Free Software Foundation, Inc.
> > >   License GPLv3+: GNU GPL version 3 or later
> > >   
> > >   This is free software: you are free to change and redistribute it.
> > >   There is NO WARRANTY, to the extent permitted by law.  Type "show
> > >   copying"
> > >   and "show warranty" for details.
> > >   This GDB was configured as "aarch64-unknown-linux-gnu"...
> > >   
> > > KERNEL:
> > > /usr/lib/debug/lib/modules/4.6.0-0.rc7.git2.1.fc25.aarch64/vmlinux
> > >   DUMPFILE: /dev/crash
> > >   CPUS: 8
> > >   DATE: Tue May 24 10:08:08 2016
> > > UPTIME: 11 days, 18:32:41
> > >   LOAD AVERAGE: 0.17, 0.09, 0.12
> > >  TASKS: 197
> > >   NODENAME: apm-mustang-ev3-36.khw.lab.eng.bos.redhat.com
> > >RELEASE: 4.6.0-0.rc7.git2.1.fc25.aarch64
> > >VERSION: #1 SMP Thu May 12 13:28:43 UTC 2016
> > >MACHINE: aarch64  (unknown Mhz)
> > > MEMORY: 16 GB
> > >PID: 7556
> > >COMMAND: "crash"
> > >   TASK: fe00beb45400  [THREAD_INFO: fe00beb98000]
> > >CPU: 7
> > >  STATE: TASK_RUNNING (ACTIVE)
> > >   
> > >   crash>
> > >   
> > > Why show that WARNING in this case?  I understant that it's not
> > > stashed during early initialization:
> > 
> > If we don't know kimage_voffset, we can't analyze the contents of
> > memory (live or kdump).
> 
> I understand.  But clearly that's not the case with my system above that
> does not have CONFIG_RANDOMIZE_BASE configured, so the warning message
> is nonsensical/confusing.  That's why I was speculating about perhaps
> having kdump export a VMCOREINFO_CONFIG(RANDOMIZE_BASE) so that it is 
> readily apparent.
> 
> > 
> > >   crash> help -m
> > >  flags: 104000c5
> > >  
> > > (KSYMS_START|VM_L2_64K|VMEMMAP|IRQ_STACKS|MACHDEP_BT_TEXT|NEW_VMEMMAP)
> > >   ... [ cut ] ...
> > >memory map layout: new  <--- BTW, this is
> > >redundant/not-a-member, and we've got NEW_VMEMMAP
> > >  VA_BITS: 42
> > >userspace_top: 0400
> > >  page_offset: fe00
> > >   vmalloc_start_addr: fc000800
> > >  vmalloc_end: fdff5ffe
> > >modules_vaddr: fc00
> > >  modules_end:

Re: [Crash-utility] [PATCH v2] arm64: fix kernel memory map handling for kaslr-enabled kernel

2016-05-26 Thread Pratyush Anand 
On 26/05/2016:04:04:08 PM, AKASHI Takahiro wrote:
> Pratyush,
> 
> Just for debug purpose.
> Please add the following line to *your* arch_crash_save_vmcoreinfo():
> > vmcoreinfo_append_str("NUMBER(kimage_voffset)=%llx\n", kimage_voffset);

Thanks for the pointer.

I did had VMCOREINFO_NUMBER(kimage_voffset) in arch_crash_save_vmcoreinfo().

https://github.com/pratyushanand/linux/commit/7011e478aae3e568cc8dca15b6c128fe728416f7#diff-cdf29c3b9471b9d813afc107dd154acdR291

But, I noticed that in crash code you have "ms->kimage_voffset = htol(string,
QUIET, NULL);". So, the change you have suggested will work.

However, I think its preferable to use VMCOREINFO_NUMBER() macro instead.
makedumpfile is able to calculate kimage_voffset correctly with that without any
issue.

I will suggest to take following modification in crash code:

diff --git a/arm64.c b/arm64.c
index 6b97093..9397d6d 100644
--- a/arm64.c
+++ b/arm64.c
@@ -122,7 +122,7 @@ arm64_init(int when)
ms = machdep->machspec;
if (!ms->kimage_voffset &&
(string = pc->read_vmcoreinfo("NUMBER(kimage_voffset)"))) {
-   ms->kimage_voffset = htol(string, QUIET, NULL);
+   ms->kimage_voffset = dtol(string, QUIET, NULL);
free(string);
}

diff --git a/tools.c b/tools.c
index 384bebd..1383e43 100644
--- a/tools.c
+++ b/tools.c
@@ -877,7 +877,7 @@ dtol(char *s, int flags, int *errptr)
 s++;

 for (j = 0; s[j] != '\0'; j++)
-if ((s[j] < '0' || s[j] > '9'))
+if ( (s[j] != '-') && ((s[j] < '0' || s[j] > '9')))
 break ;

if (s[j] != '\0') {

~Pratyush

> 
> I will add this to my next kdump patch.
> 
> Thanks,
> -Takahiro AKASHI
> 
> 
> On Thu, May 26, 2016 at 10:58:24AM +0530, Pratyush Anand wrote:
> > On 24/05/2016:01:59:06 PM, Dave Anderson wrote:
> > > 
> > > 
> > > - Original Message -
> > > > Yet some issues, but ...
> > > >
> > > 
> > > Hi Takahiro,
> > > 
> > > Here are my general comments on my testing of the v2 patch, followed 
> > > by a few comments in the patch itself. 
> > > 
> > > First, the combination of the new memory map layout and KASLR is somewhat
> > > confusing.  I am testing your patch on a 4.6.0-0.rc7.git2.1.fc25 kernel
> > > that has this configuration:
> > > 
> > >   config-arm64:# CONFIG_RANDOMIZE_BASE is not set
> > > 
> > > So KASLR doesn't really enter into the picture.  But when bringing
> > > up the crash session, it shows the "kaslr kernel" WARNING:
> > > 
> > >   # ./crash
> > >   
> > >   crash 7.1.5++
> > >   Copyright (C) 2002-2016  Red Hat, Inc.
> > >   Copyright (C) 2004, 2005, 2006, 2010  IBM Corporation
> > >   Copyright (C) 1999-2006  Hewlett-Packard Co
> > >   Copyright (C) 2005, 2006, 2011, 2012  Fujitsu Limited
> > >   Copyright (C) 2006, 2007  VA Linux Systems Japan K.K.
> > >   Copyright (C) 2005, 2011  NEC Corporation
> > >   Copyright (C) 1999, 2002, 2007  Silicon Graphics, Inc.
> > >   Copyright (C) 1999, 2000, 2001, 2002  Mission Critical Linux, Inc.
> > >   This program is free software, covered by the GNU General Public 
> > > License,
> > >   and you are welcome to change it and/or distribute copies of it under
> > >   certain conditions.  Enter "help copying" to see the conditions.
> > >   This program has absolutely no warranty.  Enter "help warranty" for 
> > > details.
> > >
> > >   WARNING: kimage_voffset not identified for kaslr kernel
> > >   GNU gdb (GDB) 7.6
> > >   Copyright (C) 2013 Free Software Foundation, Inc.
> > >   License GPLv3+: GNU GPL version 3 or later 
> > > 
> > >   This is free software: you are free to change and redistribute it.
> > >   There is NO WARRANTY, to the extent permitted by law.  Type "show 
> > > copying"
> > >   and "show warranty" for details.
> > >   This GDB was configured as "aarch64-unknown-linux-gnu"...
> > >   
> > > KERNEL: 
> > > /usr/lib/debug/lib/modules/4.6.0-0.rc7.git2.1.fc25.aarch64/vmlinux
> > >   DUMPFILE: /dev/crash
> > >   CPUS: 8
> > >   DATE: Tue May 24 10:08:08 2016
> > > UPTIME: 11 days, 18:32:41
> > >   LOAD AVERAGE: 0.17, 0.09, 0.12
> > >  TASKS: 197
> > >   NODENAME: apm-mustang-ev3-36.khw.lab.eng.bos.redhat.com
> > >RELEASE: 4.6.0-0.rc7.git2.1.fc25.aarch64
> > >VERSION: #1 SMP Thu May 12 13:28:43 UTC 2016
> > >MACHINE: aarch64  (unknown Mhz)
> > > MEMORY: 16 GB
> > >PID: 7556
> > >COMMAND: "crash"
> > >   TASK: fe00beb45400  [THREAD_INFO: fe00beb98000]
> > >CPU: 7
> > >  STATE: TASK_RUNNING (ACTIVE)
> > >   
> > >   crash>
> > 
> > Do not know whats missing, I am yet not able to reach "crash>" prompt. I 
> > have
> > crash utility having this patch on top of "commit
> > 8ceb1ac628bf6a0a7f0bbfff030ec93081bca4cd" [1]. I have kernel code 
> > v4.6+kexec-v15