Re: [Crash-utility] [PATCH v2] arm64: fix kernel memory map handling for kaslr-enabled kernel
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/7011e478aae3e568cc8dca15b6c...
>
> 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...
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