[PATCH v2] kexec: Introduce vmcoreinfo signature verification

2017-03-16 Thread Xunlei Pang
Currently vmcoreinfo data is updated at boot time subsys_initcall(),
it has the risk of being modified by some wrong code during system
is running.

As a result, vmcore dumped may contain the wrong vmcoreinfo. Later on,
when using "crash" or "makedumpfile"(etc) utility to parse this vmcore,
we probably will get "Segmentation fault" or other unexpected/confusing
errors.

As vmcoreinfo is the most fundamental information for vmcore, we better
double check its correctness. Here we generate a signature(using crc32)
after it is saved, then verify it in crash_save_vmcoreinfo() to see if
the signature was broken, if so we have to re-save the vmcoreinfo data
to get the correct vmcoreinfo for kdump as possible as we can.

Signed-off-by: Xunlei Pang 
---
v1->v2:
- Keep crash_save_vmcoreinfo_init() because "makedumpfile --mem-usage"
  uses the information.
- Add crc32 verification for vmcoreinfo, re-save when failure.

 arch/Kconfig|  1 +
 kernel/kexec_core.c | 43 +++
 2 files changed, 36 insertions(+), 8 deletions(-)

diff --git a/arch/Kconfig b/arch/Kconfig
index c4d6833..66eb296 100644
--- a/arch/Kconfig
+++ b/arch/Kconfig
@@ -4,6 +4,7 @@
 
 config KEXEC_CORE
bool
+   select CRC32
 
 config HAVE_IMA_KEXEC
bool
diff --git a/kernel/kexec_core.c b/kernel/kexec_core.c
index bfe62d5..012acbe 100644
--- a/kernel/kexec_core.c
+++ b/kernel/kexec_core.c
@@ -38,6 +38,7 @@
 #include 
 #include 
 #include 
+#include 
 
 #include 
 #include 
@@ -53,9 +54,10 @@
 
 /* vmcoreinfo stuff */
 static unsigned char vmcoreinfo_data[VMCOREINFO_BYTES];
-u32 vmcoreinfo_note[VMCOREINFO_NOTE_SIZE/4];
+static u32 vmcoreinfo_sig;
 size_t vmcoreinfo_size;
 size_t vmcoreinfo_max_size = sizeof(vmcoreinfo_data);
+u32 vmcoreinfo_note[VMCOREINFO_NOTE_SIZE/4];
 
 /* Flag to indicate we are going to kexec a new kernel */
 bool kexec_in_progress = false;
@@ -1367,12 +1369,6 @@ static void update_vmcoreinfo_note(void)
final_note(buf);
 }
 
-void crash_save_vmcoreinfo(void)
-{
-   vmcoreinfo_append_str("CRASHTIME=%ld\n", get_seconds());
-   update_vmcoreinfo_note();
-}
-
 void vmcoreinfo_append_str(const char *fmt, ...)
 {
va_list args;
@@ -1402,7 +1398,7 @@ phys_addr_t __weak paddr_vmcoreinfo_note(void)
return __pa_symbol((unsigned long)(char *)&vmcoreinfo_note);
 }
 
