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

[Pratyush Anand]

On 27/05/2016:12:18:56 PM, AKASHI Takahiro wrote:
> 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.

Not sure..When I see all the places where it has been used, it seems it can
accommodate *long* variable.

See, read_vmcoreinfo_long() of makedumpfile [1] , which is being used to read
"NUMBER" string.  This function uses strtol(). So I think, we must have similar
implementation if we intend to use VMCOREINFO_NUMBER().

[1] https://sourceforge.net/p/makedumpfile/code/ci/master/tree/makedumpfile.c#l2400

> 
> 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.

I think, we should not change existing VMCOREINFO_xxxx() macros of kernel,
because it will break the applications which rely on it's current structure.

ATM, I also do not see any urgency of defining a new macro which can have
implementation like vmcoreinfo_append_str("NUMBER(X)=%#llx\n", X), because
exiting macros is able to take care of the needs.

> 
> -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