linux-kernel.vger.kernel.org archive mirror
 help / color / mirror / Atom feed
* [PATCH] Revert "Revert "kdump, vmcoreinfo: report memory sections virtual addresses""
@ 2016-12-15 16:17 Thomas Garnier
  2016-12-15 17:50 ` Eric W. Biederman
  0 siblings, 1 reply; 5+ messages in thread
From: Thomas Garnier @ 2016-12-15 16:17 UTC (permalink / raw)
  To: Thomas Gleixner, Ingo Molnar, H . Peter Anvin, Eric Biederman,
	Andrew Morton, Baoquan He, Thomas Garnier, Xunlei Pang,
	HATAYAMA Daisuke
  Cc: x86, linux-kernel, kexec, kernel-hardening

This reverts commit 49fd897573c97b0eaf10f47d850027d78c456cd7.

Reverting back to commit 0549a3c because the values are used by crash
and other tools already. I expected this commit would not go through given
the unresolved comments. I want it to be easy to resolve major memory
section positions when KASLR memory randomization is enabled.

Signed-off-by: Thomas Garnier <thgarnie@google.com>
---
 arch/x86/kernel/machine_kexec_64.c | 3 +++
 include/linux/kexec.h              | 6 ++++++
 2 files changed, 9 insertions(+)

diff --git a/arch/x86/kernel/machine_kexec_64.c b/arch/x86/kernel/machine_kexec_64.c
index 2e3c34b..05f3367 100644
--- a/arch/x86/kernel/machine_kexec_64.c
+++ b/arch/x86/kernel/machine_kexec_64.c
@@ -339,6 +339,9 @@ void arch_crash_save_vmcoreinfo(void)
 			      kaslr_offset());
 	VMCOREINFO_NUMBER(KERNEL_IMAGE_SIZE);
 	VMCOREINFO_PHYS_BASE(phys_base);
+	VMCOREINFO_PAGE_OFFSET(PAGE_OFFSET);
+	VMCOREINFO_VMALLOC_START(VMALLOC_START);
+	VMCOREINFO_VMEMMAP_START(VMEMMAP_START);
 }
 
 /* arch-dependent functionality related to kexec file-based syscall */
diff --git a/include/linux/kexec.h b/include/linux/kexec.h
index e98e546..ff9c876 100644
--- a/include/linux/kexec.h
+++ b/include/linux/kexec.h
@@ -285,6 +285,12 @@ phys_addr_t paddr_vmcoreinfo_note(void);
 	vmcoreinfo_append_str("CONFIG_%s=y\n", #name)
 #define VMCOREINFO_PHYS_BASE(value) \
 	vmcoreinfo_append_str("PHYS_BASE=%lx\n", (unsigned long)value)
+#define VMCOREINFO_PAGE_OFFSET(value) \
+	vmcoreinfo_append_str("PAGE_OFFSET=%lx\n", (unsigned long)value)
+#define VMCOREINFO_VMALLOC_START(value) \
+	vmcoreinfo_append_str("VMALLOC_START=%lx\n", (unsigned long)value)
+#define VMCOREINFO_VMEMMAP_START(value) \
+	vmcoreinfo_append_str("VMEMMAP_START=%lx\n", (unsigned long)value)
 
 extern struct kimage *kexec_image;
 extern struct kimage *kexec_crash_image;
-- 
2.8.0.rc3.226.g39d4020

^ permalink raw reply related	[flat|nested] 5+ messages in thread

* Re: [PATCH] Revert "Revert "kdump, vmcoreinfo: report memory sections virtual addresses""
  2016-12-15 16:17 [PATCH] Revert "Revert "kdump, vmcoreinfo: report memory sections virtual addresses"" Thomas Garnier
@ 2016-12-15 17:50 ` Eric W. Biederman
  2016-12-15 18:16   ` Thomas Garnier
  0 siblings, 1 reply; 5+ messages in thread
From: Eric W. Biederman @ 2016-12-15 17:50 UTC (permalink / raw)
  To: Thomas Garnier
  Cc: Thomas Gleixner, Ingo Molnar, H . Peter Anvin, Andrew Morton,
	Baoquan He, Xunlei Pang, HATAYAMA Daisuke, x86, linux-kernel,
	kexec, kernel-hardening

Thomas Garnier <thgarnie@google.com> writes:

> This reverts commit 49fd897573c97b0eaf10f47d850027d78c456cd7.
>
> Reverting back to commit 0549a3c because the values are used by crash
> and other tools already. I expected this commit would not go through given
> the unresolved comments. I want it to be easy to resolve major memory
> section positions when KASLR memory randomization is enabled.

This patch is broken.  The commit referenced is wrong,
as is the justification.

These values are not in fact widely used by userspace (they are brand new).

This is a very fragile approach relying on kernel implementation
details, so if we can do anything else that is more robust it
is much more likely to pass the test of time.

And yes a more robust implementation has been already discussed.

Nacked-by: "Eric W. Biederman" <ebiederm@xmission.com>

>
> Signed-off-by: Thomas Garnier <thgarnie@google.com>
> ---
>  arch/x86/kernel/machine_kexec_64.c | 3 +++
>  include/linux/kexec.h              | 6 ++++++
>  2 files changed, 9 insertions(+)
>
> diff --git a/arch/x86/kernel/machine_kexec_64.c b/arch/x86/kernel/machine_kexec_64.c
> index 2e3c34b..05f3367 100644
> --- a/arch/x86/kernel/machine_kexec_64.c
> +++ b/arch/x86/kernel/machine_kexec_64.c
> @@ -339,6 +339,9 @@ void arch_crash_save_vmcoreinfo(void)
>  			      kaslr_offset());
>  	VMCOREINFO_NUMBER(KERNEL_IMAGE_SIZE);
>  	VMCOREINFO_PHYS_BASE(phys_base);
> +	VMCOREINFO_PAGE_OFFSET(PAGE_OFFSET);
> +	VMCOREINFO_VMALLOC_START(VMALLOC_START);
> +	VMCOREINFO_VMEMMAP_START(VMEMMAP_START);
>  }
>  
>  /* arch-dependent functionality related to kexec file-based syscall */
> diff --git a/include/linux/kexec.h b/include/linux/kexec.h
> index e98e546..ff9c876 100644
> --- a/include/linux/kexec.h
> +++ b/include/linux/kexec.h
> @@ -285,6 +285,12 @@ phys_addr_t paddr_vmcoreinfo_note(void);
>  	vmcoreinfo_append_str("CONFIG_%s=y\n", #name)
>  #define VMCOREINFO_PHYS_BASE(value) \
>  	vmcoreinfo_append_str("PHYS_BASE=%lx\n", (unsigned long)value)
> +#define VMCOREINFO_PAGE_OFFSET(value) \
> +	vmcoreinfo_append_str("PAGE_OFFSET=%lx\n", (unsigned long)value)
> +#define VMCOREINFO_VMALLOC_START(value) \
> +	vmcoreinfo_append_str("VMALLOC_START=%lx\n", (unsigned long)value)
> +#define VMCOREINFO_VMEMMAP_START(value) \
> +	vmcoreinfo_append_str("VMEMMAP_START=%lx\n", (unsigned long)value)
>  
>  extern struct kimage *kexec_image;
>  extern struct kimage *kexec_crash_image;

^ permalink raw reply	[flat|nested] 5+ messages in thread

* Re: [PATCH] Revert "Revert "kdump, vmcoreinfo: report memory sections virtual addresses""
  2016-12-15 17:50 ` Eric W. Biederman