-static int __init crash_save_vmcoreinfo_init(void)
+static void do_crash_save_vmcoreinfo_init(void)
 {
VMCOREINFO_OSRELEASE(init_uts_ns.name.release);
VMCOREINFO_PAGESIZE(PAGE_SIZE);
@@ -1474,6 +1470,37 @@ static int __init crash_save_vmcoreinfo_init(void)
 #endif
 
arch_crash_save_vmcoreinfo();
+}
+
+static u32 crash_calc_vmcoreinfo_sig(void)
+{
+   return crc32(~0, vmcoreinfo_data, vmcoreinfo_size);
+}
+
+static bool crash_verify_vmcoreinfo(void)
+{
+   if (crash_calc_vmcoreinfo_sig() == vmcoreinfo_sig)
+   return true;
+
+   return false;
+}
+
+void crash_save_vmcoreinfo(void)
+{
+   /* Re-save if verification fails */
+   if (!crash_verify_vmcoreinfo()) {
+   vmcoreinfo_size = 0;
+   do_crash_save_vmcoreinfo_init();
+   }
+
+   vmcoreinfo_append_str("CRASHTIME=%ld\n", get_seconds());
+   update_vmcoreinfo_note();
+}
+
+static int __init crash_save_vmcoreinfo_init(void)
+{
+   do_crash_save_vmcoreinfo_init();
+   vmcoreinfo_sig = crash_calc_vmcoreinfo_sig();
update_vmcoreinfo_note();
 
return 0;
-- 
1.8.3.1


___
kexec mailing list
kexec@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/kexec


Re: kexec regression since 4.9 caused by efi

2017-03-16 Thread Dave Young
On 03/16/17 at 12:41pm, Matt Fleming wrote:
> On Mon, 13 Mar, at 03:37:48PM, Dave Young wrote:
> > 
> > Omar, could you try below patch? Looking at the efi_mem_desc_lookup, it is 
> > not
> > correct to be used in efi_arch_mem_reserve, if it passed your test, I
> > can rewrite patch log with more background and send it out:
> > 
> > for_each_efi_memory_desc(md) {
> > [snip]
> > if (!(md->attribute & EFI_MEMORY_RUNTIME) &&
> > md->type != EFI_BOOT_SERVICES_DATA &&
> > md->type != EFI_RUNTIME_SERVICES_DATA) {
> > continue;
> > }
> > 
> > In above code, it meant to get a md of EFI_MEMORY_RUNTIME of either boot
> > data or runtime data, this is wrong for efi_mem_reserve, because we are
> > reserving boot data which has no EFI_MEMORY_RUNTIME attribute at the
> > running time. Just is happened to work and we did not capture the error.
> 
> Wouldn't something like this be simpler?
> 
> ---
> 
> diff --git a/arch/x86/platform/efi/quirks.c b/arch/x86/platform/efi/quirks.c
> index 30031d5293c4..cdfe8c628959 100644
> --- a/arch/x86/platform/efi/quirks.c
> +++ b/arch/x86/platform/efi/quirks.c
> @@ -201,6 +201,10 @@ void __init efi_arch_mem_reserve(phys_addr_t addr, u64 
> size)
>   return;
>   }
>  
> + /* No need to reserve regions that will never be freed. */
> + if (md.attribute & EFI_MEMORY_RUNTIME)
> + return;
> +

Matt, I think it should be fine although I think the md type checking in
efi_mem_desc_lookup() is causing confusion and not easy to understand..

How about move the if chunk early like below because it seems no need
to sanity check the addr + size any more if the md is still RUNTIME?

--- linux-x86.orig/arch/x86/platform/efi/quirks.c
+++ linux-x86/arch/x86/platform/efi/quirks.c
@@ -196,6 +196,10 @@ void __init efi_arch_mem_reserve(phys_ad
return;
}
 
+   /* No need to reserve regions that will never be freed. */
+   if (md.attribute & EFI_MEMORY_RUNTIME)
+   return;
+
if (addr + size > md.phys_addr + (md.num_pages << EFI_PAGE_SHIFT)) {
pr_err("Region spans EFI memory descriptors, %pa\n", &addr);
return;

Thanks
Dave

___
kexec mailing list
kexec@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/kexec


Re: kexec regression since 4.9 caused by efi

2017-03-16 Thread Omar Sandoval
On Thu, Mar 16, 2017 at 12:41:32PM +, Matt Fleming wrote:
> On Mon, 13 Mar, at 03:37:48PM, Dave Young wrote:
> > 
> > Omar, could you try below patch? Looking at the efi_mem_desc_lookup, it is 
> > not
> > correct to be used in efi_arch_mem_reserve, if it passed your test, I
> > can rewrite patch log with more background and send it out:
> > 
> > for_each_efi_memory_desc(md) {
> > [snip]
> > if (!(md->attribute & EFI_MEMORY_RUNTIME) &&
> > md->type != EFI_BOOT_SERVICES_DATA &&
> > md->type != EFI_RUNTIME_SERVICES_DATA) {
> > continue;
> > }
> > 
> > In above code, it meant to get a md of EFI_MEMORY_RUNTIME of either boot
> > data or runtime data, this is wrong for efi_mem_reserve, because we are
> > reserving boot data which has no EFI_MEMORY_RUNTIME attribute at the
> > running time. Just is happened to work and we did not capture the error.
> 
> Wouldn't something like this be simpler?
> 
> ---
> 
> diff --git a/arch/x86/platform/efi/quirks.c b/arch/x86/platform/efi/quirks.c
> index 30031d5293c4..cdfe8c628959 100644
> --- a/arch/x86/platform/efi/quirks.c
> +++ b/arch/x86/platform/efi/quirks.c
> @@ -201,6 +201,10 @@ void __init efi_arch_mem_reserve(phys_addr_t addr, u64 
> size)
>   return;
>   }
>  
> + /* No need to reserve regions that will never be freed. */
> + if (md.attribute & EFI_MEMORY_RUNTIME)
> + return;
> +
>   size += addr % EFI_PAGE_SIZE;
>   size = round_up(size, EFI_PAGE_SIZE);
>   addr = round_down(addr, EFI_PAGE_SIZE);

This works for me.

Reported-and-tested-by: Omar Sandoval 

___
kexec mailing list
kexec@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/kexec


[PATCH][kexec-tools] ppc: Fix format warning with die()

2017-03-16 Thread Jussi Kukkonen
Enable compiling kexec-tools for ppc with -Werror=format-security.

Signed-off-by: Jussi Kukkonen 
---
 kexec/arch/ppc/kexec-elf-ppc.c| 2 +-
 kexec/arch/ppc/kexec-uImage-ppc.c | 2 +-
 2 files changed, 2 insertions(+), 2 deletions(-)

diff --git a/kexec/arch/ppc/kexec-elf-ppc.c b/kexec/arch/ppc/kexec-elf-ppc.c
index 291f06d..ad43ad1 100644
--- a/kexec/arch/ppc/kexec-elf-ppc.c
+++ b/kexec/arch/ppc/kexec-elf-ppc.c
@@ -453,7 +453,7 @@ out:
if (!tmp_cmdline)
free(command_line);
if (error_msg)
-   die(error_msg);
+   die("%s", error_msg);
 
return result;
 }
diff --git a/kexec/arch/ppc/kexec-uImage-ppc.c 
b/kexec/arch/ppc/kexec-uImage-ppc.c
index 5eec6e4..e8f7adc 100644
--- a/kexec/arch/ppc/kexec-uImage-ppc.c
+++ b/kexec/arch/ppc/kexec-uImage-ppc.c
@@ -306,7 +306,7 @@ out:
if (!tmp_cmdline)
free(command_line);
if (error_msg)
-   die(error_msg);
+   die("%s", error_msg);
return ret;
 }
 
