* Re: [PATCH v2] x86_64, vmcoreinfo: Append 'page_offset_base' to vmcoreinfo
@ 2018-11-21 11:39 ` Borislav Petkov
0 siblings, 0 replies; 51+ messages in thread
From: Borislav Petkov @ 2018-11-21 11:39 UTC (permalink / raw)
To: Bhupesh Sharma
Cc: Kazuhito Hagio, Kees Cook, Baoquan He, x86, kexec, linux-kernel,
Omar Sandoval, Dave Anderson, James Morse, Thomas Gleixner,
bhupesh.linux, Ingo Molnar, linux-arm-kernel
+ Kees.
On Fri, Nov 16, 2018 at 03:17:49AM +0530, Bhupesh Sharma wrote:
> x86_64 kernel uses 'page_offset_base' variable to point to the
> start of direct mapping of all physical memory. This variable
> is also updated for KASLR boot cases, so this can be exported
> via vmcoreinfo as a standard ABI between kernel and user-space,
> to allow user-space utilities to use the same for calculating
> the start of direct mapping of all physical memory.
>
> 'arch/x86/kernel/head64.c' sets the same as:
> unsigned long page_offset_base __ro_after_init = __PAGE_OFFSET_BASE_L4;
>
> and also uses the same to indicate the base of KASLR regions on x86_64:
> static __initdata struct kaslr_memory_region {
> unsigned long *base;
> unsigned long size_tb;
> } kaslr_regions[] = {
> { &page_offset_base, 0 },
> .. snip ..
Why is that detail needed in the commit message?
> Adding 'page_offset_base' to the vmcoreinfo can be specially useful for
> live-debugging of a running kernel via user-space utilities
> like makedumpfile (see [1]).
>
> Recently, I saw an issue with the 'makedumpfile' utility (see [2] for
Use passive tone in your commit message: no "we" or "I", etc.
Also, pls read section "2) Describe your changes" in
Documentation/process/submitting-patches.rst.
> details), whose live debugging feature is broken with newer kernels
> (I tested the same with 4.19-rc8+ kernel), as KCORE_REMAP segments were
> added to kcore, thus leading to an additional sections in the same, and
> makedumpfile is not longer able to determine the start of direct
> mapping of all physical memory, as it relies on traversing the PT_LOAD
> segments inside kcore and using the last PT_LOAD segment
> to determine the start of direct mapping.
>
> Such user-space issues can be resolved if the user-space code instead
> uses a standard ABI to read the kernel exposed machine specific
> variables. With the kernel commit 23c85094fe1895caefdd
> ["proc/kcore: add vmcoreinfo note to /proc/kcore"]), it is
ERROR: Please use git commit description style 'commit <12+ chars of sha1> ("<title line>")' - ie: 'commit 23c85094fe18 ("proc/kcore: add vmcoreinfo note to /proc/kcore")'
#54:
variables. With the kernel commit 23c85094fe1895caefdd
> now possible to use the vmcoreinfo present inside kcore as the standard
> ABI which can be used by the user-space utilities for reading
> the machine specific information (and hence for debugging a
> live kernel).
>
> User-space utilities like makedumpfile, kexec-tools and crash
> are either already using this ABI or are discussing patches
> which look to add the same feature. This helps in simplifying the
> overall code and also in reducing code-rewrite across the
> user-space utilities for getting values of these kernel
> symbols/variables.
> Accordingly this patch allows appending 'page_offset_base' for
> x86_64 platforms to vmcoreinfo, so that user-space tools can use the
> same as a standard interface to determine the start of direct mapping
> of all physical memory.
>
> Testing:
> -------
> - I tested this patch (rebased on 'linux-next') on a x86_64 machine
> using the modified 'makedumpfile' user-space code (see [3] for my
> github tree which contains the same) for determining how many pages
> are dumpable when different dump_level is specified (which is
> one use-case of live-debugging via 'makedumpfile').
> - I tested both the KASLR and non-KASLR boot cases with this patch.
> - Here is one sample log (for KASLR boot case) on my x86_64 machine:
>
> < snip..>
> The kernel doesn't support mmap(),read() will be used instead.
>
> TYPE PAGES EXCLUDABLE DESCRIPTION
> ----------------------------------------------------------------------
> ZERO 21299 yes Pages filled
> with zero
> NON_PRI_CACHE 91785 yes Cache
> pages without private flag
> PRI_CACHE 1 yes Cache pages with
> private flag
> USER 14057 yes User process
> pages
> FREE 740346 yes Free pages
> KERN_DATA 58152 no Dumpable kernel
> data
>
> page size: 4096
> Total pages on system: 925640
> Total size on system: 3791421440 Byte
>
> [1]. MAN pages -> MAKEDUMPFILE(8) and CRASH(8)
> [2]. makedumpfile issue with latest kernels -> http://lists.infradead.org/pipermail/kexec/2018-October/021769.html
> [3]. https://github.com/bhupesh-sharma/makedumpfile/tree/add-page-offset-base-to-vmcore-v1
>
> Cc: Boris Petkov <bp@alien8.de>
> Cc: Baoquan He <bhe@redhat.com>
> Cc: Ingo Molnar <mingo@kernel.org>
> Cc: Thomas Gleixner <tglx@linutronix.de>
> Cc: Kazuhito Hagio <k-hagio@ab.jp.nec.com>
> Cc: Dave Anderson <anderson@redhat.com>
> Cc: James Morse <james.morse@arm.com>
> Cc: Omar Sandoval <osandov@fb.com>
> Cc: x86@kernel.org
> Cc: kexec@lists.infradead.org
> Cc: linux-arm-kernel@lists.infradead.org
> Signed-off-by: Bhupesh Sharma <bhsharma@redhat.com>
> ---
> Changes since v1:
> - Fixed the build issue reported by build bot and tested this version
> with 'make allmodconfig'.
> - Reworded most of the commit log to explain the intent behind the
> patch.
>
> arch/x86/kernel/machine_kexec_64.c | 3 +++
> 1 file changed, 3 insertions(+)
>
> diff --git a/arch/x86/kernel/machine_kexec_64.c b/arch/x86/kernel/machine_kexec_64.c
> index 4c8acdfdc5a7..6161d77c5bfb 100644
> --- a/arch/x86/kernel/machine_kexec_64.c
> +++ b/arch/x86/kernel/machine_kexec_64.c
> @@ -356,6 +356,9 @@ void arch_crash_save_vmcoreinfo(void)
> VMCOREINFO_SYMBOL(init_top_pgt);
> vmcoreinfo_append_str("NUMBER(pgtable_l5_enabled)=%d\n",
> pgtable_l5_enabled());
> +#ifdef CONFIG_RANDOMIZE_BASE
> + VMCOREINFO_NUMBER(page_offset_base);
> +#endif
>
> #ifdef CONFIG_NUMA
> VMCOREINFO_SYMBOL(node_data);
> --
All above are only nitpicks though.
My opinion is this: people are exporting all kinds of kernel-internal
stuff in vmcoreinfo and frankly, I'm not crazy about this idea.
And AFAICT, this thing basically bypasses KASLR completely but you need
root for it so it probably doesn't really matter.
Now, on another thread we agreed more or less that what gets exported in
vmcoreinfo is so tightly coupled to the running kernel so that it is not
even considered an ABI. I guess that is debatable but whatever.
So my only request right now would be to have all those things being
exported, documented somewhere and I believe Lianbo is working on that.
But I'm sure others will have more to say about it.
--
Regards/Gruss,
Boris.
Good mailing practices for 400: avoid top-posting and trim the reply.
_______________________________________________
kexec mailing list
kexec@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/kexec
^ permalink raw reply [flat|nested] 51+ messages in thread
* [PATCH v2] x86_64, vmcoreinfo: Append 'page_offset_base' to vmcoreinfo
@ 2018-11-21 11:39 ` Borislav Petkov
0 siblings, 0 replies; 51+ messages in thread
From: Borislav Petkov @ 2018-11-21 11:39 UTC (permalink / raw)
To: linux-arm-kernel
+ Kees.
On Fri, Nov 16, 2018 at 03:17:49AM +0530, Bhupesh Sharma wrote:
> x86_64 kernel uses 'page_offset_base' variable to point to the
> start of direct mapping of all physical memory. This variable
> is also updated for KASLR boot cases, so this can be exported
> via vmcoreinfo as a standard ABI between kernel and user-space,
> to allow user-space utilities to use the same for calculating
> the start of direct mapping of all physical memory.
>
> 'arch/x86/kernel/head64.c' sets the same as:
> unsigned long page_offset_base __ro_after_init = __PAGE_OFFSET_BASE_L4;
>
> and also uses the same to indicate the base of KASLR regions on x86_64:
> static __initdata struct kaslr_memory_region {
> unsigned long *base;
> unsigned long size_tb;
> } kaslr_regions[] = {
> { &page_offset_base, 0 },
> .. snip ..
Why is that detail needed in the commit message?
> Adding 'page_offset_base' to the vmcoreinfo can be specially useful for
> live-debugging of a running kernel via user-space utilities
> like makedumpfile (see [1]).
>
> Recently, I saw an issue with the 'makedumpfile' utility (see [2] for
Use passive tone in your commit message: no "we" or "I", etc.
Also, pls read section "2) Describe your changes" in
Documentation/process/submitting-patches.rst.
> details), whose live debugging feature is broken with newer kernels
> (I tested the same with 4.19-rc8+ kernel), as KCORE_REMAP segments were
> added to kcore, thus leading to an additional sections in the same, and
> makedumpfile is not longer able to determine the start of direct
> mapping of all physical memory, as it relies on traversing the PT_LOAD
> segments inside kcore and using the last PT_LOAD segment
> to determine the start of direct mapping.
>
> Such user-space issues can be resolved if the user-space code instead
> uses a standard ABI to read the kernel exposed machine specific
> variables. With the kernel commit 23c85094fe1895caefdd
> ["proc/kcore: add vmcoreinfo note to /proc/kcore"]), it is
ERROR: Please use git commit description style 'commit <12+ chars of sha1> ("<title line>")' - ie: 'commit 23c85094fe18 ("proc/kcore: add vmcoreinfo note to /proc/kcore")'
#54:
variables. With the kernel commit 23c85094fe1895caefdd
> now possible to use the vmcoreinfo present inside kcore as the standard
> ABI which can be used by the user-space utilities for reading
> the machine specific information (and hence for debugging a
> live kernel).
>
> User-space utilities like makedumpfile, kexec-tools and crash
> are either already using this ABI or are discussing patches
> which look to add the same feature. This helps in simplifying the
> overall code and also in reducing code-rewrite across the
> user-space utilities for getting values of these kernel
> symbols/variables.
> Accordingly this patch allows appending 'page_offset_base' for
> x86_64 platforms to vmcoreinfo, so that user-space tools can use the
> same as a standard interface to determine the start of direct mapping
> of all physical memory.
>
> Testing:
> -------
> - I tested this patch (rebased on 'linux-next') on a x86_64 machine
> using the modified 'makedumpfile' user-space code (see [3] for my
> github tree which contains the same) for determining how many pages
> are dumpable when different dump_level is specified (which is
> one use-case of live-debugging via 'makedumpfile').
> - I tested both the KASLR and non-KASLR boot cases with this patch.
> - Here is one sample log (for KASLR boot case) on my x86_64 machine:
>
> < snip..>
> The kernel doesn't support mmap(),read() will be used instead.
>
> TYPE PAGES EXCLUDABLE DESCRIPTION
> ----------------------------------------------------------------------
> ZERO 21299 yes Pages filled
> with zero
> NON_PRI_CACHE 91785 yes Cache
> pages without private flag
> PRI_CACHE 1 yes Cache pages with
> private flag
> USER 14057 yes User process
> pages
> FREE 740346 yes Free pages
> KERN_DATA 58152 no Dumpable kernel
> data
>
> page size: 4096
> Total pages on system: 925640
> Total size on system: 3791421440 Byte
>
> [1]. MAN pages -> MAKEDUMPFILE(8) and CRASH(8)
> [2]. makedumpfile issue with latest kernels -> http://lists.infradead.org/pipermail/kexec/2018-October/021769.html
> [3]. https://github.com/bhupesh-sharma/makedumpfile/tree/add-page-offset-base-to-vmcore-v1
>
> Cc: Boris Petkov <bp@alien8.de>
> Cc: Baoquan He <bhe@redhat.com>
> Cc: Ingo Molnar <mingo@kernel.org>
> Cc: Thomas Gleixner <tglx@linutronix.de>
> Cc: Kazuhito Hagio <k-hagio@ab.jp.nec.com>
> Cc: Dave Anderson <anderson@redhat.com>
> Cc: James Morse <james.morse@arm.com>
> Cc: Omar Sandoval <osandov@fb.com>
> Cc: x86 at kernel.org
> Cc: kexec at lists.infradead.org
> Cc: linux-arm-kernel at lists.infradead.org
> Signed-off-by: Bhupesh Sharma <bhsharma@redhat.com>
> ---
> Changes since v1:
> - Fixed the build issue reported by build bot and tested this version
> with 'make allmodconfig'.
> - Reworded most of the commit log to explain the intent behind the
> patch.
>
> arch/x86/kernel/machine_kexec_64.c | 3 +++
> 1 file changed, 3 insertions(+)
>
> diff --git a/arch/x86/kernel/machine_kexec_64.c b/arch/x86/kernel/machine_kexec_64.c
> index 4c8acdfdc5a7..6161d77c5bfb 100644
> --- a/arch/x86/kernel/machine_kexec_64.c
> +++ b/arch/x86/kernel/machine_kexec_64.c
> @@ -356,6 +356,9 @@ void arch_crash_save_vmcoreinfo(void)
> VMCOREINFO_SYMBOL(init_top_pgt);
> vmcoreinfo_append_str("NUMBER(pgtable_l5_enabled)=%d\n",
> pgtable_l5_enabled());
> +#ifdef CONFIG_RANDOMIZE_BASE
> + VMCOREINFO_NUMBER(page_offset_base);
> +#endif
>
> #ifdef CONFIG_NUMA
> VMCOREINFO_SYMBOL(node_data);
> --
All above are only nitpicks though.
My opinion is this: people are exporting all kinds of kernel-internal
stuff in vmcoreinfo and frankly, I'm not crazy about this idea.
And AFAICT, this thing basically bypasses KASLR completely but you need
root for it so it probably doesn't really matter.
Now, on another thread we agreed more or less that what gets exported in
vmcoreinfo is so tightly coupled to the running kernel so that it is not
even considered an ABI. I guess that is debatable but whatever.
So my only request right now would be to have all those things being
exported, documented somewhere and I believe Lianbo is working on that.
But I'm sure others will have more to say about it.
--
Regards/Gruss,
Boris.
Good mailing practices for 400: avoid top-posting and trim the reply.
^ permalink raw reply [flat|nested] 51+ messages in thread
* Re: [PATCH v2] x86_64, vmcoreinfo: Append 'page_offset_base' to vmcoreinfo
2018-11-21 11:39 ` Borislav Petkov
(?)
@ 2018-11-24 20:06 ` Bhupesh Sharma
-1 siblings, 0 replies; 51+ messages in thread
From: Bhupesh Sharma @ 2018-11-24 20:06 UTC (permalink / raw)
To: Borislav Petkov
Cc: Linux Kernel Mailing List, Bhupesh SHARMA, Baoquan He,
Ingo Molnar, Thomas Gleixner, Kazuhito Hagio, Dave Anderson,
James Morse, Omar Sandoval, x86, kexec mailing list,
linux-arm-kernel, Kees Cook
Hi Boris,
Thanks for your review. Please see my replies inline:
On Wed, Nov 21, 2018 at 5:10 PM Borislav Petkov <bp@alien8.de> wrote:
>
> + Kees.
>
> On Fri, Nov 16, 2018 at 03:17:49AM +0530, Bhupesh Sharma wrote:
> > x86_64 kernel uses 'page_offset_base' variable to point to the
> > start of direct mapping of all physical memory. This variable
> > is also updated for KASLR boot cases, so this can be exported
> > via vmcoreinfo as a standard ABI between kernel and user-space,
> > to allow user-space utilities to use the same for calculating
> > the start of direct mapping of all physical memory.
> >
> > 'arch/x86/kernel/head64.c' sets the same as:
> > unsigned long page_offset_base __ro_after_init = __PAGE_OFFSET_BASE_L4;
> >
> > and also uses the same to indicate the base of KASLR regions on x86_64:
> > static __initdata struct kaslr_memory_region {
> > unsigned long *base;
> > unsigned long size_tb;
> > } kaslr_regions[] = {
> > { &page_offset_base, 0 },
> > .. snip ..
>
> Why is that detail needed in the commit message?
This (and similar) details were requested by Baoquan during the v1
review, that is why I added them to the v2 commit log.
Although personally I also think that such details are probably not
needed in a commit log (may be better suited for a cover letter, which
is maybe a overkill for this single patch).
Will drop this from v3.
> > Adding 'page_offset_base' to the vmcoreinfo can be specially useful for
> > live-debugging of a running kernel via user-space utilities
> > like makedumpfile (see [1]).
> >
> > Recently, I saw an issue with the 'makedumpfile' utility (see [2] for
>
> Use passive tone in your commit message: no "we" or "I", etc.
Ok.
> Also, pls read section "2) Describe your changes" in
> Documentation/process/submitting-patches.rst.
Ok.
> > details), whose live debugging feature is broken with newer kernels
> > (I tested the same with 4.19-rc8+ kernel), as KCORE_REMAP segments were
> > added to kcore, thus leading to an additional sections in the same, and
> > makedumpfile is not longer able to determine the start of direct
> > mapping of all physical memory, as it relies on traversing the PT_LOAD
> > segments inside kcore and using the last PT_LOAD segment
> > to determine the start of direct mapping.
> >
> > Such user-space issues can be resolved if the user-space code instead
> > uses a standard ABI to read the kernel exposed machine specific
> > variables. With the kernel commit 23c85094fe1895caefdd
> > ["proc/kcore: add vmcoreinfo note to /proc/kcore"]), it is
>
> ERROR: Please use git commit description style 'commit <12+ chars of sha1> ("<title line>")' - ie: 'commit 23c85094fe18 ("proc/kcore: add vmcoreinfo note to /proc/kcore")'
> #54:
> variables. With the kernel commit 23c85094fe1895caefdd
Ok.
> > now possible to use the vmcoreinfo present inside kcore as the standard
> > ABI which can be used by the user-space utilities for reading
> > the machine specific information (and hence for debugging a
> > live kernel).
> >
> > User-space utilities like makedumpfile, kexec-tools and crash
> > are either already using this ABI or are discussing patches
> > which look to add the same feature. This helps in simplifying the
> > overall code and also in reducing code-rewrite across the
> > user-space utilities for getting values of these kernel
> > symbols/variables.
>
> > Accordingly this patch allows appending 'page_offset_base' for
> > x86_64 platforms to vmcoreinfo, so that user-space tools can use the
> > same as a standard interface to determine the start of direct mapping
> > of all physical memory.
> >
> > Testing:
> > -------
> > - I tested this patch (rebased on 'linux-next') on a x86_64 machine
> > using the modified 'makedumpfile' user-space code (see [3] for my
> > github tree which contains the same) for determining how many pages
> > are dumpable when different dump_level is specified (which is
> > one use-case of live-debugging via 'makedumpfile').
> > - I tested both the KASLR and non-KASLR boot cases with this patch.
> > - Here is one sample log (for KASLR boot case) on my x86_64 machine:
> >
> > < snip..>
> > The kernel doesn't support mmap(),read() will be used instead.
> >
> > TYPE PAGES EXCLUDABLE DESCRIPTION
> > ----------------------------------------------------------------------
> > ZERO 21299 yes Pages filled
> > with zero
> > NON_PRI_CACHE 91785 yes Cache
> > pages without private flag
> > PRI_CACHE 1 yes Cache pages with
> > private flag
> > USER 14057 yes User process
> > pages
> > FREE 740346 yes Free pages
> > KERN_DATA 58152 no Dumpable kernel
> > data
> >
> > page size: 4096
> > Total pages on system: 925640
> > Total size on system: 3791421440 Byte
> >
> > [1]. MAN pages -> MAKEDUMPFILE(8) and CRASH(8)
> > [2]. makedumpfile issue with latest kernels -> http://lists.infradead.org/pipermail/kexec/2018-October/021769.html
> > [3]. https://github.com/bhupesh-sharma/makedumpfile/tree/add-page-offset-base-to-vmcore-v1
> >
> > Cc: Boris Petkov <bp@alien8.de>
> > Cc: Baoquan He <bhe@redhat.com>
> > Cc: Ingo Molnar <mingo@kernel.org>
> > Cc: Thomas Gleixner <tglx@linutronix.de>
> > Cc: Kazuhito Hagio <k-hagio@ab.jp.nec.com>
> > Cc: Dave Anderson <anderson@redhat.com>
> > Cc: James Morse <james.morse@arm.com>
> > Cc: Omar Sandoval <osandov@fb.com>
> > Cc: x86@kernel.org
> > Cc: kexec@lists.infradead.org
> > Cc: linux-arm-kernel@lists.infradead.org
> > Signed-off-by: Bhupesh Sharma <bhsharma@redhat.com>
> > ---
> > Changes since v1:
> > - Fixed the build issue reported by build bot and tested this version
> > with 'make allmodconfig'.
> > - Reworded most of the commit log to explain the intent behind the
> > patch.
> >
> > arch/x86/kernel/machine_kexec_64.c | 3 +++
> > 1 file changed, 3 insertions(+)
> >
> > diff --git a/arch/x86/kernel/machine_kexec_64.c b/arch/x86/kernel/machine_kexec_64.c
> > index 4c8acdfdc5a7..6161d77c5bfb 100644
> > --- a/arch/x86/kernel/machine_kexec_64.c
> > +++ b/arch/x86/kernel/machine_kexec_64.c
> > @@ -356,6 +356,9 @@ void arch_crash_save_vmcoreinfo(void)
> > VMCOREINFO_SYMBOL(init_top_pgt);
> > vmcoreinfo_append_str("NUMBER(pgtable_l5_enabled)=%d\n",
> > pgtable_l5_enabled());
> > +#ifdef CONFIG_RANDOMIZE_BASE
> > + VMCOREINFO_NUMBER(page_offset_base);
> > +#endif
> >
> > #ifdef CONFIG_NUMA
> > VMCOREINFO_SYMBOL(node_data);
> > --
>
> All above are only nitpicks though.
Right the above are mostly nitpicks, but useful ones, so I will
address these in the v3.
> My opinion is this: people are exporting all kinds of kernel-internal
> stuff in vmcoreinfo and frankly, I'm not crazy about this idea.
>
> And AFAICT, this thing basically bypasses KASLR completely but you need
> root for it so it probably doesn't really matter.
>
> Now, on another thread we agreed more or less that what gets exported in
> vmcoreinfo is so tightly coupled to the running kernel so that it is not
> even considered an ABI. I guess that is debatable but whatever.
I understand.
> So my only request right now would be to have all those things being
> exported, documented somewhere and I believe Lianbo is working on that.
I will work with Lianbo to get this added to his documentation patch as well.
> But I'm sure others will have more to say about it.
Sure, I will wait for a couple of days more and then send out a v3 and
also make sure that
this variable being export'ed also gets added to the overall documentation patch
which Lianbo is creating.
Regards,
Bhupesh
^ permalink raw reply [flat|nested] 51+ messages in thread
* Re: [PATCH v2] x86_64, vmcoreinfo: Append 'page_offset_base' to vmcoreinfo
@ 2018-11-24 20:06 ` Bhupesh Sharma
0 siblings, 0 replies; 51+ messages in thread
From: Bhupesh Sharma @ 2018-11-24 20:06 UTC (permalink / raw)
To: Borislav Petkov
Cc: Kazuhito Hagio, Kees Cook, Baoquan He, x86, kexec mailing list,
Linux Kernel Mailing List, Omar Sandoval, Dave Anderson,
James Morse, Thomas Gleixner, Bhupesh SHARMA, Ingo Molnar,
linux-arm-kernel
Hi Boris,
Thanks for your review. Please see my replies inline:
On Wed, Nov 21, 2018 at 5:10 PM Borislav Petkov <bp@alien8.de> wrote:
>
> + Kees.
>
> On Fri, Nov 16, 2018 at 03:17:49AM +0530, Bhupesh Sharma wrote:
> > x86_64 kernel uses 'page_offset_base' variable to point to the
> > start of direct mapping of all physical memory. This variable
> > is also updated for KASLR boot cases, so this can be exported
> > via vmcoreinfo as a standard ABI between kernel and user-space,
> > to allow user-space utilities to use the same for calculating
> > the start of direct mapping of all physical memory.
> >
> > 'arch/x86/kernel/head64.c' sets the same as:
> > unsigned long page_offset_base __ro_after_init = __PAGE_OFFSET_BASE_L4;
> >
> > and also uses the same to indicate the base of KASLR regions on x86_64:
> > static __initdata struct kaslr_memory_region {
> > unsigned long *base;
> > unsigned long size_tb;
> > } kaslr_regions[] = {
> > { &page_offset_base, 0 },
> > .. snip ..
>
> Why is that detail needed in the commit message?
This (and similar) details were requested by Baoquan during the v1
review, that is why I added them to the v2 commit log.
Although personally I also think that such details are probably not
needed in a commit log (may be better suited for a cover letter, which
is maybe a overkill for this single patch).
Will drop this from v3.
> > Adding 'page_offset_base' to the vmcoreinfo can be specially useful for
> > live-debugging of a running kernel via user-space utilities
> > like makedumpfile (see [1]).
> >
> > Recently, I saw an issue with the 'makedumpfile' utility (see [2] for
>
> Use passive tone in your commit message: no "we" or "I", etc.
Ok.
> Also, pls read section "2) Describe your changes" in
> Documentation/process/submitting-patches.rst.
Ok.
> > details), whose live debugging feature is broken with newer kernels
> > (I tested the same with 4.19-rc8+ kernel), as KCORE_REMAP segments were
> > added to kcore, thus leading to an additional sections in the same, and
> > makedumpfile is not longer able to determine the start of direct
> > mapping of all physical memory, as it relies on traversing the PT_LOAD
> > segments inside kcore and using the last PT_LOAD segment
> > to determine the start of direct mapping.
> >
> > Such user-space issues can be resolved if the user-space code instead
> > uses a standard ABI to read the kernel exposed machine specific
> > variables. With the kernel commit 23c85094fe1895caefdd
> > ["proc/kcore: add vmcoreinfo note to /proc/kcore"]), it is
>
> ERROR: Please use git commit description style 'commit <12+ chars of sha1> ("<title line>")' - ie: 'commit 23c85094fe18 ("proc/kcore: add vmcoreinfo note to /proc/kcore")'
> #54:
> variables. With the kernel commit 23c85094fe1895caefdd
Ok.
> > now possible to use the vmcoreinfo present inside kcore as the standard
> > ABI which can be used by the user-space utilities for reading
> > the machine specific information (and hence for debugging a
> > live kernel).
> >
> > User-space utilities like makedumpfile, kexec-tools and crash
> > are either already using this ABI or are discussing patches
> > which look to add the same feature. This helps in simplifying the
> > overall code and also in reducing code-rewrite across the
> > user-space utilities for getting values of these kernel
> > symbols/variables.
>
> > Accordingly this patch allows appending 'page_offset_base' for
> > x86_64 platforms to vmcoreinfo, so that user-space tools can use the
> > same as a standard interface to determine the start of direct mapping
> > of all physical memory.
> >
> > Testing:
> > -------
> > - I tested this patch (rebased on 'linux-next') on a x86_64 machine
> > using the modified 'makedumpfile' user-space code (see [3] for my
> > github tree which contains the same) for determining how many pages
> > are dumpable when different dump_level is specified (which is
> > one use-case of live-debugging via 'makedumpfile').
> > - I tested both the KASLR and non-KASLR boot cases with this patch.
> > - Here is one sample log (for KASLR boot case) on my x86_64 machine:
> >
> > < snip..>
> > The kernel doesn't support mmap(),read() will be used instead.
> >
> > TYPE PAGES EXCLUDABLE DESCRIPTION
> > ----------------------------------------------------------------------
> > ZERO 21299 yes Pages filled
> > with zero
> > NON_PRI_CACHE 91785 yes Cache
> > pages without private flag
> > PRI_CACHE 1 yes Cache pages with
> > private flag
> > USER 14057 yes User process
> > pages
> > FREE 740346 yes Free pages
> > KERN_DATA 58152 no Dumpable kernel
> > data
> >
> > page size: 4096
> > Total pages on system: 925640
> > Total size on system: 3791421440 Byte
> >
> > [1]. MAN pages -> MAKEDUMPFILE(8) and CRASH(8)
> > [2]. makedumpfile issue with latest kernels -> http://lists.infradead.org/pipermail/kexec/2018-October/021769.html
> > [3]. https://github.com/bhupesh-sharma/makedumpfile/tree/add-page-offset-base-to-vmcore-v1
> >
> > Cc: Boris Petkov <bp@alien8.de>
> > Cc: Baoquan He <bhe@redhat.com>
> > Cc: Ingo Molnar <mingo@kernel.org>
> > Cc: Thomas Gleixner <tglx@linutronix.de>
> > Cc: Kazuhito Hagio <k-hagio@ab.jp.nec.com>
> > Cc: Dave Anderson <anderson@redhat.com>
> > Cc: James Morse <james.morse@arm.com>
> > Cc: Omar Sandoval <osandov@fb.com>
> > Cc: x86@kernel.org
> > Cc: kexec@lists.infradead.org
> > Cc: linux-arm-kernel@lists.infradead.org
> > Signed-off-by: Bhupesh Sharma <bhsharma@redhat.com>
> > ---
> > Changes since v1:
> > - Fixed the build issue reported by build bot and tested this version
> > with 'make allmodconfig'.
> > - Reworded most of the commit log to explain the intent behind the
> > patch.
> >
> > arch/x86/kernel/machine_kexec_64.c | 3 +++
> > 1 file changed, 3 insertions(+)
> >
> > diff --git a/arch/x86/kernel/machine_kexec_64.c b/arch/x86/kernel/machine_kexec_64.c
> > index 4c8acdfdc5a7..6161d77c5bfb 100644
> > --- a/arch/x86/kernel/machine_kexec_64.c
> > +++ b/arch/x86/kernel/machine_kexec_64.c
> > @@ -356,6 +356,9 @@ void arch_crash_save_vmcoreinfo(void)
> > VMCOREINFO_SYMBOL(init_top_pgt);
> > vmcoreinfo_append_str("NUMBER(pgtable_l5_enabled)=%d\n",
> > pgtable_l5_enabled());
> > +#ifdef CONFIG_RANDOMIZE_BASE
> > + VMCOREINFO_NUMBER(page_offset_base);
> > +#endif
> >
> > #ifdef CONFIG_NUMA
> > VMCOREINFO_SYMBOL(node_data);
> > --
>
> All above are only nitpicks though.
Right the above are mostly nitpicks, but useful ones, so I will
address these in the v3.
> My opinion is this: people are exporting all kinds of kernel-internal
> stuff in vmcoreinfo and frankly, I'm not crazy about this idea.
>
> And AFAICT, this thing basically bypasses KASLR completely but you need
> root for it so it probably doesn't really matter.
>
> Now, on another thread we agreed more or less that what gets exported in
> vmcoreinfo is so tightly coupled to the running kernel so that it is not
> even considered an ABI. I guess that is debatable but whatever.
I understand.
> So my only request right now would be to have all those things being
> exported, documented somewhere and I believe Lianbo is working on that.
I will work with Lianbo to get this added to his documentation patch as well.
> But I'm sure others will have more to say about it.
Sure, I will wait for a couple of days more and then send out a v3 and
also make sure that
this variable being export'ed also gets added to the overall documentation patch
which Lianbo is creating.
Regards,
Bhupesh
_______________________________________________
kexec mailing list
kexec@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/kexec
^ permalink raw reply [flat|nested] 51+ messages in thread
* [PATCH v2] x86_64, vmcoreinfo: Append 'page_offset_base' to vmcoreinfo
@ 2018-11-24 20:06 ` Bhupesh Sharma
0 siblings, 0 replies; 51+ messages in thread
From: Bhupesh Sharma @ 2018-11-24 20:06 UTC (permalink / raw)
To: linux-arm-kernel
Hi Boris,
Thanks for your review. Please see my replies inline:
On Wed, Nov 21, 2018 at 5:10 PM Borislav Petkov <bp@alien8.de> wrote:
>
> + Kees.
>
> On Fri, Nov 16, 2018 at 03:17:49AM +0530, Bhupesh Sharma wrote:
> > x86_64 kernel uses 'page_offset_base' variable to point to the
> > start of direct mapping of all physical memory. This variable
> > is also updated for KASLR boot cases, so this can be exported
> > via vmcoreinfo as a standard ABI between kernel and user-space,
> > to allow user-space utilities to use the same for calculating
> > the start of direct mapping of all physical memory.
> >
> > 'arch/x86/kernel/head64.c' sets the same as:
> > unsigned long page_offset_base __ro_after_init = __PAGE_OFFSET_BASE_L4;
> >
> > and also uses the same to indicate the base of KASLR regions on x86_64:
> > static __initdata struct kaslr_memory_region {
> > unsigned long *base;
> > unsigned long size_tb;
> > } kaslr_regions[] = {
> > { &page_offset_base, 0 },
> > .. snip ..
>
> Why is that detail needed in the commit message?
This (and similar) details were requested by Baoquan during the v1
review, that is why I added them to the v2 commit log.
Although personally I also think that such details are probably not
needed in a commit log (may be better suited for a cover letter, which
is maybe a overkill for this single patch).
Will drop this from v3.
> > Adding 'page_offset_base' to the vmcoreinfo can be specially useful for
> > live-debugging of a running kernel via user-space utilities
> > like makedumpfile (see [1]).
> >
> > Recently, I saw an issue with the 'makedumpfile' utility (see [2] for
>
> Use passive tone in your commit message: no "we" or "I", etc.
Ok.
> Also, pls read section "2) Describe your changes" in
> Documentation/process/submitting-patches.rst.
Ok.
> > details), whose live debugging feature is broken with newer kernels
> > (I tested the same with 4.19-rc8+ kernel), as KCORE_REMAP segments were
> > added to kcore, thus leading to an additional sections in the same, and
> > makedumpfile is not longer able to determine the start of direct
> > mapping of all physical memory, as it relies on traversing the PT_LOAD
> > segments inside kcore and using the last PT_LOAD segment
> > to determine the start of direct mapping.
> >
> > Such user-space issues can be resolved if the user-space code instead
> > uses a standard ABI to read the kernel exposed machine specific
> > variables. With the kernel commit 23c85094fe1895caefdd
> > ["proc/kcore: add vmcoreinfo note to /proc/kcore"]), it is
>
> ERROR: Please use git commit description style 'commit <12+ chars of sha1> ("<title line>")' - ie: 'commit 23c85094fe18 ("proc/kcore: add vmcoreinfo note to /proc/kcore")'
> #54:
> variables. With the kernel commit 23c85094fe1895caefdd
Ok.
> > now possible to use the vmcoreinfo present inside kcore as the standard
> > ABI which can be used by the user-space utilities for reading
> > the machine specific information (and hence for debugging a
> > live kernel).
> >
> > User-space utilities like makedumpfile, kexec-tools and crash
> > are either already using this ABI or are discussing patches
> > which look to add the same feature. This helps in simplifying the
> > overall code and also in reducing code-rewrite across the
> > user-space utilities for getting values of these kernel
> > symbols/variables.
>
> > Accordingly this patch allows appending 'page_offset_base' for
> > x86_64 platforms to vmcoreinfo, so that user-space tools can use the
> > same as a standard interface to determine the start of direct mapping
> > of all physical memory.
> >
> > Testing:
> > -------
> > - I tested this patch (rebased on 'linux-next') on a x86_64 machine
> > using the modified 'makedumpfile' user-space code (see [3] for my
> > github tree which contains the same) for determining how many pages
> > are dumpable when different dump_level is specified (which is
> > one use-case of live-debugging via 'makedumpfile').
> > - I tested both the KASLR and non-KASLR boot cases with this patch.
> > - Here is one sample log (for KASLR boot case) on my x86_64 machine:
> >
> > < snip..>
> > The kernel doesn't support mmap(),read() will be used instead.
> >
> > TYPE PAGES EXCLUDABLE DESCRIPTION
> > ----------------------------------------------------------------------
> > ZERO 21299 yes Pages filled
> > with zero
> > NON_PRI_CACHE 91785 yes Cache
> > pages without private flag
> > PRI_CACHE 1 yes Cache pages with
> > private flag
> > USER 14057 yes User process
> > pages
> > FREE 740346 yes Free pages
> > KERN_DATA 58152 no Dumpable kernel
> > data
> >
> > page size: 4096
> > Total pages on system: 925640
> > Total size on system: 3791421440 Byte
> >
> > [1]. MAN pages -> MAKEDUMPFILE(8) and CRASH(8)
> > [2]. makedumpfile issue with latest kernels -> http://lists.infradead.org/pipermail/kexec/2018-October/021769.html
> > [3]. https://github.com/bhupesh-sharma/makedumpfile/tree/add-page-offset-base-to-vmcore-v1
> >
> > Cc: Boris Petkov <bp@alien8.de>
> > Cc: Baoquan He <bhe@redhat.com>
> > Cc: Ingo Molnar <mingo@kernel.org>
> > Cc: Thomas Gleixner <tglx@linutronix.de>
> > Cc: Kazuhito Hagio <k-hagio@ab.jp.nec.com>
> > Cc: Dave Anderson <anderson@redhat.com>
> > Cc: James Morse <james.morse@arm.com>
> > Cc: Omar Sandoval <osandov@fb.com>
> > Cc: x86 at kernel.org
> > Cc: kexec at lists.infradead.org
> > Cc: linux-arm-kernel at lists.infradead.org
> > Signed-off-by: Bhupesh Sharma <bhsharma@redhat.com>
> > ---
> > Changes since v1:
> > - Fixed the build issue reported by build bot and tested this version
> > with 'make allmodconfig'.
> > - Reworded most of the commit log to explain the intent behind the
> > patch.
> >
> > arch/x86/kernel/machine_kexec_64.c | 3 +++
> > 1 file changed, 3 insertions(+)
> >
> > diff --git a/arch/x86/kernel/machine_kexec_64.c b/arch/x86/kernel/machine_kexec_64.c
> > index 4c8acdfdc5a7..6161d77c5bfb 100644
> > --- a/arch/x86/kernel/machine_kexec_64.c
> > +++ b/arch/x86/kernel/machine_kexec_64.c
> > @@ -356,6 +356,9 @@ void arch_crash_save_vmcoreinfo(void)
> > VMCOREINFO_SYMBOL(init_top_pgt);
> > vmcoreinfo_append_str("NUMBER(pgtable_l5_enabled)=%d\n",
> > pgtable_l5_enabled());
> > +#ifdef CONFIG_RANDOMIZE_BASE
> > + VMCOREINFO_NUMBER(page_offset_base);
> > +#endif
> >
> > #ifdef CONFIG_NUMA
> > VMCOREINFO_SYMBOL(node_data);
> > --
>
> All above are only nitpicks though.
Right the above are mostly nitpicks, but useful ones, so I will
address these in the v3.
> My opinion is this: people are exporting all kinds of kernel-internal
> stuff in vmcoreinfo and frankly, I'm not crazy about this idea.
>
> And AFAICT, this thing basically bypasses KASLR completely but you need
> root for it so it probably doesn't really matter.
>
> Now, on another thread we agreed more or less that what gets exported in
> vmcoreinfo is so tightly coupled to the running kernel so that it is not
> even considered an ABI. I guess that is debatable but whatever.
I understand.
> So my only request right now would be to have all those things being
> exported, documented somewhere and I believe Lianbo is working on that.
I will work with Lianbo to get this added to his documentation patch as well.
> But I'm sure others will have more to say about it.
Sure, I will wait for a couple of days more and then send out a v3 and
also make sure that
this variable being export'ed also gets added to the overall documentation patch
which Lianbo is creating.
Regards,
Bhupesh
^ permalink raw reply [flat|nested] 51+ messages in thread
* Re: [PATCH v2] x86_64, vmcoreinfo: Append 'page_offset_base' to vmcoreinfo
2018-11-24 20:06 ` Bhupesh Sharma
(?)
@ 2018-11-25 10:19 ` Baoquan He
-1 siblings, 0 replies; 51+ messages in thread
From: Baoquan He @ 2018-11-25 10:19 UTC (permalink / raw)
To: Bhupesh Sharma
Cc: Borislav Petkov, Linux Kernel Mailing List, Bhupesh SHARMA,
Ingo Molnar, Thomas Gleixner, Kazuhito Hagio, Dave Anderson,
James Morse, Omar Sandoval, x86, kexec mailing list,
linux-arm-kernel, Kees Cook
On 11/25/18 at 01:36am, Bhupesh Sharma wrote:
> Hi Boris,
>
> Thanks for your review. Please see my replies inline:
>
> On Wed, Nov 21, 2018 at 5:10 PM Borislav Petkov <bp@alien8.de> wrote:
> >
> > + Kees.
> >
> > On Fri, Nov 16, 2018 at 03:17:49AM +0530, Bhupesh Sharma wrote:
> > > x86_64 kernel uses 'page_offset_base' variable to point to the
> > > start of direct mapping of all physical memory. This variable
> > > is also updated for KASLR boot cases, so this can be exported
> > > via vmcoreinfo as a standard ABI between kernel and user-space,
> > > to allow user-space utilities to use the same for calculating
> > > the start of direct mapping of all physical memory.
> > >
> > > 'arch/x86/kernel/head64.c' sets the same as:
> > > unsigned long page_offset_base __ro_after_init = __PAGE_OFFSET_BASE_L4;
> > >
> > > and also uses the same to indicate the base of KASLR regions on x86_64:
> > > static __initdata struct kaslr_memory_region {
> > > unsigned long *base;
> > > unsigned long size_tb;
> > > } kaslr_regions[] = {
> > > { &page_offset_base, 0 },
> > > .. snip ..
> >
> > Why is that detail needed in the commit message?
>
> This (and similar) details were requested by Baoquan during the v1
> review, that is why I added them to the v2 commit log.
Hmm, Bhupesh, this is what I requested in your v1:
lkml.kernel.org/r/20181030020752.GB11408@MiWiFi-R3L-srv
You said x86 makedumpfile is broken because KCORE_REMAP is added. I
wanted to know why it's broken. Now I think I don't need to request it
in this thread, becasue Kazu has made it clear and fixed it:
4AE2DC15AC0B8543882A74EA0D43DBEC0355DFC9@BPXM09GP.gisp.nec.co.jp
Or could you abstract the sentences in which I requested you to add
above information into patch log so that I can notice my expression
and can improve on future reviewing and communicating?
And for problem solving and describing, if things are simple, just
tell it direclty; If things are complex, try to simplify it and make it
be simply told. For this page_offset_base exporting, we can save time on
'what' and 'how' since it's very simple and very easy to see what it is
and how it's done. You just need tell WHY it's needed. BUT I saw so many
words and not easy to get why. If simplifying problems is one kind of
ability, I don't know what to say about complicating one simple problem,
another kind of ability?
I have read your patch log when you sent out v2, honestly I don't like it.
I tried to not touch this patch, hope other people can review and give
comments. This can make you know I didn't intentionally embarrass you on
patch reviewing.
Honestly, if this patch is merged, no matter v2 or your old v1,
a small record will be made. On my x1 laptop, I need scroll down FOUR
full screens till the real patch content is seen. Are those really
needed? You can check other vmcoreinfo exporting patch.
I really don't want to review people's patch if they don't welcome my
words, just I was asked to do. Hope you know I had no any offence when
I started to read your patch, just I felt not good when my head was
scratched to bleed when read. My english is not good, I don't know how to
express euphemistically sometime, so if I say so about one thing, that is
how I think so to the thing, without any personal prejudice or attack to
person. Please forgive me if any offence felt.
Thanks
Baoquan
> Although personally I also think that such details are probably not
> needed in a commit log (may be better suited for a cover letter, which
> is maybe a overkill for this single patch).
> Will drop this from v3.
^ permalink raw reply [flat|nested] 51+ messages in thread
* Re: [PATCH v2] x86_64, vmcoreinfo: Append 'page_offset_base' to vmcoreinfo
@ 2018-11-25 10:19 ` Baoquan He
0 siblings, 0 replies; 51+ messages in thread
From: Baoquan He @ 2018-11-25 10:19 UTC (permalink / raw)
To: Bhupesh Sharma
Cc: Kazuhito Hagio, James Morse, x86, kexec mailing list,
Linux Kernel Mailing List, Omar Sandoval, Borislav Petkov,
Dave Anderson, Thomas Gleixner, Bhupesh SHARMA, Ingo Molnar,
linux-arm-kernel, Kees Cook
On 11/25/18 at 01:36am, Bhupesh Sharma wrote:
> Hi Boris,
>
> Thanks for your review. Please see my replies inline:
>
> On Wed, Nov 21, 2018 at 5:10 PM Borislav Petkov <bp@alien8.de> wrote:
> >
> > + Kees.
> >
> > On Fri, Nov 16, 2018 at 03:17:49AM +0530, Bhupesh Sharma wrote:
> > > x86_64 kernel uses 'page_offset_base' variable to point to the
> > > start of direct mapping of all physical memory. This variable
> > > is also updated for KASLR boot cases, so this can be exported
> > > via vmcoreinfo as a standard ABI between kernel and user-space,
> > > to allow user-space utilities to use the same for calculating
> > > the start of direct mapping of all physical memory.
> > >
> > > 'arch/x86/kernel/head64.c' sets the same as:
> > > unsigned long page_offset_base __ro_after_init = __PAGE_OFFSET_BASE_L4;
> > >
> > > and also uses the same to indicate the base of KASLR regions on x86_64:
> > > static __initdata struct kaslr_memory_region {
> > > unsigned long *base;
> > > unsigned long size_tb;
> > > } kaslr_regions[] = {
> > > { &page_offset_base, 0 },
> > > .. snip ..
> >
> > Why is that detail needed in the commit message?
>
> This (and similar) details were requested by Baoquan during the v1
> review, that is why I added them to the v2 commit log.
Hmm, Bhupesh, this is what I requested in your v1:
lkml.kernel.org/r/20181030020752.GB11408@MiWiFi-R3L-srv
You said x86 makedumpfile is broken because KCORE_REMAP is added. I
wanted to know why it's broken. Now I think I don't need to request it
in this thread, becasue Kazu has made it clear and fixed it:
4AE2DC15AC0B8543882A74EA0D43DBEC0355DFC9@BPXM09GP.gisp.nec.co.jp
Or could you abstract the sentences in which I requested you to add
above information into patch log so that I can notice my expression
and can improve on future reviewing and communicating?
And for problem solving and describing, if things are simple, just
tell it direclty; If things are complex, try to simplify it and make it
be simply told. For this page_offset_base exporting, we can save time on
'what' and 'how' since it's very simple and very easy to see what it is
and how it's done. You just need tell WHY it's needed. BUT I saw so many
words and not easy to get why. If simplifying problems is one kind of
ability, I don't know what to say about complicating one simple problem,
another kind of ability?
I have read your patch log when you sent out v2, honestly I don't like it.
I tried to not touch this patch, hope other people can review and give
comments. This can make you know I didn't intentionally embarrass you on
patch reviewing.
Honestly, if this patch is merged, no matter v2 or your old v1,
a small record will be made. On my x1 laptop, I need scroll down FOUR
full screens till the real patch content is seen. Are those really
needed? You can check other vmcoreinfo exporting patch.
I really don't want to review people's patch if they don't welcome my
words, just I was asked to do. Hope you know I had no any offence when
I started to read your patch, just I felt not good when my head was
scratched to bleed when read. My english is not good, I don't know how to
express euphemistically sometime, so if I say so about one thing, that is
how I think so to the thing, without any personal prejudice or attack to
person. Please forgive me if any offence felt.
Thanks
Baoquan
> Although personally I also think that such details are probably not
> needed in a commit log (may be better suited for a cover letter, which
> is maybe a overkill for this single patch).
> Will drop this from v3.
_______________________________________________
kexec mailing list
kexec@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/kexec
^ permalink raw reply [flat|nested] 51+ messages in thread
* [PATCH v2] x86_64, vmcoreinfo: Append 'page_offset_base' to vmcoreinfo
@ 2018-11-25 10:19 ` Baoquan He
0 siblings, 0 replies; 51+ messages in thread
From: Baoquan He @ 2018-11-25 10:19 UTC (permalink / raw)
To: linux-arm-kernel
On 11/25/18 at 01:36am, Bhupesh Sharma wrote:
> Hi Boris,
>
> Thanks for your review. Please see my replies inline:
>
> On Wed, Nov 21, 2018 at 5:10 PM Borislav Petkov <bp@alien8.de> wrote:
> >
> > + Kees.
> >
> > On Fri, Nov 16, 2018 at 03:17:49AM +0530, Bhupesh Sharma wrote:
> > > x86_64 kernel uses 'page_offset_base' variable to point to the
> > > start of direct mapping of all physical memory. This variable
> > > is also updated for KASLR boot cases, so this can be exported
> > > via vmcoreinfo as a standard ABI between kernel and user-space,
> > > to allow user-space utilities to use the same for calculating
> > > the start of direct mapping of all physical memory.
> > >
> > > 'arch/x86/kernel/head64.c' sets the same as:
> > > unsigned long page_offset_base __ro_after_init = __PAGE_OFFSET_BASE_L4;
> > >
> > > and also uses the same to indicate the base of KASLR regions on x86_64:
> > > static __initdata struct kaslr_memory_region {
> > > unsigned long *base;
> > > unsigned long size_tb;
> > > } kaslr_regions[] = {
> > > { &page_offset_base, 0 },
> > > .. snip ..
> >
> > Why is that detail needed in the commit message?
>
> This (and similar) details were requested by Baoquan during the v1
> review, that is why I added them to the v2 commit log.
Hmm, Bhupesh, this is what I requested in your v1:
lkml.kernel.org/r/20181030020752.GB11408 at MiWiFi-R3L-srv
You said x86 makedumpfile is broken because KCORE_REMAP is added. I
wanted to know why it's broken. Now I think I don't need to request it
in this thread, becasue Kazu has made it clear and fixed it:
4AE2DC15AC0B8543882A74EA0D43DBEC0355DFC9 at BPXM09GP.gisp.nec.co.jp
Or could you abstract the sentences in which I requested you to add
above information into patch log so that I can notice my expression
and can improve on future reviewing and communicating?
And for problem solving and describing, if things are simple, just
tell it direclty; If things are complex, try to simplify it and make it
be simply told. For this page_offset_base exporting, we can save time on
'what' and 'how' since it's very simple and very easy to see what it is
and how it's done. You just need tell WHY it's needed. BUT I saw so many
words and not easy to get why. If simplifying problems is one kind of
ability, I don't know what to say about complicating one simple problem,
another kind of ability?
I have read your patch log when you sent out v2, honestly I don't like it.
I tried to not touch this patch, hope other people can review and give
comments. This can make you know I didn't intentionally embarrass you on
patch reviewing.
Honestly, if this patch is merged, no matter v2 or your old v1,
a small record will be made. On my x1 laptop, I need scroll down FOUR
full screens till the real patch content is seen. Are those really
needed? You can check other vmcoreinfo exporting patch.
I really don't want to review people's patch if they don't welcome my
words, just I was asked to do. Hope you know I had no any offence when
I started to read your patch, just I felt not good when my head was
scratched to bleed when read. My english is not good, I don't know how to
express euphemistically sometime, so if I say so about one thing, that is
how I think so to the thing, without any personal prejudice or attack to
person. Please forgive me if any offence felt.
Thanks
Baoquan
> Although personally I also think that such details are probably not
> needed in a commit log (may be better suited for a cover letter, which
> is maybe a overkill for this single patch).
> Will drop this from v3.
^ permalink raw reply [flat|nested] 51+ messages in thread
* Re: [PATCH v2] x86_64, vmcoreinfo: Append 'page_offset_base' to vmcoreinfo
2018-11-21 11:39 ` Borislav Petkov
(?)
@ 2018-11-27 22:16 ` Kees Cook
-1 siblings, 0 replies; 51+ messages in thread
From: Kees Cook @ 2018-11-27 22:16 UTC (permalink / raw)
To: Borislav Petkov
Cc: Bhupesh Sharma, LKML, Bhupesh SHARMA, Baoquan He, Ingo Molnar,
Thomas Gleixner, Kazuhito Hagio, Dave Anderson, James Morse,
Omar Sandoval, X86 ML, Kexec Mailing List, linux-arm-kernel
On Wed, Nov 21, 2018 at 3:39 AM, Borislav Petkov <bp@alien8.de> wrote:
> + Kees.
>
> On Fri, Nov 16, 2018 at 03:17:49AM +0530, Bhupesh Sharma wrote:
>> x86_64 kernel uses 'page_offset_base' variable to point to the
>> start of direct mapping of all physical memory. This variable
>> is also updated for KASLR boot cases, so this can be exported
>> via vmcoreinfo as a standard ABI between kernel and user-space,
>> to allow user-space utilities to use the same for calculating
>> the start of direct mapping of all physical memory.
Why is KERNELOFFSET= not sufficient?
See commit b6085a865762 ("x86, kaslr: export offset in VMCOREINFO ELF notes")
+ vmcoreinfo_append_str("KERNELOFFSET=%lx\n",
+ (unsigned long)&_text - __START_KERNEL);
-Kees
>> diff --git a/arch/x86/kernel/machine_kexec_64.c b/arch/x86/kernel/machine_kexec_64.c
>> index 4c8acdfdc5a7..6161d77c5bfb 100644
>> --- a/arch/x86/kernel/machine_kexec_64.c
>> +++ b/arch/x86/kernel/machine_kexec_64.c
>> @@ -356,6 +356,9 @@ void arch_crash_save_vmcoreinfo(void)
>> VMCOREINFO_SYMBOL(init_top_pgt);
>> vmcoreinfo_append_str("NUMBER(pgtable_l5_enabled)=%d\n",
>> pgtable_l5_enabled());
>> +#ifdef CONFIG_RANDOMIZE_BASE
>> + VMCOREINFO_NUMBER(page_offset_base);
>> +#endif
>>
>> #ifdef CONFIG_NUMA
>> VMCOREINFO_SYMBOL(node_data);
--
Kees Cook
^ permalink raw reply [flat|nested] 51+ messages in thread
* Re: [PATCH v2] x86_64, vmcoreinfo: Append 'page_offset_base' to vmcoreinfo
@ 2018-11-27 22:16 ` Kees Cook
0 siblings, 0 replies; 51+ messages in thread
From: Kees Cook @ 2018-11-27 22:16 UTC (permalink / raw)
To: Borislav Petkov
Cc: Kazuhito Hagio, Baoquan He, Bhupesh Sharma, X86 ML,
Kexec Mailing List, LKML, Omar Sandoval, Dave Anderson,
James Morse, Thomas Gleixner, Bhupesh SHARMA, Ingo Molnar,
linux-arm-kernel
On Wed, Nov 21, 2018 at 3:39 AM, Borislav Petkov <bp@alien8.de> wrote:
> + Kees.
>
> On Fri, Nov 16, 2018 at 03:17:49AM +0530, Bhupesh Sharma wrote:
>> x86_64 kernel uses 'page_offset_base' variable to point to the
>> start of direct mapping of all physical memory. This variable
>> is also updated for KASLR boot cases, so this can be exported
>> via vmcoreinfo as a standard ABI between kernel and user-space,
>> to allow user-space utilities to use the same for calculating
>> the start of direct mapping of all physical memory.
Why is KERNELOFFSET= not sufficient?
See commit b6085a865762 ("x86, kaslr: export offset in VMCOREINFO ELF notes")
+ vmcoreinfo_append_str("KERNELOFFSET=%lx\n",
+ (unsigned long)&_text - __START_KERNEL);
-Kees
>> diff --git a/arch/x86/kernel/machine_kexec_64.c b/arch/x86/kernel/machine_kexec_64.c
>> index 4c8acdfdc5a7..6161d77c5bfb 100644
>> --- a/arch/x86/kernel/machine_kexec_64.c
>> +++ b/arch/x86/kernel/machine_kexec_64.c
>> @@ -356,6 +356,9 @@ void arch_crash_save_vmcoreinfo(void)
>> VMCOREINFO_SYMBOL(init_top_pgt);
>> vmcoreinfo_append_str("NUMBER(pgtable_l5_enabled)=%d\n",
>> pgtable_l5_enabled());
>> +#ifdef CONFIG_RANDOMIZE_BASE
>> + VMCOREINFO_NUMBER(page_offset_base);
>> +#endif
>>
>> #ifdef CONFIG_NUMA
>> VMCOREINFO_SYMBOL(node_data);
--
Kees Cook
_______________________________________________
kexec mailing list
kexec@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/kexec
^ permalink raw reply [flat|nested] 51+ messages in thread
* [PATCH v2] x86_64, vmcoreinfo: Append 'page_offset_base' to vmcoreinfo
@ 2018-11-27 22:16 ` Kees Cook
0 siblings, 0 replies; 51+ messages in thread
From: Kees Cook @ 2018-11-27 22:16 UTC (permalink / raw)
To: linux-arm-kernel
On Wed, Nov 21, 2018 at 3:39 AM, Borislav Petkov <bp@alien8.de> wrote:
> + Kees.
>
> On Fri, Nov 16, 2018 at 03:17:49AM +0530, Bhupesh Sharma wrote:
>> x86_64 kernel uses 'page_offset_base' variable to point to the
>> start of direct mapping of all physical memory. This variable
>> is also updated for KASLR boot cases, so this can be exported
>> via vmcoreinfo as a standard ABI between kernel and user-space,
>> to allow user-space utilities to use the same for calculating
>> the start of direct mapping of all physical memory.
Why is KERNELOFFSET= not sufficient?
See commit b6085a865762 ("x86, kaslr: export offset in VMCOREINFO ELF notes")
+ vmcoreinfo_append_str("KERNELOFFSET=%lx\n",
+ (unsigned long)&_text - __START_KERNEL);
-Kees
>> diff --git a/arch/x86/kernel/machine_kexec_64.c b/arch/x86/kernel/machine_kexec_64.c
>> index 4c8acdfdc5a7..6161d77c5bfb 100644
>> --- a/arch/x86/kernel/machine_kexec_64.c
>> +++ b/arch/x86/kernel/machine_kexec_64.c
>> @@ -356,6 +356,9 @@ void arch_crash_save_vmcoreinfo(void)
>> VMCOREINFO_SYMBOL(init_top_pgt);
>> vmcoreinfo_append_str("NUMBER(pgtable_l5_enabled)=%d\n",
>> pgtable_l5_enabled());
>> +#ifdef CONFIG_RANDOMIZE_BASE
>> + VMCOREINFO_NUMBER(page_offset_base);
>> +#endif
>>
>> #ifdef CONFIG_NUMA
>> VMCOREINFO_SYMBOL(node_data);
--
Kees Cook
^ permalink raw reply [flat|nested] 51+ messages in thread
* Re: [PATCH v2] x86_64, vmcoreinfo: Append 'page_offset_base' to vmcoreinfo
2018-11-27 22:16 ` Kees Cook
(?)
@ 2018-11-27 23:29 ` Baoquan He
-1 siblings, 0 replies; 51+ messages in thread
From: Baoquan He @ 2018-11-27 23:29 UTC (permalink / raw)
To: Kees Cook
Cc: Borislav Petkov, Kazuhito Hagio, Bhupesh Sharma, X86 ML,
Kexec Mailing List, LKML, Omar Sandoval, Dave Anderson,
James Morse, Thomas Gleixner, Bhupesh SHARMA, Ingo Molnar,
linux-arm-kernel
On 11/27/18 at 02:16pm, Kees Cook wrote:
> Why is KERNELOFFSET= not sufficient?
>
> See commit b6085a865762 ("x86, kaslr: export offset in VMCOREINFO ELF notes")
>
> + vmcoreinfo_append_str("KERNELOFFSET=%lx\n",
> + (unsigned long)&_text - __START_KERNEL);
KERNELOFFSET is virtual address delta after kernel text KASLR, namely
the offset from the original default kernel text virtual address,
0xffffffff88000000.
While after memory region KASLR in kernel_randomize_memory(), the
starting address of the direct mapping of physical memory, PAGE_OFFSET,
is changed too. We need get it to analyze memory in makedumpfile/crash.
Currently we deduce it from elf program segment of kcore:
Program Headers:
Type Offset VirtAddr PhysAddr
FileSiz MemSiz Flags Align
......
LOAD 0x00000a62c0004000 0xffff8a62c0001000 0x0000000000001000
0x000000000009c000 0x000000000009c000 RWE 1000
page_offset = 0xffff8a62c0001000 - 0x0000000000001000;
Since we put the direct mapping segments at the bottom part of kcore, we
can always get page_offset right.
Thanks
Baoquan
>
> -Kees
>
> >> diff --git a/arch/x86/kernel/machine_kexec_64.c b/arch/x86/kernel/machine_kexec_64.c
> >> index 4c8acdfdc5a7..6161d77c5bfb 100644
> >> --- a/arch/x86/kernel/machine_kexec_64.c
> >> +++ b/arch/x86/kernel/machine_kexec_64.c
> >> @@ -356,6 +356,9 @@ void arch_crash_save_vmcoreinfo(void)
> >> VMCOREINFO_SYMBOL(init_top_pgt);
> >> vmcoreinfo_append_str("NUMBER(pgtable_l5_enabled)=%d\n",
> >> pgtable_l5_enabled());
> >> +#ifdef CONFIG_RANDOMIZE_BASE
> >> + VMCOREINFO_NUMBER(page_offset_base);
> >> +#endif
> >>
> >> #ifdef CONFIG_NUMA
> >> VMCOREINFO_SYMBOL(node_data);
>
> --
> Kees Cook
>
> _______________________________________________
> kexec mailing list
> kexec@lists.infradead.org
> http://lists.infradead.org/mailman/listinfo/kexec
^ permalink raw reply [flat|nested] 51+ messages in thread
* Re: [PATCH v2] x86_64, vmcoreinfo: Append 'page_offset_base' to vmcoreinfo
@ 2018-11-27 23:29 ` Baoquan He
0 siblings, 0 replies; 51+ messages in thread
From: Baoquan He @ 2018-11-27 23:29 UTC (permalink / raw)
To: Kees Cook
Cc: Kazuhito Hagio, James Morse, Bhupesh Sharma, X86 ML,
Kexec Mailing List, LKML, Ingo Molnar, Borislav Petkov,
Dave Anderson, Thomas Gleixner, Bhupesh SHARMA, Omar Sandoval,
linux-arm-kernel
On 11/27/18 at 02:16pm, Kees Cook wrote:
> Why is KERNELOFFSET= not sufficient?
>
> See commit b6085a865762 ("x86, kaslr: export offset in VMCOREINFO ELF notes")
>
> + vmcoreinfo_append_str("KERNELOFFSET=%lx\n",
> + (unsigned long)&_text - __START_KERNEL);
KERNELOFFSET is virtual address delta after kernel text KASLR, namely
the offset from the original default kernel text virtual address,
0xffffffff88000000.
While after memory region KASLR in kernel_randomize_memory(), the
starting address of the direct mapping of physical memory, PAGE_OFFSET,
is changed too. We need get it to analyze memory in makedumpfile/crash.
Currently we deduce it from elf program segment of kcore:
Program Headers:
Type Offset VirtAddr PhysAddr
FileSiz MemSiz Flags Align
......
LOAD 0x00000a62c0004000 0xffff8a62c0001000 0x0000000000001000
0x000000000009c000 0x000000000009c000 RWE 1000
page_offset = 0xffff8a62c0001000 - 0x0000000000001000;
Since we put the direct mapping segments at the bottom part of kcore, we
can always get page_offset right.
Thanks
Baoquan
>
> -Kees
>
> >> diff --git a/arch/x86/kernel/machine_kexec_64.c b/arch/x86/kernel/machine_kexec_64.c
> >> index 4c8acdfdc5a7..6161d77c5bfb 100644
> >> --- a/arch/x86/kernel/machine_kexec_64.c
> >> +++ b/arch/x86/kernel/machine_kexec_64.c
> >> @@ -356,6 +356,9 @@ void arch_crash_save_vmcoreinfo(void)
> >> VMCOREINFO_SYMBOL(init_top_pgt);
> >> vmcoreinfo_append_str("NUMBER(pgtable_l5_enabled)=%d\n",
> >> pgtable_l5_enabled());
> >> +#ifdef CONFIG_RANDOMIZE_BASE
> >> + VMCOREINFO_NUMBER(page_offset_base);
> >> +#endif
> >>
> >> #ifdef CONFIG_NUMA
> >> VMCOREINFO_SYMBOL(node_data);
>
> --
> Kees Cook
>
> _______________________________________________
> kexec mailing list
> kexec@lists.infradead.org
> http://lists.infradead.org/mailman/listinfo/kexec
_______________________________________________
kexec mailing list
kexec@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/kexec
^ permalink raw reply [flat|nested] 51+ messages in thread
* [PATCH v2] x86_64, vmcoreinfo: Append 'page_offset_base' to vmcoreinfo
@ 2018-11-27 23:29 ` Baoquan He
0 siblings, 0 replies; 51+ messages in thread
From: Baoquan He @ 2018-11-27 23:29 UTC (permalink / raw)
To: linux-arm-kernel
On 11/27/18 at 02:16pm, Kees Cook wrote:
> Why is KERNELOFFSET= not sufficient?
>
> See commit b6085a865762 ("x86, kaslr: export offset in VMCOREINFO ELF notes")
>
> + vmcoreinfo_append_str("KERNELOFFSET=%lx\n",
> + (unsigned long)&_text - __START_KERNEL);
KERNELOFFSET is virtual address delta after kernel text KASLR, namely
the offset from the original default kernel text virtual address,
0xffffffff88000000.
While after memory region KASLR in kernel_randomize_memory(), the
starting address of the direct mapping of physical memory, PAGE_OFFSET,
is changed too. We need get it to analyze memory in makedumpfile/crash.
Currently we deduce it from elf program segment of kcore:
Program Headers:
Type Offset VirtAddr PhysAddr
FileSiz MemSiz Flags Align
......
LOAD 0x00000a62c0004000 0xffff8a62c0001000 0x0000000000001000
0x000000000009c000 0x000000000009c000 RWE 1000
page_offset = 0xffff8a62c0001000 - 0x0000000000001000;
Since we put the direct mapping segments at the bottom part of kcore, we
can always get page_offset right.
Thanks
Baoquan
>
> -Kees
>
> >> diff --git a/arch/x86/kernel/machine_kexec_64.c b/arch/x86/kernel/machine_kexec_64.c
> >> index 4c8acdfdc5a7..6161d77c5bfb 100644
> >> --- a/arch/x86/kernel/machine_kexec_64.c
> >> +++ b/arch/x86/kernel/machine_kexec_64.c
> >> @@ -356,6 +356,9 @@ void arch_crash_save_vmcoreinfo(void)
> >> VMCOREINFO_SYMBOL(init_top_pgt);
> >> vmcoreinfo_append_str("NUMBER(pgtable_l5_enabled)=%d\n",
> >> pgtable_l5_enabled());
> >> +#ifdef CONFIG_RANDOMIZE_BASE
> >> + VMCOREINFO_NUMBER(page_offset_base);
> >> +#endif
> >>
> >> #ifdef CONFIG_NUMA
> >> VMCOREINFO_SYMBOL(node_data);
>
> --
> Kees Cook
>
> _______________________________________________
> kexec mailing list
> kexec at lists.infradead.org
> http://lists.infradead.org/mailman/listinfo/kexec
^ permalink raw reply [flat|nested] 51+ messages in thread
* Re: [PATCH v2] x86_64, vmcoreinfo: Append 'page_offset_base' to vmcoreinfo
2018-11-27 23:29 ` Baoquan He
(?)
@ 2018-11-28 0:39 ` Kees Cook
-1 siblings, 0 replies; 51+ messages in thread
From: Kees Cook @ 2018-11-28 0:39 UTC (permalink / raw)
To: Baoquan He
Cc: Borislav Petkov, Kazuhito Hagio, Bhupesh Sharma, X86 ML,
Kexec Mailing List, LKML, Omar Sandoval, Dave Anderson,
James Morse, Thomas Gleixner, Bhupesh SHARMA, Ingo Molnar,
linux-arm-kernel
On Tue, Nov 27, 2018 at 3:29 PM, Baoquan He <bhe@redhat.com> wrote:
> On 11/27/18 at 02:16pm, Kees Cook wrote:
>> Why is KERNELOFFSET= not sufficient?
>>
>> See commit b6085a865762 ("x86, kaslr: export offset in VMCOREINFO ELF notes")
>>
>> + vmcoreinfo_append_str("KERNELOFFSET=%lx\n",
>> + (unsigned long)&_text - __START_KERNEL);
>
> KERNELOFFSET is virtual address delta after kernel text KASLR, namely
> the offset from the original default kernel text virtual address,
> 0xffffffff88000000.
>
> While after memory region KASLR in kernel_randomize_memory(), the
> starting address of the direct mapping of physical memory, PAGE_OFFSET,
> is changed too. We need get it to analyze memory in makedumpfile/crash.
> Currently we deduce it from elf program segment of kcore:
> Program Headers:
> Type Offset VirtAddr PhysAddr
> FileSiz MemSiz Flags Align
> ......
>
> LOAD 0x00000a62c0004000 0xffff8a62c0001000 0x0000000000001000
> 0x000000000009c000 0x000000000009c000 RWE 1000
>
> page_offset = 0xffff8a62c0001000 - 0x0000000000001000;
> Since we put the direct mapping segments at the bottom part of kcore, we
> can always get page_offset right.
>
> Thanks
> Baoquan
>
>>
>> -Kees
>>
>> >> diff --git a/arch/x86/kernel/machine_kexec_64.c b/arch/x86/kernel/machine_kexec_64.c
>> >> index 4c8acdfdc5a7..6161d77c5bfb 100644
>> >> --- a/arch/x86/kernel/machine_kexec_64.c
>> >> +++ b/arch/x86/kernel/machine_kexec_64.c
>> >> @@ -356,6 +356,9 @@ void arch_crash_save_vmcoreinfo(void)
>> >> VMCOREINFO_SYMBOL(init_top_pgt);
>> >> vmcoreinfo_append_str("NUMBER(pgtable_l5_enabled)=%d\n",
>> >> pgtable_l5_enabled());
>> >> +#ifdef CONFIG_RANDOMIZE_BASE
Okay, gotcha. In that case, shouldn't this be CONFIG_RANDOMIZE_MEMORY?
-Kees
>> >> + VMCOREINFO_NUMBER(page_offset_base);
>> >> +#endif
>> >>
>> >> #ifdef CONFIG_NUMA
>> >> VMCOREINFO_SYMBOL(node_data);
>>
>> --
>> Kees Cook
>>
>> _______________________________________________
>> kexec mailing list
>> kexec@lists.infradead.org
>> http://lists.infradead.org/mailman/listinfo/kexec
--
Kees Cook
^ permalink raw reply [flat|nested] 51+ messages in thread
* Re: [PATCH v2] x86_64, vmcoreinfo: Append 'page_offset_base' to vmcoreinfo
@ 2018-11-28 0:39 ` Kees Cook
0 siblings, 0 replies; 51+ messages in thread
From: Kees Cook @ 2018-11-28 0:39 UTC (permalink / raw)
To: Baoquan He
Cc: Kazuhito Hagio, James Morse, Bhupesh Sharma, X86 ML,
Kexec Mailing List, LKML, Ingo Molnar, Borislav Petkov,
Dave Anderson, Thomas Gleixner, Bhupesh SHARMA, Omar Sandoval,
linux-arm-kernel
On Tue, Nov 27, 2018 at 3:29 PM, Baoquan He <bhe@redhat.com> wrote:
> On 11/27/18 at 02:16pm, Kees Cook wrote:
>> Why is KERNELOFFSET= not sufficient?
>>
>> See commit b6085a865762 ("x86, kaslr: export offset in VMCOREINFO ELF notes")
>>
>> + vmcoreinfo_append_str("KERNELOFFSET=%lx\n",
>> + (unsigned long)&_text - __START_KERNEL);
>
> KERNELOFFSET is virtual address delta after kernel text KASLR, namely
> the offset from the original default kernel text virtual address,
> 0xffffffff88000000.
>
> While after memory region KASLR in kernel_randomize_memory(), the
> starting address of the direct mapping of physical memory, PAGE_OFFSET,
> is changed too. We need get it to analyze memory in makedumpfile/crash.
> Currently we deduce it from elf program segment of kcore:
> Program Headers:
> Type Offset VirtAddr PhysAddr
> FileSiz MemSiz Flags Align
> ......
>
> LOAD 0x00000a62c0004000 0xffff8a62c0001000 0x0000000000001000
> 0x000000000009c000 0x000000000009c000 RWE 1000
>
> page_offset = 0xffff8a62c0001000 - 0x0000000000001000;
> Since we put the direct mapping segments at the bottom part of kcore, we
> can always get page_offset right.
>
> Thanks
> Baoquan
>
>>
>> -Kees
>>
>> >> diff --git a/arch/x86/kernel/machine_kexec_64.c b/arch/x86/kernel/machine_kexec_64.c
>> >> index 4c8acdfdc5a7..6161d77c5bfb 100644
>> >> --- a/arch/x86/kernel/machine_kexec_64.c
>> >> +++ b/arch/x86/kernel/machine_kexec_64.c
>> >> @@ -356,6 +356,9 @@ void arch_crash_save_vmcoreinfo(void)
>> >> VMCOREINFO_SYMBOL(init_top_pgt);
>> >> vmcoreinfo_append_str("NUMBER(pgtable_l5_enabled)=%d\n",
>> >> pgtable_l5_enabled());
>> >> +#ifdef CONFIG_RANDOMIZE_BASE
Okay, gotcha. In that case, shouldn't this be CONFIG_RANDOMIZE_MEMORY?
-Kees
>> >> + VMCOREINFO_NUMBER(page_offset_base);
>> >> +#endif
>> >>
>> >> #ifdef CONFIG_NUMA
>> >> VMCOREINFO_SYMBOL(node_data);
>>
>> --
>> Kees Cook
>>
>> _______________________________________________
>> kexec mailing list
>> kexec@lists.infradead.org
>> http://lists.infradead.org/mailman/listinfo/kexec
--
Kees Cook
_______________________________________________
kexec mailing list
kexec@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/kexec
^ permalink raw reply [flat|nested] 51+ messages in thread
* [PATCH v2] x86_64, vmcoreinfo: Append 'page_offset_base' to vmcoreinfo
@ 2018-11-28 0:39 ` Kees Cook
0 siblings, 0 replies; 51+ messages in thread
From: Kees Cook @ 2018-11-28 0:39 UTC (permalink / raw)
To: linux-arm-kernel
On Tue, Nov 27, 2018 at 3:29 PM, Baoquan He <bhe@redhat.com> wrote:
> On 11/27/18 at 02:16pm, Kees Cook wrote:
>> Why is KERNELOFFSET= not sufficient?
>>
>> See commit b6085a865762 ("x86, kaslr: export offset in VMCOREINFO ELF notes")
>>
>> + vmcoreinfo_append_str("KERNELOFFSET=%lx\n",
>> + (unsigned long)&_text - __START_KERNEL);
>
> KERNELOFFSET is virtual address delta after kernel text KASLR, namely
> the offset from the original default kernel text virtual address,
> 0xffffffff88000000.
>
> While after memory region KASLR in kernel_randomize_memory(), the
> starting address of the direct mapping of physical memory, PAGE_OFFSET,
> is changed too. We need get it to analyze memory in makedumpfile/crash.
> Currently we deduce it from elf program segment of kcore:
> Program Headers:
> Type Offset VirtAddr PhysAddr
> FileSiz MemSiz Flags Align
> ......
>
> LOAD 0x00000a62c0004000 0xffff8a62c0001000 0x0000000000001000
> 0x000000000009c000 0x000000000009c000 RWE 1000
>
> page_offset = 0xffff8a62c0001000 - 0x0000000000001000;
> Since we put the direct mapping segments at the bottom part of kcore, we
> can always get page_offset right.
>
> Thanks
> Baoquan
>
>>
>> -Kees
>>
>> >> diff --git a/arch/x86/kernel/machine_kexec_64.c b/arch/x86/kernel/machine_kexec_64.c
>> >> index 4c8acdfdc5a7..6161d77c5bfb 100644
>> >> --- a/arch/x86/kernel/machine_kexec_64.c
>> >> +++ b/arch/x86/kernel/machine_kexec_64.c
>> >> @@ -356,6 +356,9 @@ void arch_crash_save_vmcoreinfo(void)
>> >> VMCOREINFO_SYMBOL(init_top_pgt);
>> >> vmcoreinfo_append_str("NUMBER(pgtable_l5_enabled)=%d\n",
>> >> pgtable_l5_enabled());
>> >> +#ifdef CONFIG_RANDOMIZE_BASE
Okay, gotcha. In that case, shouldn't this be CONFIG_RANDOMIZE_MEMORY?
-Kees
>> >> + VMCOREINFO_NUMBER(page_offset_base);
>> >> +#endif
>> >>
>> >> #ifdef CONFIG_NUMA
>> >> VMCOREINFO_SYMBOL(node_data);
>>
>> --
>> Kees Cook
>>
>> _______________________________________________
>> kexec mailing list
>> kexec at lists.infradead.org
>> http://lists.infradead.org/mailman/listinfo/kexec
--
Kees Cook
^ permalink raw reply [flat|nested] 51+ messages in thread
* Re: [PATCH v2] x86_64, vmcoreinfo: Append 'page_offset_base' to vmcoreinfo
2018-11-28 0:39 ` Kees Cook
(?)
@ 2018-11-28 1:39 ` Baoquan He
-1 siblings, 0 replies; 51+ messages in thread
From: Baoquan He @ 2018-11-28 1:39 UTC (permalink / raw)
To: Kees Cook
Cc: Borislav Petkov, Kazuhito Hagio, Bhupesh Sharma, X86 ML,
Kexec Mailing List, LKML, Omar Sandoval, Dave Anderson,
James Morse, Thomas Gleixner, Bhupesh SHARMA, Ingo Molnar,
linux-arm-kernel
On 11/27/18 at 04:39pm, Kees Cook wrote:
> >> >> diff --git a/arch/x86/kernel/machine_kexec_64.c b/arch/x86/kernel/machine_kexec_64.c
> >> >> index 4c8acdfdc5a7..6161d77c5bfb 100644
> >> >> --- a/arch/x86/kernel/machine_kexec_64.c
> >> >> +++ b/arch/x86/kernel/machine_kexec_64.c
> >> >> @@ -356,6 +356,9 @@ void arch_crash_save_vmcoreinfo(void)
> >> >> VMCOREINFO_SYMBOL(init_top_pgt);
> >> >> vmcoreinfo_append_str("NUMBER(pgtable_l5_enabled)=%d\n",
> >> >> pgtable_l5_enabled());
> >> >> +#ifdef CONFIG_RANDOMIZE_BASE
>
> Okay, gotcha. In that case, shouldn't this be CONFIG_RANDOMIZE_MEMORY?
Currently, Kirill added level5 support to x86_64, and kernel with level5
enabled can be boot switched into level4 or level5 with kernel option
"no5lvl". So page_offset_base will be changed accordingly. You can see
code pasted at bottom, DYNAMIC_MEMORY_LAYOUT is added for this change,
not only KASLR, but 5LEVEL.
If only put it inside ifdef CONFIG_RANDOMIZE_MEMORY, change between l4
and l5 will force us to decide page_offset again if
CONFIG_RANDOMIZE_MEMORY or CONFIG_RANDOMIZE_BASE is not set. Besides,
below commit change the starting address of the direct mapping again, if
only judge CONFIG_RANDOMIZE_MEMORY, in case KASLR is disabled, code in
userspace may need many if-else checking as below. So if we pass, better
pass it for all.
get_page_offset()
{
if(get_page_offset_from_vmcoreinfo()) {
xxx //in KASLR case
return;
} else if (check_5level_paging()) {
if (version < 4.21) {
page_offset = 0xff10000000000000;
} else //version > = 4.21
{
page_offset = 0xff11000000000000;
}
} else { //4level
if (version < 2.6.27) {
page_offset = 0xffff810000000000;
} else if (version < 4.21) {
page_offset = 0xffff880000000000;
} else //version > = 4.21
{
page_offset = 0xffff888000000000,;
}
}
}
Sign, seeing above code, I still think that deducing it from
kcore/vmcore elf header is better. Can't we be better to ourselves?
commit d52888aa2753e3063a9d3a0c9f72f94aa9809c15
Author: Kirill A. Shutemov <kirill.shutemov@linux.intel.com>
Date: Fri Oct 26 15:28:54 2018 +0300
x86/mm: Move LDT remap out of KASLR region on 5-level paging
[bhe@ linux]$ git describe --contains d52888aa2753e3063a9d3a0c9f72f94aa9809c15
v4.20-rc2~5^2~4
~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~
#ifdef CONFIG_DYNAMIC_MEMORY_LAYOUT
unsigned long page_offset_base __ro_after_init = __PAGE_OFFSET_BASE_L4;
EXPORT_SYMBOL(page_offset_base);
unsigned long vmalloc_base __ro_after_init = __VMALLOC_BASE_L4;
EXPORT_SYMBOL(vmalloc_base);
unsigned long vmemmap_base __ro_after_init = __VMEMMAP_BASE_L4;
EXPORT_SYMBOL(vmemmap_base);
#endif
config DYNAMIC_MEMORY_LAYOUT
bool
---help---
This option makes base addresses of vmalloc and vmemmap as well as
__PAGE_OFFSET movable during boot.
config RANDOMIZE_MEMORY
bool "Randomize the kernel memory sections"
depends on X86_64
depends on RANDOMIZE_BASE
select DYNAMIC_MEMORY_LAYOUT
default RANDOMIZE_BASE
config X86_5LEVEL
bool "Enable 5-level page tables support"
select DYNAMIC_MEMORY_LAYOUT
select SPARSEMEM_VMEMMAP
depends on X86_64
^ permalink raw reply [flat|nested] 51+ messages in thread
* Re: [PATCH v2] x86_64, vmcoreinfo: Append 'page_offset_base' to vmcoreinfo
@ 2018-11-28 1:39 ` Baoquan He
0 siblings, 0 replies; 51+ messages in thread
From: Baoquan He @ 2018-11-28 1:39 UTC (permalink / raw)
To: Kees Cook
Cc: Kazuhito Hagio, James Morse, Bhupesh Sharma, X86 ML,
Kexec Mailing List, LKML, Ingo Molnar, Borislav Petkov,
Dave Anderson, Thomas Gleixner, Bhupesh SHARMA, Omar Sandoval,
linux-arm-kernel
On 11/27/18 at 04:39pm, Kees Cook wrote:
> >> >> diff --git a/arch/x86/kernel/machine_kexec_64.c b/arch/x86/kernel/machine_kexec_64.c
> >> >> index 4c8acdfdc5a7..6161d77c5bfb 100644
> >> >> --- a/arch/x86/kernel/machine_kexec_64.c
> >> >> +++ b/arch/x86/kernel/machine_kexec_64.c
> >> >> @@ -356,6 +356,9 @@ void arch_crash_save_vmcoreinfo(void)
> >> >> VMCOREINFO_SYMBOL(init_top_pgt);
> >> >> vmcoreinfo_append_str("NUMBER(pgtable_l5_enabled)=%d\n",
> >> >> pgtable_l5_enabled());
> >> >> +#ifdef CONFIG_RANDOMIZE_BASE
>
> Okay, gotcha. In that case, shouldn't this be CONFIG_RANDOMIZE_MEMORY?
Currently, Kirill added level5 support to x86_64, and kernel with level5
enabled can be boot switched into level4 or level5 with kernel option
"no5lvl". So page_offset_base will be changed accordingly. You can see
code pasted at bottom, DYNAMIC_MEMORY_LAYOUT is added for this change,
not only KASLR, but 5LEVEL.
If only put it inside ifdef CONFIG_RANDOMIZE_MEMORY, change between l4
and l5 will force us to decide page_offset again if
CONFIG_RANDOMIZE_MEMORY or CONFIG_RANDOMIZE_BASE is not set. Besides,
below commit change the starting address of the direct mapping again, if
only judge CONFIG_RANDOMIZE_MEMORY, in case KASLR is disabled, code in
userspace may need many if-else checking as below. So if we pass, better
pass it for all.
get_page_offset()
{
if(get_page_offset_from_vmcoreinfo()) {
xxx //in KASLR case
return;
} else if (check_5level_paging()) {
if (version < 4.21) {
page_offset = 0xff10000000000000;
} else //version > = 4.21
{
page_offset = 0xff11000000000000;
}
} else { //4level
if (version < 2.6.27) {
page_offset = 0xffff810000000000;
} else if (version < 4.21) {
page_offset = 0xffff880000000000;
} else //version > = 4.21
{
page_offset = 0xffff888000000000,;
}
}
}
Sign, seeing above code, I still think that deducing it from
kcore/vmcore elf header is better. Can't we be better to ourselves?
commit d52888aa2753e3063a9d3a0c9f72f94aa9809c15
Author: Kirill A. Shutemov <kirill.shutemov@linux.intel.com>
Date: Fri Oct 26 15:28:54 2018 +0300
x86/mm: Move LDT remap out of KASLR region on 5-level paging
[bhe@ linux]$ git describe --contains d52888aa2753e3063a9d3a0c9f72f94aa9809c15
v4.20-rc2~5^2~4
~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~
#ifdef CONFIG_DYNAMIC_MEMORY_LAYOUT
unsigned long page_offset_base __ro_after_init = __PAGE_OFFSET_BASE_L4;
EXPORT_SYMBOL(page_offset_base);
unsigned long vmalloc_base __ro_after_init = __VMALLOC_BASE_L4;
EXPORT_SYMBOL(vmalloc_base);
unsigned long vmemmap_base __ro_after_init = __VMEMMAP_BASE_L4;
EXPORT_SYMBOL(vmemmap_base);
#endif
config DYNAMIC_MEMORY_LAYOUT
bool
---help---
This option makes base addresses of vmalloc and vmemmap as well as
__PAGE_OFFSET movable during boot.
config RANDOMIZE_MEMORY
bool "Randomize the kernel memory sections"
depends on X86_64
depends on RANDOMIZE_BASE
select DYNAMIC_MEMORY_LAYOUT
default RANDOMIZE_BASE
config X86_5LEVEL
bool "Enable 5-level page tables support"
select DYNAMIC_MEMORY_LAYOUT
select SPARSEMEM_VMEMMAP
depends on X86_64
_______________________________________________
kexec mailing list
kexec@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/kexec
^ permalink raw reply [flat|nested] 51+ messages in thread
* [PATCH v2] x86_64, vmcoreinfo: Append 'page_offset_base' to vmcoreinfo
@ 2018-11-28 1:39 ` Baoquan He
0 siblings, 0 replies; 51+ messages in thread
From: Baoquan He @ 2018-11-28 1:39 UTC (permalink / raw)
To: linux-arm-kernel
On 11/27/18 at 04:39pm, Kees Cook wrote:
> >> >> diff --git a/arch/x86/kernel/machine_kexec_64.c b/arch/x86/kernel/machine_kexec_64.c
> >> >> index 4c8acdfdc5a7..6161d77c5bfb 100644
> >> >> --- a/arch/x86/kernel/machine_kexec_64.c
> >> >> +++ b/arch/x86/kernel/machine_kexec_64.c
> >> >> @@ -356,6 +356,9 @@ void arch_crash_save_vmcoreinfo(void)
> >> >> VMCOREINFO_SYMBOL(init_top_pgt);
> >> >> vmcoreinfo_append_str("NUMBER(pgtable_l5_enabled)=%d\n",
> >> >> pgtable_l5_enabled());
> >> >> +#ifdef CONFIG_RANDOMIZE_BASE
>
> Okay, gotcha. In that case, shouldn't this be CONFIG_RANDOMIZE_MEMORY?
Currently, Kirill added level5 support to x86_64, and kernel with level5
enabled can be boot switched into level4 or level5 with kernel option
"no5lvl". So page_offset_base will be changed accordingly. You can see
code pasted at bottom, DYNAMIC_MEMORY_LAYOUT is added for this change,
not only KASLR, but 5LEVEL.
If only put it inside ifdef CONFIG_RANDOMIZE_MEMORY, change between l4
and l5 will force us to decide page_offset again if
CONFIG_RANDOMIZE_MEMORY or CONFIG_RANDOMIZE_BASE is not set. Besides,
below commit change the starting address of the direct mapping again, if
only judge CONFIG_RANDOMIZE_MEMORY, in case KASLR is disabled, code in
userspace may need many if-else checking as below. So if we pass, better
pass it for all.
get_page_offset()
{
if(get_page_offset_from_vmcoreinfo()) {
xxx //in KASLR case
return;
} else if (check_5level_paging()) {
if (version < 4.21) {
page_offset = 0xff10000000000000;
} else //version > = 4.21
{
page_offset = 0xff11000000000000;
}
} else { //4level
if (version < 2.6.27) {
page_offset = 0xffff810000000000;
} else if (version < 4.21) {
page_offset = 0xffff880000000000;
} else //version > = 4.21
{
page_offset = 0xffff888000000000,;
}
}
}
Sign, seeing above code, I still think that deducing it from
kcore/vmcore elf header is better. Can't we be better to ourselves?
commit d52888aa2753e3063a9d3a0c9f72f94aa9809c15
Author: Kirill A. Shutemov <kirill.shutemov@linux.intel.com>
Date: Fri Oct 26 15:28:54 2018 +0300
x86/mm: Move LDT remap out of KASLR region on 5-level paging
[bhe@ linux]$ git describe --contains d52888aa2753e3063a9d3a0c9f72f94aa9809c15
v4.20-rc2~5^2~4
~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~
#ifdef CONFIG_DYNAMIC_MEMORY_LAYOUT
unsigned long page_offset_base __ro_after_init = __PAGE_OFFSET_BASE_L4;
EXPORT_SYMBOL(page_offset_base);
unsigned long vmalloc_base __ro_after_init = __VMALLOC_BASE_L4;
EXPORT_SYMBOL(vmalloc_base);
unsigned long vmemmap_base __ro_after_init = __VMEMMAP_BASE_L4;
EXPORT_SYMBOL(vmemmap_base);
#endif
config DYNAMIC_MEMORY_LAYOUT
bool
---help---
This option makes base addresses of vmalloc and vmemmap as well as
__PAGE_OFFSET movable during boot.
config RANDOMIZE_MEMORY
bool "Randomize the kernel memory sections"
depends on X86_64
depends on RANDOMIZE_BASE
select DYNAMIC_MEMORY_LAYOUT
default RANDOMIZE_BASE
config X86_5LEVEL
bool "Enable 5-level page tables support"
select DYNAMIC_MEMORY_LAYOUT
select SPARSEMEM_VMEMMAP
depends on X86_64
^ permalink raw reply [flat|nested] 51+ messages in thread
* Re: [PATCH v2] x86_64, vmcoreinfo: Append 'page_offset_base' to vmcoreinfo
2018-11-28 0:39 ` Kees Cook
(?)
@ 2018-11-28 1:57 ` Baoquan He
-1 siblings, 0 replies; 51+ messages in thread
From: Baoquan He @ 2018-11-28 1:57 UTC (permalink / raw)
To: Kees Cook
Cc: Borislav Petkov, Kazuhito Hagio, Bhupesh Sharma, X86 ML,
Kexec Mailing List, LKML, Omar Sandoval, Dave Anderson,
James Morse, Thomas Gleixner, Bhupesh SHARMA, Ingo Molnar,
linux-arm-kernel
On 11/27/18 at 04:39pm, Kees Cook wrote:
> >> >> diff --git a/arch/x86/kernel/machine_kexec_64.c b/arch/x86/kernel/machine_kexec_64.c
> >> >> index 4c8acdfdc5a7..6161d77c5bfb 100644
> >> >> --- a/arch/x86/kernel/machine_kexec_64.c
> >> >> +++ b/arch/x86/kernel/machine_kexec_64.c
> >> >> @@ -356,6 +356,9 @@ void arch_crash_save_vmcoreinfo(void)
> >> >> VMCOREINFO_SYMBOL(init_top_pgt);
> >> >> vmcoreinfo_append_str("NUMBER(pgtable_l5_enabled)=%d\n",
> >> >> pgtable_l5_enabled());
> >> >> +#ifdef CONFIG_RANDOMIZE_BASE
>
> Okay, gotcha. In that case, shouldn't this be CONFIG_RANDOMIZE_MEMORY?
And yes, if we only care about KASLR, it should be
CONFIG_RANDOMIZE_MEMORY, but not CONFIG_RANDOMIZE_BASE.
^ permalink raw reply [flat|nested] 51+ messages in thread
* Re: [PATCH v2] x86_64, vmcoreinfo: Append 'page_offset_base' to vmcoreinfo
@ 2018-11-28 1:57 ` Baoquan He
0 siblings, 0 replies; 51+ messages in thread
From: Baoquan He @ 2018-11-28 1:57 UTC (permalink / raw)
To: Kees Cook
Cc: Kazuhito Hagio, James Morse, Bhupesh Sharma, X86 ML,
Kexec Mailing List, LKML, Ingo Molnar, Borislav Petkov,
Dave Anderson, Thomas Gleixner, Bhupesh SHARMA, Omar Sandoval,
linux-arm-kernel
On 11/27/18 at 04:39pm, Kees Cook wrote:
> >> >> diff --git a/arch/x86/kernel/machine_kexec_64.c b/arch/x86/kernel/machine_kexec_64.c
> >> >> index 4c8acdfdc5a7..6161d77c5bfb 100644
> >> >> --- a/arch/x86/kernel/machine_kexec_64.c
> >> >> +++ b/arch/x86/kernel/machine_kexec_64.c
> >> >> @@ -356,6 +356,9 @@ void arch_crash_save_vmcoreinfo(void)
> >> >> VMCOREINFO_SYMBOL(init_top_pgt);
> >> >> vmcoreinfo_append_str("NUMBER(pgtable_l5_enabled)=%d\n",
> >> >> pgtable_l5_enabled());
> >> >> +#ifdef CONFIG_RANDOMIZE_BASE
>
> Okay, gotcha. In that case, shouldn't this be CONFIG_RANDOMIZE_MEMORY?
And yes, if we only care about KASLR, it should be
CONFIG_RANDOMIZE_MEMORY, but not CONFIG_RANDOMIZE_BASE.
_______________________________________________
kexec mailing list
kexec@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/kexec
^ permalink raw reply [flat|nested] 51+ messages in thread
* [PATCH v2] x86_64, vmcoreinfo: Append 'page_offset_base' to vmcoreinfo
@ 2018-11-28 1:57 ` Baoquan He
0 siblings, 0 replies; 51+ messages in thread
From: Baoquan He @ 2018-11-28 1:57 UTC (permalink / raw)
To: linux-arm-kernel
On 11/27/18 at 04:39pm, Kees Cook wrote:
> >> >> diff --git a/arch/x86/kernel/machine_kexec_64.c b/arch/x86/kernel/machine_kexec_64.c
> >> >> index 4c8acdfdc5a7..6161d77c5bfb 100644
> >> >> --- a/arch/x86/kernel/machine_kexec_64.c
> >> >> +++ b/arch/x86/kernel/machine_kexec_64.c
> >> >> @@ -356,6 +356,9 @@ void arch_crash_save_vmcoreinfo(void)
> >> >> VMCOREINFO_SYMBOL(init_top_pgt);
> >> >> vmcoreinfo_append_str("NUMBER(pgtable_l5_enabled)=%d\n",
> >> >> pgtable_l5_enabled());
> >> >> +#ifdef CONFIG_RANDOMIZE_BASE
>
> Okay, gotcha. In that case, shouldn't this be CONFIG_RANDOMIZE_MEMORY?
And yes, if we only care about KASLR, it should be
CONFIG_RANDOMIZE_MEMORY, but not CONFIG_RANDOMIZE_BASE.
^ permalink raw reply [flat|nested] 51+ messages in thread
* Re: [PATCH v2] x86_64, vmcoreinfo: Append 'page_offset_base' to vmcoreinfo
2018-11-28 1:57 ` Baoquan He
(?)
@ 2018-11-28 4:26 ` Bhupesh Sharma
-1 siblings, 0 replies; 51+ messages in thread
From: Bhupesh Sharma @ 2018-11-28 4:26 UTC (permalink / raw)
To: Baoquan He
Cc: Kees Cook, Borislav Petkov, Kazuhito Hagio, x86,
kexec mailing list, Linux Kernel Mailing List, Omar Sandoval,
Dave Anderson, James Morse, Thomas Gleixner, Bhupesh SHARMA,
Ingo Molnar, linux-arm-kernel
On Wed, Nov 28, 2018 at 7:27 AM Baoquan He <bhe@redhat.com> wrote:
>
> On 11/27/18 at 04:39pm, Kees Cook wrote:
> > >> >> diff --git a/arch/x86/kernel/machine_kexec_64.c b/arch/x86/kernel/machine_kexec_64.c
> > >> >> index 4c8acdfdc5a7..6161d77c5bfb 100644
> > >> >> --- a/arch/x86/kernel/machine_kexec_64.c
> > >> >> +++ b/arch/x86/kernel/machine_kexec_64.c
> > >> >> @@ -356,6 +356,9 @@ void arch_crash_save_vmcoreinfo(void)
> > >> >> VMCOREINFO_SYMBOL(init_top_pgt);
> > >> >> vmcoreinfo_append_str("NUMBER(pgtable_l5_enabled)=%d\n",
> > >> >> pgtable_l5_enabled());
> > >> >> +#ifdef CONFIG_RANDOMIZE_BASE
> >
> > Okay, gotcha. In that case, shouldn't this be CONFIG_RANDOMIZE_MEMORY?
>
> And yes, if we only care about KASLR, it should be
> CONFIG_RANDOMIZE_MEMORY, but not CONFIG_RANDOMIZE_BASE.
>
Have you tried building the changes with 'make allmodconfig' with all
the different CONFIG options you have suggested so far?
^ permalink raw reply [flat|nested] 51+ messages in thread
* Re: [PATCH v2] x86_64, vmcoreinfo: Append 'page_offset_base' to vmcoreinfo
@ 2018-11-28 4:26 ` Bhupesh Sharma
0 siblings, 0 replies; 51+ messages in thread
From: Bhupesh Sharma @ 2018-11-28 4:26 UTC (permalink / raw)
To: Baoquan He
Cc: Kazuhito Hagio, Kees Cook, x86, kexec mailing list,
Linux Kernel Mailing List, Ingo Molnar, Borislav Petkov,
James Morse, Dave Anderson, Thomas Gleixner, Bhupesh SHARMA,
Omar Sandoval, linux-arm-kernel
On Wed, Nov 28, 2018 at 7:27 AM Baoquan He <bhe@redhat.com> wrote:
>
> On 11/27/18 at 04:39pm, Kees Cook wrote:
> > >> >> diff --git a/arch/x86/kernel/machine_kexec_64.c b/arch/x86/kernel/machine_kexec_64.c
> > >> >> index 4c8acdfdc5a7..6161d77c5bfb 100644
> > >> >> --- a/arch/x86/kernel/machine_kexec_64.c
> > >> >> +++ b/arch/x86/kernel/machine_kexec_64.c
> > >> >> @@ -356,6 +356,9 @@ void arch_crash_save_vmcoreinfo(void)
> > >> >> VMCOREINFO_SYMBOL(init_top_pgt);
> > >> >> vmcoreinfo_append_str("NUMBER(pgtable_l5_enabled)=%d\n",
> > >> >> pgtable_l5_enabled());
> > >> >> +#ifdef CONFIG_RANDOMIZE_BASE
> >
> > Okay, gotcha. In that case, shouldn't this be CONFIG_RANDOMIZE_MEMORY?
>
> And yes, if we only care about KASLR, it should be
> CONFIG_RANDOMIZE_MEMORY, but not CONFIG_RANDOMIZE_BASE.
>
Have you tried building the changes with 'make allmodconfig' with all
the different CONFIG options you have suggested so far?
_______________________________________________
kexec mailing list
kexec@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/kexec
^ permalink raw reply [flat|nested] 51+ messages in thread
* [PATCH v2] x86_64, vmcoreinfo: Append 'page_offset_base' to vmcoreinfo
@ 2018-11-28 4:26 ` Bhupesh Sharma
0 siblings, 0 replies; 51+ messages in thread
From: Bhupesh Sharma @ 2018-11-28 4:26 UTC (permalink / raw)
To: linux-arm-kernel
On Wed, Nov 28, 2018 at 7:27 AM Baoquan He <bhe@redhat.com> wrote:
>
> On 11/27/18 at 04:39pm, Kees Cook wrote:
> > >> >> diff --git a/arch/x86/kernel/machine_kexec_64.c b/arch/x86/kernel/machine_kexec_64.c
> > >> >> index 4c8acdfdc5a7..6161d77c5bfb 100644
> > >> >> --- a/arch/x86/kernel/machine_kexec_64.c
> > >> >> +++ b/arch/x86/kernel/machine_kexec_64.c
> > >> >> @@ -356,6 +356,9 @@ void arch_crash_save_vmcoreinfo(void)
> > >> >> VMCOREINFO_SYMBOL(init_top_pgt);
> > >> >> vmcoreinfo_append_str("NUMBER(pgtable_l5_enabled)=%d\n",
> > >> >> pgtable_l5_enabled());
> > >> >> +#ifdef CONFIG_RANDOMIZE_BASE
> >
> > Okay, gotcha. In that case, shouldn't this be CONFIG_RANDOMIZE_MEMORY?
>
> And yes, if we only care about KASLR, it should be
> CONFIG_RANDOMIZE_MEMORY, but not CONFIG_RANDOMIZE_BASE.
>
Have you tried building the changes with 'make allmodconfig' with all
the different CONFIG options you have suggested so far?
^ permalink raw reply [flat|nested] 51+ messages in thread
* Re: [PATCH v2] x86_64, vmcoreinfo: Append 'page_offset_base' to vmcoreinfo
2018-11-21 11:39 ` Borislav Petkov
(?)
@ 2018-11-28 11:38 ` Dave Young
-1 siblings, 0 replies; 51+ messages in thread
From: Dave Young @ 2018-11-28 11:38 UTC (permalink / raw)
To: Borislav Petkov
Cc: Bhupesh Sharma, Kazuhito Hagio, Kees Cook, Baoquan He, x86,
kexec, linux-kernel, Omar Sandoval, Dave Anderson, James Morse,
Thomas Gleixner, bhupesh.linux, Ingo Molnar, linux-arm-kernel
> > now possible to use the vmcoreinfo present inside kcore as the standard
> > ABI which can be used by the user-space utilities for reading
> > the machine specific information (and hence for debugging a
> > live kernel).
> >
> > User-space utilities like makedumpfile, kexec-tools and crash
> > are either already using this ABI or are discussing patches
> > which look to add the same feature. This helps in simplifying the
> > overall code and also in reducing code-rewrite across the
> > user-space utilities for getting values of these kernel
> > symbols/variables.
>
> > Accordingly this patch allows appending 'page_offset_base' for
> > x86_64 platforms to vmcoreinfo, so that user-space tools can use the
> > same as a standard interface to determine the start of direct mapping
> > of all physical memory.
> >
[snip]
> All above are only nitpicks though.
>
> My opinion is this: people are exporting all kinds of kernel-internal
> stuff in vmcoreinfo and frankly, I'm not crazy about this idea.
>
> And AFAICT, this thing basically bypasses KASLR completely but you need
> root for it so it probably doesn't really matter.
>
> Now, on another thread we agreed more or less that what gets exported in
> vmcoreinfo is so tightly coupled to the running kernel so that it is not
> even considered an ABI. I guess that is debatable but whatever.
We do not regard this strictly as an ABI, but we also carefully review
every new extra exported thing and only export when we have to do so eg.
something breaks.
Seems this change only make userspace tools handling on the kaslr case
easier but since everything works without this patch I would prefer not to
do it.
>
> So my only request right now would be to have all those things being
> exported, documented somewhere and I believe Lianbo is working on that.
>
> But I'm sure others will have more to say about it.
>
> --
> Regards/Gruss,
> Boris.
>
> Good mailing practices for 400: avoid top-posting and trim the reply.
>
> _______________________________________________
> kexec mailing list
> kexec@lists.infradead.org
> http://lists.infradead.org/mailman/listinfo/kexec
Thanks
Dave
^ permalink raw reply [flat|nested] 51+ messages in thread
* Re: [PATCH v2] x86_64, vmcoreinfo: Append 'page_offset_base' to vmcoreinfo
@ 2018-11-28 11:38 ` Dave Young
0 siblings, 0 replies; 51+ messages in thread
From: Dave Young @ 2018-11-28 11:38 UTC (permalink / raw)
To: Borislav Petkov
Cc: Kazuhito Hagio, Kees Cook, Baoquan He, Bhupesh Sharma, x86,
kexec, linux-kernel, Ingo Molnar, Dave Anderson, James Morse,
Thomas Gleixner, bhupesh.linux, Omar Sandoval, linux-arm-kernel
> > now possible to use the vmcoreinfo present inside kcore as the standard
> > ABI which can be used by the user-space utilities for reading
> > the machine specific information (and hence for debugging a
> > live kernel).
> >
> > User-space utilities like makedumpfile, kexec-tools and crash
> > are either already using this ABI or are discussing patches
> > which look to add the same feature. This helps in simplifying the
> > overall code and also in reducing code-rewrite across the
> > user-space utilities for getting values of these kernel
> > symbols/variables.
>
> > Accordingly this patch allows appending 'page_offset_base' for
> > x86_64 platforms to vmcoreinfo, so that user-space tools can use the
> > same as a standard interface to determine the start of direct mapping
> > of all physical memory.
> >
[snip]
> All above are only nitpicks though.
>
> My opinion is this: people are exporting all kinds of kernel-internal
> stuff in vmcoreinfo and frankly, I'm not crazy about this idea.
>
> And AFAICT, this thing basically bypasses KASLR completely but you need
> root for it so it probably doesn't really matter.
>
> Now, on another thread we agreed more or less that what gets exported in
> vmcoreinfo is so tightly coupled to the running kernel so that it is not
> even considered an ABI. I guess that is debatable but whatever.
We do not regard this strictly as an ABI, but we also carefully review
every new extra exported thing and only export when we have to do so eg.
something breaks.
Seems this change only make userspace tools handling on the kaslr case
easier but since everything works without this patch I would prefer not to
do it.
>
> So my only request right now would be to have all those things being
> exported, documented somewhere and I believe Lianbo is working on that.
>
> But I'm sure others will have more to say about it.
>
> --
> Regards/Gruss,
> Boris.
>
> Good mailing practices for 400: avoid top-posting and trim the reply.
>
> _______________________________________________
> kexec mailing list
> kexec@lists.infradead.org
> http://lists.infradead.org/mailman/listinfo/kexec
Thanks
Dave
_______________________________________________
kexec mailing list
kexec@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/kexec
^ permalink raw reply [flat|nested] 51+ messages in thread
* [PATCH v2] x86_64, vmcoreinfo: Append 'page_offset_base' to vmcoreinfo
@ 2018-11-28 11:38 ` Dave Young
0 siblings, 0 replies; 51+ messages in thread
From: Dave Young @ 2018-11-28 11:38 UTC (permalink / raw)
To: linux-arm-kernel
> > now possible to use the vmcoreinfo present inside kcore as the standard
> > ABI which can be used by the user-space utilities for reading
> > the machine specific information (and hence for debugging a
> > live kernel).
> >
> > User-space utilities like makedumpfile, kexec-tools and crash
> > are either already using this ABI or are discussing patches
> > which look to add the same feature. This helps in simplifying the
> > overall code and also in reducing code-rewrite across the
> > user-space utilities for getting values of these kernel
> > symbols/variables.
>
> > Accordingly this patch allows appending 'page_offset_base' for
> > x86_64 platforms to vmcoreinfo, so that user-space tools can use the
> > same as a standard interface to determine the start of direct mapping
> > of all physical memory.
> >
[snip]
> All above are only nitpicks though.
>
> My opinion is this: people are exporting all kinds of kernel-internal
> stuff in vmcoreinfo and frankly, I'm not crazy about this idea.
>
> And AFAICT, this thing basically bypasses KASLR completely but you need
> root for it so it probably doesn't really matter.
>
> Now, on another thread we agreed more or less that what gets exported in
> vmcoreinfo is so tightly coupled to the running kernel so that it is not
> even considered an ABI. I guess that is debatable but whatever.
We do not regard this strictly as an ABI, but we also carefully review
every new extra exported thing and only export when we have to do so eg.
something breaks.
Seems this change only make userspace tools handling on the kaslr case
easier but since everything works without this patch I would prefer not to
do it.
>
> So my only request right now would be to have all those things being
> exported, documented somewhere and I believe Lianbo is working on that.
>
> But I'm sure others will have more to say about it.
>
> --
> Regards/Gruss,
> Boris.
>
> Good mailing practices for 400: avoid top-posting and trim the reply.
>
> _______________________________________________
> kexec mailing list
> kexec at lists.infradead.org
> http://lists.infradead.org/mailman/listinfo/kexec
Thanks
Dave
^ permalink raw reply [flat|nested] 51+ messages in thread