@ 2016-12-15 18:16   ` Thomas Garnier
  2016-12-15 19:45     ` Thomas Garnier
  0 siblings, 1 reply; 5+ messages in thread
From: Thomas Garnier @ 2016-12-15 18:16 UTC (permalink / raw)
  To: Eric W. Biederman
  Cc: Thomas Gleixner, Ingo Molnar, H . Peter Anvin, Andrew Morton,
	Baoquan He, Xunlei Pang, HATAYAMA Daisuke,
	the arch/x86 maintainers, LKML, kexec, Kernel Hardening

On Thu, Dec 15, 2016 at 9:50 AM, Eric W. Biederman
<ebiederm@xmission.com> wrote:
> Thomas Garnier <thgarnie@google.com> writes:
>
>> This reverts commit 49fd897573c97b0eaf10f47d850027d78c456cd7.
>>
>> Reverting back to commit 0549a3c because the values are used by crash
>> and other tools already. I expected this commit would not go through given
>> the unresolved comments. I want it to be easy to resolve major memory
>> section positions when KASLR memory randomization is enabled.
>
> This patch is broken.  The commit referenced is wrong,

Yes, I based them on linux-next. I can update them to linux main tree.
Sorry about that.

> as is the justification.
>
> These values are not in fact widely used by userspace (they are brand new).
>

They were new and got through to the master tree before and on 4.9. I
didn't get any feedback on that when it went through.

> This is a very fragile approach relying on kernel implementation
> details, so if we can do anything else that is more robust it
> is much more likely to pass the test of time.
>
> And yes a more robust implementation has been already discussed.
>