-- 
2.11.0


___
kexec mailing list
kexec@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/kexec


Re: [PATCH] kexec: Update vmcoreinfo after crash happened

2017-03-16 Thread Xunlei Pang
On 03/16/2017 at 09:18 PM, Baoquan He wrote:
> On 03/16/17 at 08:36pm, Xunlei Pang wrote:
>> On 03/16/2017 at 08:27 PM, Baoquan He wrote:
>>> Hi Xunlei,
>>>
>>> Did you really see this ever happened? Because the vmcore size estimate
>>> feature, namely --mem-usage option of makedumpfile, depends on the
>>> vmcoreinfo in 1st kernel, your change will break it.
>> Hi Baoquan,
>>
>> I can reproduce it using a kernel module which modifies the vmcoreinfo,
>> so it's a problem can actually happen.
>>
>>> If not, it could be not good to change that.
>> That's a good point, then I guess we can keep the 
>> crash_save_vmcoreinfo_init(),
>> and store again all the vmcoreinfo after crash. What do you think?
> Well, then it will make makedumpfile segfault happen too when execute
> below command in 1st kernel if it existed:
>   makedumpfile --mem-usage /proc/kcore

Yes, if the initial vmcoreinfo data was modified before "makedumpfile 
--mem-usage", it might happen,
after all the system is going something wrong. And that's why we deploy kdump 
service at the very
beginning when the system has a low possibility of going wrong.

But we have to guarantee kdump vmcore can be generated correctly as possible as 
it can.

>
> So we still need to face that problem and need fix it. vmcoreinfo_note
> is in kernel data area, how does module intrude into this area? And can
> we fix the module code?
>

Bugs always exist in products, we can't know what will happen and fix all the 
errors,
that's why we need kdump.

I think the following update should guarantee the correct vmcoreinfo for kdump.

---
 kernel/kexec_core.c | 22 +++---
 1 file changed, 15 insertions(+), 7 deletions(-)

diff --git a/kernel/kexec_core.c b/kernel/kexec_core.c
index bfe62d5..0f7b328 100644
--- a/kernel/kexec_core.c
+++ b/kernel/kexec_core.c
@@ -1367,12 +1367,6 @@ static void update_vmcoreinfo_note(void)
 final_note(buf);
 }
 
-void crash_save_vmcoreinfo(void)
-{
-vmcoreinfo_append_str("CRASHTIME=%ld\n", get_seconds());
-update_vmcoreinfo_note();
-}
-
 void vmcoreinfo_append_str(const char *fmt, ...)
 {
 va_list args;
@@ -1402,7 +1396,7 @@ phys_addr_t __weak paddr_vmcoreinfo_note(void)
 return __pa_symbol((unsigned long)(char *)&vmcoreinfo_note);
 }
 