There were discussed for the PAGE_OFFSET but not VMALLOC_START. What's
your approach for it?

> Nacked-by: "Eric W. Biederman" <ebiederm@xmission.com>
>
>>
>> Signed-off-by: Thomas Garnier <thgarnie@google.com>
>> ---
>>  arch/x86/kernel/machine_kexec_64.c | 3 +++
>>  include/linux/kexec.h              | 6 ++++++
>>  2 files changed, 9 insertions(+)
>>
>> diff --git a/arch/x86/kernel/machine_kexec_64.c b/arch/x86/kernel/machine_kexec_64.c
>> index 2e3c34b..05f3367 100644
>> --- a/arch/x86/kernel/machine_kexec_64.c
>> +++ b/arch/x86/kernel/machine_kexec_64.c
>> @@ -339,6 +339,9 @@ void arch_crash_save_vmcoreinfo(void)
>>                             kaslr_offset());
>>       VMCOREINFO_NUMBER(KERNEL_IMAGE_SIZE);
>>       VMCOREINFO_PHYS_BASE(phys_base);
>> +     VMCOREINFO_PAGE_OFFSET(PAGE_OFFSET);
>> +     VMCOREINFO_VMALLOC_START(VMALLOC_START);
>> +     VMCOREINFO_VMEMMAP_START(VMEMMAP_START);
>>  }
>>
>>  /* arch-dependent functionality related to kexec file-based syscall */
>> diff --git a/include/linux/kexec.h b/include/linux/kexec.h
>> index e98e546..ff9c876 100644
>> --- a/include/linux/kexec.h
>> +++ b/include/linux/kexec.h
>> @@ -285,6 +285,12 @@ phys_addr_t paddr_vmcoreinfo_note(void);
>>       vmcoreinfo_append_str("CONFIG_%s=y\n", #name)
>>  #define VMCOREINFO_PHYS_BASE(value) \
>>       vmcoreinfo_append_str("PHYS_BASE=%lx\n", (unsigned long)value)
>> +#define VMCOREINFO_PAGE_OFFSET(value) \
>> +     vmcoreinfo_append_str("PAGE_OFFSET=%lx\n", (unsigned long)value)
>> +#define VMCOREINFO_VMALLOC_START(value) \
>> +     vmcoreinfo_append_str("VMALLOC_START=%lx\n", (unsigned long)value)
>> +#define VMCOREINFO_VMEMMAP_START(value) \
>> +     vmcoreinfo_append_str("VMEMMAP_START=%lx\n", (unsigned long)value)
>>
>>  extern struct kimage *kexec_image;
>>  extern struct kimage *kexec_crash_image;



-- 
Thomas

^ permalink raw reply	[flat|nested] 5+ messages in thread

* Re: [PATCH] Revert "Revert "kdump, vmcoreinfo: report memory sections virtual addresses""
  2016-12-15 18:16   ` Thomas Garnier
@ 2016-12-15 19:45     ` Thomas Garnier
  2016-12-16  7:52       ` Baoquan He
  0 siblings, 1 reply; 5+ messages in thread
From: Thomas Garnier @ 2016-12-15 19:45 UTC (permalink / raw)
  To: Eric W. Biederman
  Cc: Thomas Gleixner, Ingo Molnar, H . Peter Anvin, Andrew Morton,
	Baoquan He, Xunlei Pang, HATAYAMA Daisuke,
	the arch/x86 maintainers, LKML, kexec, Kernel Hardening

On Thu, Dec 15, 2016 at 10:16 AM, Thomas Garnier <thgarnie@google.com> wrote:
> On Thu, Dec 15, 2016 at 9:50 AM, Eric W. Biederman
> <ebiederm@xmission.com> wrote:
>> Thomas Garnier <thgarnie@google.com> writes:
>>
>>> This reverts commit 49fd897573c97b0eaf10f47d850027d78c456cd7.
>>>
>>> Reverting back to commit 0549a3c because the values are used by crash
>>> and other tools already. I expected this commit would not go through given
>>> the unresolved comments. I want it to be easy to resolve major memory
>>> section positions when KASLR memory randomization is enabled.
>>
>> This patch is broken.  The commit referenced is wrong,
>
> Yes, I based them on linux-next. I can update them to linux main tree.
> Sorry about that.
>
>> as is the justification.
>>
>> These values are not in fact widely used by userspace (they are brand new).
>>
>
> They were new and got through to the master tree before and on 4.9. I
> didn't get any feedback on that when it went through.
>
>> This is a very fragile approach relying on kernel implementation
>> details, so if we can do anything else that is more robust it
>> is much more likely to pass the test of time.
>>
>> And yes a more robust implementation has been already discussed.
>>
>
> There were discussed for the PAGE_OFFSET but not VMALLOC_START. What's
> your approach for it?
>

Also with improvement on KASLR memory randomization, I don't think we
should assume memory sections by looking at PT_LOAD first entries. We
will certainly try to randomize the order. That's why I thought
clearly indicating which VA define which memory section was a good
idea.

I also want to find a time proof approach where we can use it no
matter the changes to KASLR.

>> Nacked-by: "Eric W. Biederman" <ebiederm@xmission.com>
>>
>>>
>>> Signed-off-by: Thomas Garnier <thgarnie@google.com>
>>> ---
>>>  arch/x86/kernel/machine_kexec_64.c | 3 +++
>>>  include/linux/kexec.h              | 6 ++++++
>>>  2 files changed, 9 insertions(+)
>>>
>>> diff --git a/arch/x86/kernel/machine_kexec_64.c b/arch/x86/kernel/machine_kexec_64.c
>>> index 2e3c34b..05f3367 100644
>>> --- a/arch/x86/kernel/machine_kexec_64.c
>>> +++ b/arch/x86/kernel/machine_kexec_64.c
>>> @@ -339,6 +339,9 @@ void arch_crash_save_vmcoreinfo(void)
>>>                             kaslr_offset());
>>>       VMCOREINFO_NUMBER(KERNEL_IMAGE_SIZE);
>>>       VMCOREINFO_PHYS_BASE(phys_base);
>>> +     VMCOREINFO_PAGE_OFFSET(PAGE_OFFSET);
>>> +     VMCOREINFO_VMALLOC_START(VMALLOC_START);
>>> +     VMCOREINFO_VMEMMAP_START(VMEMMAP_START);
>>>  }
>>>
>>>  /* arch-dependent functionality related to kexec file-based syscall */
>>> diff --git a/include/linux/kexec.h b/include/linux/kexec.h
>>> index e98e546..ff9c876 100644
>>> --- a/include/linux/kexec.h
>>> +++ b/include/linux/kexec.h
>>> @@ -285,6 +285,12 @@ phys_addr_t paddr_vmcoreinfo_note(void);
>>>       vmcoreinfo_append_str("CONFIG_%s=y\n", #name)
>>>  #define VMCOREINFO_PHYS_BASE(value) \
>>>       vmcoreinfo_append_str("PHYS_BASE=%lx\n", (unsigned long)value)
>>> +#define VMCOREINFO_PAGE_OFFSET(value) \
>>> +     vmcoreinfo_append_str("PAGE_OFFSET=%lx\n", (unsigned long)value)
>>> +#define VMCOREINFO_VMALLOC_START(value) \
>>> +     vmcoreinfo_append_str("VMALLOC_START=%lx\n", (unsigned long)value)
>>> +#define VMCOREINFO_VMEMMAP_START(value) \
>>> +     vmcoreinfo_append_str("VMEMMAP_START=%lx\n", (unsigned long)value)
>>>
>>>  extern struct kimage *kexec_image;
>>>  extern struct kimage *kexec_crash_image;
>
>
>
> --
> Thomas



-- 
Thomas

^ permalink raw reply	[flat|nested] 5+ messages in thread

* Re: [PATCH] Revert "Revert "kdump, vmcoreinfo: report memory sections virtual addresses""
  2016-12-15 19:45     ` Thomas Garnier
@ 2016-12-16  7:52       ` Baoquan He
  0 siblings, 0 replies; 5+ messages in thread
From: Baoquan He @ 2016-12-16  7:52 UTC (permalink / raw)
  To: Thomas Garnier
  Cc: Eric W. Biederman, Kernel Hardening, kexec,
	the arch/x86 maintainers, Xunlei Pang, LKML, HATAYAMA Daisuke,
	Ingo Molnar, H . Peter Anvin, Thomas Gleixner, Andrew Morton

On 12/15/16 at 11:45am, Thomas Garnier wrote:
> >> as is the justification.
> >>
> >> These values are not in fact widely used by userspace (they are brand new).
> >>
> >
> > They were new and got through to the master tree before and on 4.9. I
> > didn't get any feedback on that when it went through.
> >
> >> This is a very fragile approach relying on kernel implementation
> >> details, so if we can do anything else that is more robust it
> >> is much more likely to pass the test of time.
> >>
> >> And yes a more robust implementation has been already discussed.
> >>
> >
> > There were discussed for the PAGE_OFFSET but not VMALLOC_START. What's
> > your approach for it?

Sorry, Thomas. I didn't realize these are also required other than
Makedumpfile. So just revert it after we use a new way to get
page_offset and translate VA to PA by page table in makedumpfile. For
the time being they are not needed any more by makedumpfile.

While from below discussion we can see Atsushi, maintainer of
Makedumpfile, still worry about the parsing PT_LOAD segment header way.
I personally think exporting them is easier and more understandable for
user space utility.

http://lists.infradead.org/pipermail/kexec/2016-December/017815.html

I can't think of other way for your tool, maybe page_offset you can
use the similar way makedumpfile is taking. As for VMALLOC_START and
VMEMMAP_START, it could be necessary. And if you are planning to
randomize the memory section order, thinking about them carefully
now is a good chance lest they need be adjusted again later. 

Sorry again for this.

> >
> 
> Also with improvement on KASLR memory randomization, I don't think we
> should assume memory sections by looking at PT_LOAD first entries. We
> will certainly try to randomize the order. That's why I thought
> clearly indicating which VA define which memory section was a good
> idea.
> 
> I also want to find a time proof approach where we can use it no
> matter the changes to KASLR.
> 
> >> Nacked-by: "Eric W. Biederman" <ebiederm@xmission.com>
> >>
> >>>
> >>> Signed-off-by: Thomas Garnier <thgarnie@google.com>
> >>> ---
> >>>  arch/x86/kernel/machine_kexec_64.c | 3 +++
> >>>  include/linux/kexec.h              | 6 ++++++
> >>>  2 files changed, 9 insertions(+)
> >>>
> >>> diff --git a/arch/x86/kernel/machine_kexec_64.c b/arch/x86/kernel/machine_kexec_64.c
> >>> index 2e3c34b..05f3367 100644
> >>> --- a/arch/x86/kernel/machine_kexec_64.c
> >>> +++ b/arch/x86/kernel/machine_kexec_64.c
> >>> @@ -339,6 +339,9 @@ void arch_crash_save_vmcoreinfo(void)
> >>>                             kaslr_offset());
> >>>       VMCOREINFO_NUMBER(KERNEL_IMAGE_SIZE);
> >>>       VMCOREINFO_PHYS_BASE(phys_base);
> >>> +     VMCOREINFO_PAGE_OFFSET(PAGE_OFFSET);
> >>> +     VMCOREINFO_VMALLOC_START(VMALLOC_START);
> >>> +     VMCOREINFO_VMEMMAP_START(VMEMMAP_START);
> >>>  }
> >>>
> >>>  /* arch-dependent functionality related to kexec file-based syscall */
> >>> diff --git a/include/linux/kexec.h b/include/linux/kexec.h
> >>> index e98e546..ff9c876 100644
> >>> --- a/include/linux/kexec.h
> >>> +++ b/include/linux/kexec.h
> >>> @@ -285,6 +285,12 @@ phys_addr_t paddr_vmcoreinfo_note(void);
> >>>       vmcoreinfo_append_str("CONFIG_%s=y\n", #name)
> >>>  #define VMCOREINFO_PHYS_BASE(value) \
> >>>       vmcoreinfo_append_str("PHYS_BASE=%lx\n", (unsigned long)value)
> >>> +#define VMCOREINFO_PAGE_OFFSET(value) \
> >>> +     vmcoreinfo_append_str("PAGE_OFFSET=%lx\n", (unsigned long)value)
> >>> +#define VMCOREINFO_VMALLOC_START(value) \
> >>> +     vmcoreinfo_append_str("VMALLOC_START=%lx\n", (unsigned long)value)
> >>> +#define VMCOREINFO_VMEMMAP_START(value) \
> >>> +     vmcoreinfo_append_str("VMEMMAP_START=%lx\n", (unsigned long)value)
> >>>
> >>>  extern struct kimage *kexec_image;
> >>>  extern struct kimage *kexec_crash_image;
> >
> >
> >
> > --
> > Thomas
> 
> 
> 
> -- 
> Thomas
> 
> _______________________________________________
> kexec mailing list
> kexec@lists.infradead.org
> http://lists.infradead.org/mailman/listinfo/kexec

^ permalink raw reply	[flat|nested] 5+ messages in thread

end of thread, other threads:[~2016-12-16  8:01 UTC | newest]

Thread overview: 5+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2016-12-15 16:17 [PATCH] Revert "Revert "kdump, vmcoreinfo: report memory sections virtual addresses"" Thomas Garnier
2016-12-15 17:50 ` Eric W. Biederman
2016-12-15 18:16   ` Thomas Garnier
2016-12-15 19:45     ` Thomas Garnier
2016-12-16  7:52       ` Baoquan He

This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).