-static int __init crash_save_vmcoreinfo_init(void)
+static void do_crash_save_vmcoreinfo_init(void)
 {
 VMCOREINFO_OSRELEASE(init_uts_ns.name.release);
 VMCOREINFO_PAGESIZE(PAGE_SIZE);
@@ -1474,6 +1468,20 @@ static int __init crash_save_vmcoreinfo_init(void)
 #endif
 
 arch_crash_save_vmcoreinfo();
+}
+
+void crash_save_vmcoreinfo(void)
+{
+/* Save again to protect vmcoreinfo from being modified */
+vmcoreinfo_size = 0;
+do_crash_save_vmcoreinfo_init();
+vmcoreinfo_append_str("CRASHTIME=%ld\n", get_seconds());
+update_vmcoreinfo_note();
+}
+
+static int __init crash_save_vmcoreinfo_init(void)
+{
+do_crash_save_vmcoreinfo_init();
 update_vmcoreinfo_note();
 
 return 0;
-- 
1.8.3.1

___
kexec mailing list
kexec@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/kexec


Re: [PATCH] kexec: Update vmcoreinfo after crash happened

2017-03-16 Thread Baoquan He
On 03/16/17 at 08:36pm, Xunlei Pang wrote:
> On 03/16/2017 at 08:27 PM, Baoquan He wrote:
> > Hi Xunlei,
> >
> > Did you really see this ever happened? Because the vmcore size estimate
> > feature, namely --mem-usage option of makedumpfile, depends on the
> > vmcoreinfo in 1st kernel, your change will break it.
> 
> Hi Baoquan,
> 
> I can reproduce it using a kernel module which modifies the vmcoreinfo,
> so it's a problem can actually happen.
> 
> > If not, it could be not good to change that.
> 
> That's a good point, then I guess we can keep the 
> crash_save_vmcoreinfo_init(),
> and store again all the vmcoreinfo after crash. What do you think?

Well, then it will make makedumpfile segfault happen too when execute
below command in 1st kernel if it existed:
makedumpfile --mem-usage /proc/kcore

So we still need to face that problem and need fix it. vmcoreinfo_note
is in kernel data area, how does module intrude into this area? And can
we fix the module code?


> 
> >
> > Baoquan
> >
> > On 03/16/17 at 08:16pm, Xunlei Pang wrote:
> >> Currently vmcoreinfo data is updated at boot time subsys_initcall(),
> >> it has the risk of being modified by some wrong code during system
> >> is running.
> >>
> >> As a result, vmcore dumped will contain the wrong vmcoreinfo. Later on,
> >> when using "crash" utility to parse this vmcore, we probably will get
> >> "Segmentation fault".
> >>
> >> Based on the fact that the value of each vmcoreinfo stays invariable
> >> once kernel boots up, we safely move all the vmcoreinfo operations into
> >> crash_save_vmcoreinfo() which is called after crash happened. In this
> >> way, vmcoreinfo data correctness is always guaranteed.
> >>
> >> Signed-off-by: Xunlei Pang 
> >> ---
> >>  kernel/kexec_core.c | 14 +++---
> >>  1 file changed, 3 insertions(+), 11 deletions(-)
> >>
> >> diff --git a/kernel/kexec_core.c b/kernel/kexec_core.c
> >> index bfe62d5..1bfdd96 100644
> >> --- a/kernel/kexec_core.c
> >> +++ b/kernel/kexec_core.c
> >> @@ -1367,12 +1367,6 @@ static void update_vmcoreinfo_note(void)
> >>final_note(buf);
> >>  }
> >>  
> >> -void crash_save_vmcoreinfo(void)
> >> -{
> >> -  vmcoreinfo_append_str("CRASHTIME=%ld\n", get_seconds());
> >> -  update_vmcoreinfo_note();
> >> -}
> >> -
> >>  void vmcoreinfo_append_str(const char *fmt, ...)
> >>  {
> >>va_list args;
> >> @@ -1402,7 +1396,7 @@ phys_addr_t __weak paddr_vmcoreinfo_note(void)
> >>return __pa_symbol((unsigned long)(char *)&vmcoreinfo_note);
> >>  }
> >>  
> >> -static int __init crash_save_vmcoreinfo_init(void)
> >> +void crash_save_vmcoreinfo(void)
> >>  {
> >>VMCOREINFO_OSRELEASE(init_uts_ns.name.release);
> >>VMCOREINFO_PAGESIZE(PAGE_SIZE);
> >> @@ -1474,13 +1468,11 @@ static int __init crash_save_vmcoreinfo_init(void)
> >>  #endif
> >>  
> >>arch_crash_save_vmcoreinfo();
> >> -  update_vmcoreinfo_note();
> >> +  vmcoreinfo_append_str("CRASHTIME=%ld\n", get_seconds());
> >>  
> >> -  return 0;
> >> +  update_vmcoreinfo_note();
> >>  }
> >>  
> >> -subsys_initcall(crash_save_vmcoreinfo_init);
> >> -
> >>  /*
> >>   * Move into place and start executing a preloaded standalone
> >>   * executable.  If nothing was preloaded return an error.
> >> -- 
> >> 1.8.3.1
> >>
> 

___
kexec mailing list
kexec@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/kexec


Re: [PATCH] kexec: Update vmcoreinfo after crash happened

2017-03-16 Thread Baoquan He
Hi Xunlei,

Did you really see this ever happened? Because the vmcore size estimate
feature, namely --mem-usage option of makedumpfile, depends on the
vmcoreinfo in 1st kernel, your change will break it.

If not, it could be not good to change that.

Baoquan

On 03/16/17 at 08:16pm, Xunlei Pang wrote:
> Currently vmcoreinfo data is updated at boot time subsys_initcall(),
> it has the risk of being modified by some wrong code during system
> is running.
> 
> As a result, vmcore dumped will contain the wrong vmcoreinfo. Later on,
> when using "crash" utility to parse this vmcore, we probably will get
> "Segmentation fault".
> 
> Based on the fact that the value of each vmcoreinfo stays invariable
> once kernel boots up, we safely move all the vmcoreinfo operations into
> crash_save_vmcoreinfo() which is called after crash happened. In this
> way, vmcoreinfo data correctness is always guaranteed.
> 
> Signed-off-by: Xunlei Pang 
> ---
>  kernel/kexec_core.c | 14 +++---
>  1 file changed, 3 insertions(+), 11 deletions(-)
> 
> diff --git a/kernel/kexec_core.c b/kernel/kexec_core.c
> index bfe62d5..1bfdd96 100644
> --- a/kernel/kexec_core.c
> +++ b/kernel/kexec_core.c
> @@ -1367,12 +1367,6 @@ static void update_vmcoreinfo_note(void)
>   final_note(buf);
>  }
>  
> -void crash_save_vmcoreinfo(void)
> -{
> - vmcoreinfo_append_str("CRASHTIME=%ld\n", get_seconds());
> - update_vmcoreinfo_note();
> -}
> -
>  void vmcoreinfo_append_str(const char *fmt, ...)
>  {
>   va_list args;
> @@ -1402,7 +1396,7 @@ phys_addr_t __weak paddr_vmcoreinfo_note(void)
>   return __pa_symbol((unsigned long)(char *)&vmcoreinfo_note);
>  }
>  
> -static int __init crash_save_vmcoreinfo_init(void)
> +void crash_save_vmcoreinfo(void)
>  {
>   VMCOREINFO_OSRELEASE(init_uts_ns.name.release);
>   VMCOREINFO_PAGESIZE(PAGE_SIZE);
> @@ -1474,13 +1468,11 @@ static int __init crash_save_vmcoreinfo_init(void)
>  #endif
>  
>   arch_crash_save_vmcoreinfo();
> - update_vmcoreinfo_note();
> + vmcoreinfo_append_str("CRASHTIME=%ld\n", get_seconds());
>  
> - return 0;
> + update_vmcoreinfo_note();
>  }
>  
> -subsys_initcall(crash_save_vmcoreinfo_init);
> -
>  /*
>   * Move into place and start executing a preloaded standalone
>   * executable.  If nothing was preloaded return an error.
> -- 
> 1.8.3.1
> 

___
kexec mailing list
kexec@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/kexec


Re: [PATCH] kexec: Update vmcoreinfo after crash happened

2017-03-16 Thread Xunlei Pang
On 03/16/2017 at 08:27 PM, Baoquan He wrote:
> Hi Xunlei,
>
> Did you really see this ever happened? Because the vmcore size estimate
> feature, namely --mem-usage option of makedumpfile, depends on the
> vmcoreinfo in 1st kernel, your change will break it.

Hi Baoquan,

I can reproduce it using a kernel module which modifies the vmcoreinfo,
so it's a problem can actually happen.

> If not, it could be not good to change that.

That's a good point, then I guess we can keep the crash_save_vmcoreinfo_init(),
and store again all the vmcoreinfo after crash. What do you think?

Regards,
Xunlei

>
> Baoquan
>
> On 03/16/17 at 08:16pm, Xunlei Pang wrote:
>> Currently vmcoreinfo data is updated at boot time subsys_initcall(),
>> it has the risk of being modified by some wrong code during system
>> is running.
>>
>> As a result, vmcore dumped will contain the wrong vmcoreinfo. Later on,
>> when using "crash" utility to parse this vmcore, we probably will get
>> "Segmentation fault".
>>
>> Based on the fact that the value of each vmcoreinfo stays invariable
>> once kernel boots up, we safely move all the vmcoreinfo operations into
>> crash_save_vmcoreinfo() which is called after crash happened. In this
>> way, vmcoreinfo data correctness is always guaranteed.
>>
>> Signed-off-by: Xunlei Pang 
>> ---
>>  kernel/kexec_core.c | 14 +++---
>>  1 file changed, 3 insertions(+), 11 deletions(-)
>>
>> diff --git a/kernel/kexec_core.c b/kernel/kexec_core.c
>> index bfe62d5..1bfdd96 100644
>> --- a/kernel/kexec_core.c
>> +++ b/kernel/kexec_core.c
>> @@ -1367,12 +1367,6 @@ static void update_vmcoreinfo_note(void)
>>  final_note(buf);
>>  }
>>  
>> -void crash_save_vmcoreinfo(void)
>> -{
>> -vmcoreinfo_append_str("CRASHTIME=%ld\n", get_seconds());
>> -update_vmcoreinfo_note();
>> -}
>> -
>>  void vmcoreinfo_append_str(const char *fmt, ...)
>>  {
>>  va_list args;
>> @@ -1402,7 +1396,7 @@ phys_addr_t __weak paddr_vmcoreinfo_note(void)
>>  return __pa_symbol((unsigned long)(char *)&vmcoreinfo_note);
>>  }
>>  
>> -static int __init crash_save_vmcoreinfo_init(void)
>> +void crash_save_vmcoreinfo(void)
>>  {
>>  VMCOREINFO_OSRELEASE(init_uts_ns.name.release);
>>  VMCOREINFO_PAGESIZE(PAGE_SIZE);
>> @@ -1474,13 +1468,11 @@ static int __init crash_save_vmcoreinfo_init(void)
>>  #endif
>>  
>>  arch_crash_save_vmcoreinfo();
>> -update_vmcoreinfo_note();
>> +vmcoreinfo_append_str("CRASHTIME=%ld\n", get_seconds());
>>  
>> -return 0;
>> +update_vmcoreinfo_note();
>>  }
>>  
>> -subsys_initcall(crash_save_vmcoreinfo_init);
>> -
>>  /*
>>   * Move into place and start executing a preloaded standalone
>>   * executable.  If nothing was preloaded return an error.
>> -- 
>> 1.8.3.1
>>


___
kexec mailing list
kexec@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/kexec


Re: kexec regression since 4.9 caused by efi

2017-03-16 Thread Matt Fleming
On Mon, 13 Mar, at 03:37:48PM, Dave Young wrote:
> 
> Omar, could you try below patch? Looking at the efi_mem_desc_lookup, it is not
> correct to be used in efi_arch_mem_reserve, if it passed your test, I
> can rewrite patch log with more background and send it out:
> 
> for_each_efi_memory_desc(md) {
>   [snip]
> if (!(md->attribute & EFI_MEMORY_RUNTIME) &&
> md->type != EFI_BOOT_SERVICES_DATA &&
> md->type != EFI_RUNTIME_SERVICES_DATA) {
> continue;
> }
> 
> In above code, it meant to get a md of EFI_MEMORY_RUNTIME of either boot
> data or runtime data, this is wrong for efi_mem_reserve, because we are
> reserving boot data which has no EFI_MEMORY_RUNTIME attribute at the
> running time. Just is happened to work and we did not capture the error.

Wouldn't something like this be simpler?

---

diff --git a/arch/x86/platform/efi/quirks.c b/arch/x86/platform/efi/quirks.c
index 30031d5293c4..cdfe8c628959 100644
--- a/arch/x86/platform/efi/quirks.c
+++ b/arch/x86/platform/efi/quirks.c
@@ -201,6 +201,10 @@ void __init efi_arch_mem_reserve(phys_addr_t addr, u64 
size)
return;
}
 
+   /* No need to reserve regions that will never be freed. */
+   if (md.attribute & EFI_MEMORY_RUNTIME)
+   return;
+
size += addr % EFI_PAGE_SIZE;
size = round_up(size, EFI_PAGE_SIZE);
addr = round_down(addr, EFI_PAGE_SIZE);

___
kexec mailing list
kexec@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/kexec


[PATCH] kexec: Update vmcoreinfo after crash happened

2017-03-16 Thread Xunlei Pang
Currently vmcoreinfo data is updated at boot time subsys_initcall(),
it has the risk of being modified by some wrong code during system
is running.

As a result, vmcore dumped will contain the wrong vmcoreinfo. Later on,
when using "crash" utility to parse this vmcore, we probably will get
"Segmentation fault".

Based on the fact that the value of each vmcoreinfo stays invariable
once kernel boots up, we safely move all the vmcoreinfo operations into
crash_save_vmcoreinfo() which is called after crash happened. In this
way, vmcoreinfo data correctness is always guaranteed.

Signed-off-by: Xunlei Pang 
---
 kernel/kexec_core.c | 14 +++---
 1 file changed, 3 insertions(+), 11 deletions(-)

diff --git a/kernel/kexec_core.c b/kernel/kexec_core.c
index bfe62d5..1bfdd96 100644
--- a/kernel/kexec_core.c
+++ b/kernel/kexec_core.c
@@ -1367,12 +1367,6 @@ static void update_vmcoreinfo_note(void)
final_note(buf);
 }
 
-void crash_save_vmcoreinfo(void)
-{
-   vmcoreinfo_append_str("CRASHTIME=%ld\n", get_seconds());
-   update_vmcoreinfo_note();
-}
-
 void vmcoreinfo_append_str(const char *fmt, ...)
 {
va_list args;
@@ -1402,7 +1396,7 @@ phys_addr_t __weak paddr_vmcoreinfo_note(void)
return __pa_symbol((unsigned long)(char *)&vmcoreinfo_note);
 }
 
-static int __init crash_save_vmcoreinfo_init(void)
+void crash_save_vmcoreinfo(void)
 {
VMCOREINFO_OSRELEASE(init_uts_ns.name.release);
VMCOREINFO_PAGESIZE(PAGE_SIZE);
@@ -1474,13 +1468,11 @@ static int __init crash_save_vmcoreinfo_init(void)
 #endif
 
arch_crash_save_vmcoreinfo();
-   update_vmcoreinfo_note();
+   vmcoreinfo_append_str("CRASHTIME=%ld\n", get_seconds());
 
-   return 0;
+   update_vmcoreinfo_note();
 }
 
-subsys_initcall(crash_save_vmcoreinfo_init);
-
 /*
  * Move into place and start executing a preloaded standalone
  * executable.  If nothing was preloaded return an error.
-- 
1.8.3.1


___
kexec mailing list
kexec@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/kexec


Re: kexec regression since 4.9 caused by efi

2017-03-16 Thread Matt Fleming
On Thu, 09 Mar, at 12:53:36PM, Ard Biesheuvel wrote:
> 
> Hi Omar,
> 
> Thanks for tracking this down.
> 
> I wonder if this is an unintended side effect of the way we repurpose
> the EFI_MEMORY_RUNTIME attribute in efi_arch_mem_reserve(). AFAIUI,
> splitting memory map entries should only be necessary for regions that
> are not runtime memory regions to begin with, and so whether their
> virtual mapping address makes sense or not should be irrelevant.
> 
> Perhaps this only illustrates my lack of understanding of the x86 way
> of doing this, so perhaps Matt can shed some light on this?

Sorry for the delay.

Yes, Ard is correct. It's not necessary to split/reserve memory
regions that already have the EFI_MEMORY_RUNTIME attribute.

___
kexec mailing list
kexec@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/kexec


Re: [PATCH v33 00/14] add kdump support

2017-03-16 Thread David Woodhouse
On Thu, 2017-03-16 at 09:23 +0900, AKASHI Takahiro wrote:
> 
> I double-checked but saw no warnings like these neither for v32 nor v33.
> I'm afraid that you might have done something wrong in backporting,
> particularly patch#5.

Then I apologise for the noise. Thanks for checking.

smime.p7s
Description: S/MIME cryptographic signature
___
kexec mailing list
kexec@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/kexec


[PATCH] crashdump: Remove stray get_crashkernel_region() declaration

2017-03-16 Thread Daniel Kiper
Signed-off-by: Daniel Kiper 
---
 kexec/crashdump.h |1 -
 1 file changed, 1 deletion(-)

diff --git a/kexec/crashdump.h b/kexec/crashdump.h
index 96219a8..86e1ef2 100644
--- a/kexec/crashdump.h
+++ b/kexec/crashdump.h
@@ -1,7 +1,6 @@
 #ifndef CRASHDUMP_H
 #define CRASHDUMP_H
 
-int get_crashkernel_region(uint64_t *start, uint64_t *end);
 extern int get_crash_notes_per_cpu(int cpu, uint64_t *addr, uint64_t *len);
 extern int get_kernel_vmcoreinfo(uint64_t *addr, uint64_t *len);
 extern int get_xen_vmcoreinfo(uint64_t *addr, uint64_t *len);
-- 
1.7.10.4


___
kexec mailing list
kexec@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/kexec