Groups | Search | Server Info | Keyboard shortcuts | Login | Register [http] [https] [nntp] [nntps]

Groups > linux.kernel > #1297368 > unrolled thread

[PATCH 1/2] kexec: Introduce a protection mechanism for the crashkernel reserved memory

Back to article view | Back to linux.kernel

Contents

  [PATCH 1/2] kexec: Introduce a protection mechanism for the crashkernel reserved memory Xunlei Pang <xlpang@redhat.com> - 2015-12-23 12:20 +0100
    [PATCH 2/2] kexec: Provide arch_kexec_protect(unprotect)_crashkres() Xunlei Pang <xlpang@redhat.com> - 2015-12-23 12:20 +0100
      Re: [PATCH 2/2] kexec: Provide  arch_kexec_protect(unprotect)_crashkres() Dave Young <dyoung@redhat.com> - 2015-12-24 07:00 +0100
        Re: [PATCH 2/2] kexec: Provide  arch_kexec_protect(unprotect)_crashkres() Xunlei Pang <xlpang@redhat.com> - 2015-12-24 07:10 +0100
          Re: [PATCH 2/2] kexec: Provide  arch_kexec_protect(unprotect)_crashkres() Dave Young <dyoung@redhat.com> - 2015-12-24 07:20 +0100
            Re: [PATCH 2/2] kexec: Provide  arch_kexec_protect(unprotect)_crashkres() Xunlei Pang <xlpang@redhat.com> - 2015-12-24 07:50 +0100
              Re: [PATCH 2/2] kexec: Provide  arch_kexec_protect(unprotect)_crashkres() Xunlei Pang <xlpang@redhat.com> - 2015-12-28 07:40 +0100
                Re: [PATCH 2/2] kexec: Provide  arch_kexec_protect(unprotect)_crashkres() Minfei Huang <mhuang@redhat.com> - 2015-12-28 13:20 +0100
                  Re: [PATCH 2/2] kexec: Provide  arch_kexec_protect(unprotect)_crashkres() Minfei Huang <mhuang@redhat.com> - 2015-12-28 13:20 +0100
                  Re: [PATCH 2/2] kexec: Provide  arch_kexec_protect(unprotect)_crashkres() Xunlei Pang <xlpang@redhat.com> - 2015-12-29 12:10 +0100
      Re: [PATCH 2/2] kexec: Provide  arch_kexec_protect(unprotect)_crashkres() Minfei Huang <mhuang@redhat.com> - 2015-12-26 16:20 +0100

#1297368 — [PATCH 1/2] kexec: Introduce a protection mechanism for the crashkernel reserved memory

For the cases that some kernel (module) path stamps the crash
reserved memory(already mapped by the kernel) where has been
loaded the second kernel data, the kdump kernel will probably
fail to boot when panic happens (or even not happens) leaving
the culprit at large, this is unacceptable.

The patch introduces a mechanism for detecting such cases:
1) After each crash kexec loading, it simply marks the reserved
memory regions readonly since we no longer access it after that.
When someone stamps the region, the first kernel will panic and
trigger the kdump. The weak arch_kexec_protect_crashkres() is
introduced to do the actual protection.

2) To allow multiple loading, once 1) was done we also need to
remark the reserved memory to readwrite each time a system call
related to kdump is made. The weak arch_kexec_unprotect_crashkres()
is introduced to do the actual protection.

The architecture can make its specific implementation by overriding
arch_kexec_protect_crashkres() and arch_kexec_unprotect_crashkres().

Signed-off-by: Xunlei Pang <xlpang@redhat.com>
---
 include/linux/kexec.h | 2 ++
 kernel/kexec.c        | 9 ++++++++-
 kernel/kexec_core.c   | 6 ++++++
 kernel/kexec_file.c   | 8 +++++++-
 4 files changed, 23 insertions(+), 2 deletions(-)

diff --git a/include/linux/kexec.h b/include/linux/kexec.h
index 7b68d27..ebd6950 100644
--- a/include/linux/kexec.h
+++ b/include/linux/kexec.h
@@ -329,6 +329,8 @@ int __weak arch_kexec_apply_relocations_add(const Elf_Ehdr *ehdr,
 					Elf_Shdr *sechdrs, unsigned int relsec);
 int __weak arch_kexec_apply_relocations(const Elf_Ehdr *ehdr, Elf_Shdr *sechdrs,
 					unsigned int relsec);
+void arch_kexec_protect_crashkres(void);
+void arch_kexec_unprotect_crashkres(void);
 
 #else /* !CONFIG_KEXEC_CORE */
 struct pt_regs;
diff --git a/kernel/kexec.c b/kernel/kexec.c
index d873b64..3680f9c 100644
--- a/kernel/kexec.c
+++ b/kernel/kexec.c
@@ -167,8 +167,12 @@ SYSCALL_DEFINE4(kexec_load, unsigned long, entry, unsigned long, nr_segments,
 		return -EBUSY;
 
 	dest_image = &kexec_image;
-	if (flags & KEXEC_ON_CRASH)
+	if (flags & KEXEC_ON_CRASH) {
 		dest_image = &kexec_crash_image;
+		if (kexec_crash_image)
+			arch_kexec_unprotect_crashkres();
+	}
+
 	if (nr_segments > 0) {
 		unsigned long i;
 
@@ -211,6 +215,9 @@ SYSCALL_DEFINE4(kexec_load, unsigned long, entry, unsigned long, nr_segments,
 	image = xchg(dest_image, image);
 
 out:
+	if ((flags & KEXEC_ON_CRASH) && kexec_crash_image)
+		arch_kexec_protect_crashkres();
+
 	mutex_unlock(&kexec_mutex);
 	kimage_free(image);
 
diff --git a/kernel/kexec_core.c b/kernel/kexec_core.c
index c823f30..de4dd80 100644
--- a/kernel/kexec_core.c
+++ b/kernel/kexec_core.c
@@ -1560,3 +1560,9 @@ void __weak crash_map_reserved_pages(void)
 
 void __weak crash_unmap_reserved_pages(void)
 {}
+
+void __weak arch_kexec_protect_crashkres(void)
+{}
+
+void __weak arch_kexec_unprotect_crashkres(void)
+{}
diff --git a/kernel/kexec_file.c b/kernel/kexec_file.c
index b70ada0..211eb97 100644
--- a/kernel/kexec_file.c
+++ b/kernel/kexec_file.c
@@ -327,8 +327,11 @@ SYSCALL_DEFINE5(kexec_file_load, int, kernel_fd, int, initrd_fd,
 		return -EBUSY;
 
 	dest_image = &kexec_image;
-	if (flags & KEXEC_FILE_ON_CRASH)
+	if (flags & KEXEC_FILE_ON_CRASH) {
 		dest_image = &kexec_crash_image;
+		if (kexec_crash_image)
+			arch_kexec_unprotect_crashkres();
+	}
 
 	if (flags & KEXEC_FILE_UNLOAD)
 		goto exchange;
@@ -377,6 +380,9 @@ SYSCALL_DEFINE5(kexec_file_load, int, kernel_fd, int, initrd_fd,
 exchange:
 	image = xchg(dest_image, image);
 out:
+	if ((flags & KEXEC_ON_CRASH) && kexec_crash_image)
+		arch_kexec_protect_crashkres();
+
 	mutex_unlock(&kexec_mutex);
 	kimage_free(image);
 	return ret;
-- 
2.5.0

--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majordomo@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/

[toc] | [next] | [standalone]

#1297372 — [PATCH 2/2] kexec: Provide arch_kexec_protect(unprotect)_crashkres()

Implement the protection method for the crash kernel memory
reservation for the 64-bit x86 kdump.

Signed-off-by: Xunlei Pang <xlpang@redhat.com>
---
Only provided x86_64 implementation, as I've only tested on x86_64 so far.

 arch/x86/kernel/machine_kexec_64.c | 43 ++++++++++++++++++++++++++++++++++++++
 1 file changed, 43 insertions(+)

diff --git a/arch/x86/kernel/machine_kexec_64.c b/arch/x86/kernel/machine_kexec_64.c
index 819ab3f..a3d289c 100644
--- a/arch/x86/kernel/machine_kexec_64.c
+++ b/arch/x86/kernel/machine_kexec_64.c
@@ -536,3 +536,46 @@ overflow:
 	return -ENOEXEC;
 }
 #endif /* CONFIG_KEXEC_FILE */
+
+#ifdef CONFIG_KEXEC_CORE
+static int
+kexec_mark_range(unsigned long start, unsigned long end, bool protect)
+{
+	struct page *page;
+	unsigned int nr_pages;
+
+	if (!start || !end || start >= end)
+		return 0;
+
+	page = pfn_to_page(start >> PAGE_SHIFT);
+	nr_pages = (end + 1 - start) >> PAGE_SHIFT;
+	if (protect)
+		return set_pages_ro(page, nr_pages);
+	else
+		return set_pages_rw(page, nr_pages);
+}
+
+static void kexec_mark_crashkres(bool protect)
+{
+	unsigned long control;
+
+	kexec_mark_range(crashk_low_res.start, crashk_low_res.end, protect);
+
+	/* Don't touch the control code page used in crash_kexec().*/
+	control = PFN_PHYS(page_to_pfn(kexec_crash_image->control_code_page));
+	/* Control code page is located in the 2nd page. */
+	control = control + PAGE_SIZE;
+	kexec_mark_range(crashk_res.start, control - 1, protect);
+	kexec_mark_range(control + PAGE_SIZE, crashk_res.end, protect);
+}
+
+void arch_kexec_protect_crashkres(void)
+{
+	kexec_mark_crashkres(true);
+}
+
+void arch_kexec_unprotect_crashkres(void)
+{
+	kexec_mark_crashkres(false);
+}
+#endif
-- 
2.5.0

--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majordomo@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/

[toc] | [prev] | [next] | [standalone]

#1297763 — Re: [PATCH 2/2] kexec: Provide arch_kexec_protect(unprotect)_crashkres()

Ccing Vivek

On 12/23/15 at 07:12pm, Xunlei Pang wrote:
> Implement the protection method for the crash kernel memory
> reservation for the 64-bit x86 kdump.
> 
> Signed-off-by: Xunlei Pang <xlpang@redhat.com>
> ---
> Only provided x86_64 implementation, as I've only tested on x86_64 so far.
> 
>  arch/x86/kernel/machine_kexec_64.c | 43 ++++++++++++++++++++++++++++++++++++++
>  1 file changed, 43 insertions(+)
> 
> diff --git a/arch/x86/kernel/machine_kexec_64.c b/arch/x86/kernel/machine_kexec_64.c
> index 819ab3f..a3d289c 100644
> --- a/arch/x86/kernel/machine_kexec_64.c
> +++ b/arch/x86/kernel/machine_kexec_64.c
> @@ -536,3 +536,46 @@ overflow:
>  	return -ENOEXEC;
>  }
>  #endif /* CONFIG_KEXEC_FILE */
> +
> +#ifdef CONFIG_KEXEC_CORE

The file is only compiled when CONFIG_KEXEC_CORE=y so #ifdef is not necessary

> +static int
> +kexec_mark_range(unsigned long start, unsigned long end, bool protect)
> +{
> +	struct page *page;
> +	unsigned int nr_pages;
> +
> +	if (!start || !end || start >= end)
> +		return 0;
> +
> +	page = pfn_to_page(start >> PAGE_SHIFT);
> +	nr_pages = (end + 1 - start) >> PAGE_SHIFT;
> +	if (protect)
> +		return set_pages_ro(page, nr_pages);
> +	else
> +		return set_pages_rw(page, nr_pages);

May use set_memory_ro/rw to avoid converting to *page?

> +}
> +
> +static void kexec_mark_crashkres(bool protect)
> +{
> +	unsigned long control;
> +
> +	kexec_mark_range(crashk_low_res.start, crashk_low_res.end, protect);
> +
> +	/* Don't touch the control code page used in crash_kexec().*/
> +	control = PFN_PHYS(page_to_pfn(kexec_crash_image->control_code_page));
> +	/* Control code page is located in the 2nd page. */
> +	control = control + PAGE_SIZE;
> +	kexec_mark_range(crashk_res.start, control - 1, protect);
> +	kexec_mark_range(control + PAGE_SIZE, crashk_res.end, protect);

X86 kexec will copy the page while kexecing, could you check if we can move
that copying to earliyer while kexec loading, maybe machine_kexec_prepare so
that we can make a arch-independent implementation.

> +}
> +
> +void arch_kexec_protect_crashkres(void)
> +{
> +	kexec_mark_crashkres(true);
> +}
> +
> +void arch_kexec_unprotect_crashkres(void)
> +{
> +	kexec_mark_crashkres(false);
> +}
> +#endif
> -- 
> 2.5.0
> 
> 
> _______________________________________________
> kexec mailing list
> kexec@lists.infradead.org
> http://lists.infradead.org/mailman/listinfo/kexec
--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majordomo@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/

[toc] | [prev] | [next] | [standalone]

#1297765 — Re: [PATCH 2/2] kexec: Provide arch_kexec_protect(unprotect)_crashkres()

On 12/24/2015 at 01:54 PM, Dave Young wrote:
> Ccing Vivek
>
> On 12/23/15 at 07:12pm, Xunlei Pang wrote:
>> Implement the protection method for the crash kernel memory
>> reservation for the 64-bit x86 kdump.
>>
>> Signed-off-by: Xunlei Pang <xlpang@redhat.com>
>> ---
>> Only provided x86_64 implementation, as I've only tested on x86_64 so far.
>>
>>  arch/x86/kernel/machine_kexec_64.c | 43 ++++++++++++++++++++++++++++++++++++++
>>  1 file changed, 43 insertions(+)
>>
>> diff --git a/arch/x86/kernel/machine_kexec_64.c b/arch/x86/kernel/machine_kexec_64.c
>> index 819ab3f..a3d289c 100644
>> --- a/arch/x86/kernel/machine_kexec_64.c
>> +++ b/arch/x86/kernel/machine_kexec_64.c
>> @@ -536,3 +536,46 @@ overflow:
>>  	return -ENOEXEC;
>>  }
>>  #endif /* CONFIG_KEXEC_FILE */
>> +
>> +#ifdef CONFIG_KEXEC_CORE
> The file is only compiled when CONFIG_KEXEC_CORE=y so #ifdef is not necessary

Yes, indeed. I'll remove this macro and send v2 later.

>
>> +static int
>> +kexec_mark_range(unsigned long start, unsigned long end, bool protect)
>> +{
>> +	struct page *page;
>> +	unsigned int nr_pages;
>> +
>> +	if (!start || !end || start >= end)
>> +		return 0;
>> +
>> +	page = pfn_to_page(start >> PAGE_SHIFT);
>> +	nr_pages = (end + 1 - start) >> PAGE_SHIFT;
>> +	if (protect)
>> +		return set_pages_ro(page, nr_pages);
>> +	else
>> +		return set_pages_rw(page, nr_pages);
> May use set_memory_ro/rw to avoid converting to *page?

on x86 it just a wrapper of set_memory_ro/rw, I think both are ok.

>
>> +}
>> +
>> +static void kexec_mark_crashkres(bool protect)
>> +{
>> +	unsigned long control;
>> +
>> +	kexec_mark_range(crashk_low_res.start, crashk_low_res.end, protect);
>> +
>> +	/* Don't touch the control code page used in crash_kexec().*/
>> +	control = PFN_PHYS(page_to_pfn(kexec_crash_image->control_code_page));
>> +	/* Control code page is located in the 2nd page. */
>> +	control = control + PAGE_SIZE;
>> +	kexec_mark_range(crashk_res.start, control - 1, protect);
>> +	kexec_mark_range(control + PAGE_SIZE, crashk_res.end, protect);
> X86 kexec will copy the page while kexecing, could you check if we can move
> that copying to earliyer while kexec loading, maybe machine_kexec_prepare so
> that we can make a arch-independent implementation.

For some arch, may use huge tlb directly to do the kernel mapping,
in such cases, we can't implement this function. So I think it should
be arch-dependent.

Regards,
Xunlei

>
>> +}
>> +
>> +void arch_kexec_protect_crashkres(void)
>> +{
>> +	kexec_mark_crashkres(true);
>> +}
>> +
>> +void arch_kexec_unprotect_crashkres(void)
>> +{
>> +	kexec_mark_crashkres(false);
>> +}
>> +#endif
>> -- 
>> 2.5.0
>>
>>
>> _______________________________________________
>> kexec mailing list
>> kexec@lists.infradead.org
>> http://lists.infradead.org/mailman/listinfo/kexec

--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majordomo@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/

[toc] | [prev] | [next] | [standalone]

#1297766 — Re: [PATCH 2/2] kexec: Provide arch_kexec_protect(unprotect)_crashkres()

Hi, Xunlei

On 12/24/15 at 02:05pm, Xunlei Pang wrote:
> On 12/24/2015 at 01:54 PM, Dave Young wrote:
> > Ccing Vivek
> >
> > On 12/23/15 at 07:12pm, Xunlei Pang wrote:
> >> Implement the protection method for the crash kernel memory
> >> reservation for the 64-bit x86 kdump.
> >>
> >> Signed-off-by: Xunlei Pang <xlpang@redhat.com>
> >> ---
> >> Only provided x86_64 implementation, as I've only tested on x86_64 so far.
> >>
> >>  arch/x86/kernel/machine_kexec_64.c | 43 ++++++++++++++++++++++++++++++++++++++
> >>  1 file changed, 43 insertions(+)
> >>
> >> diff --git a/arch/x86/kernel/machine_kexec_64.c b/arch/x86/kernel/machine_kexec_64.c
> >> index 819ab3f..a3d289c 100644
> >> --- a/arch/x86/kernel/machine_kexec_64.c
> >> +++ b/arch/x86/kernel/machine_kexec_64.c
> >> @@ -536,3 +536,46 @@ overflow:
> >>  	return -ENOEXEC;
> >>  }
> >>  #endif /* CONFIG_KEXEC_FILE */
> >> +
> >> +#ifdef CONFIG_KEXEC_CORE
> > The file is only compiled when CONFIG_KEXEC_CORE=y so #ifdef is not necessary
> 
> Yes, indeed. I'll remove this macro and send v2 later.
> 
> >
> >> +static int
> >> +kexec_mark_range(unsigned long start, unsigned long end, bool protect)
> >> +{
> >> +	struct page *page;
> >> +	unsigned int nr_pages;
> >> +
> >> +	if (!start || !end || start >= end)
> >> +		return 0;
> >> +
> >> +	page = pfn_to_page(start >> PAGE_SHIFT);
> >> +	nr_pages = (end + 1 - start) >> PAGE_SHIFT;
> >> +	if (protect)
> >> +		return set_pages_ro(page, nr_pages);
> >> +	else
> >> +		return set_pages_rw(page, nr_pages);
> > May use set_memory_ro/rw to avoid converting to *page?
> 
> on x86 it just a wrapper of set_memory_ro/rw, I think both are ok.

Ok, I have no strong opinion on that..

> 
> >
> >> +}
> >> +
> >> +static void kexec_mark_crashkres(bool protect)
> >> +{
> >> +	unsigned long control;
> >> +
> >> +	kexec_mark_range(crashk_low_res.start, crashk_low_res.end, protect);
> >> +
> >> +	/* Don't touch the control code page used in crash_kexec().*/
> >> +	control = PFN_PHYS(page_to_pfn(kexec_crash_image->control_code_page));
> >> +	/* Control code page is located in the 2nd page. */
> >> +	control = control + PAGE_SIZE;

Though it works because the control code is less than 1 page, but use the macro
of KEXEC_CONTROL_PAGE_SIZE looks better..

> >> +	kexec_mark_range(crashk_res.start, control - 1, protect);
> >> +	kexec_mark_range(control + PAGE_SIZE, crashk_res.end, protect);
> > X86 kexec will copy the page while kexecing, could you check if we can move
> > that copying to earliyer while kexec loading, maybe machine_kexec_prepare so
> > that we can make a arch-independent implementation.
> 
> For some arch, may use huge tlb directly to do the kernel mapping,
> in such cases, we can't implement this function. So I think it should
> be arch-dependent.

Ok, that's fine.

> 
> Regards,
> Xunlei
> 
> >
> >> +}
> >> +
> >> +void arch_kexec_protect_crashkres(void)
> >> +{
> >> +	kexec_mark_crashkres(true);
> >> +}
> >> +
> >> +void arch_kexec_unprotect_crashkres(void)
> >> +{
> >> +	kexec_mark_crashkres(false);
> >> +}
> >> +#endif
> >> -- 
> >> 2.5.0
> >>
> >>
> >> _______________________________________________
> >> 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

Thanks
Dave
--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majordomo@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/

[toc] | [prev] | [next] | [standalone]

#1297770 — Re: [PATCH 2/2] kexec: Provide arch_kexec_protect(unprotect)_crashkres()

On 12/24/2015 at 02:16 PM, Dave Young wrote:
> Hi, Xunlei
>
> On 12/24/15 at 02:05pm, Xunlei Pang wrote:
>> On 12/24/2015 at 01:54 PM, Dave Young wrote:
>>> Ccing Vivek
>>>
>>> On 12/23/15 at 07:12pm, Xunlei Pang wrote:
>>>> Implement the protection method for the crash kernel memory
>>>> reservation for the 64-bit x86 kdump.
>>>>
>>>> Signed-off-by: Xunlei Pang <xlpang@redhat.com>
>>>> ---
>>>> Only provided x86_64 implementation, as I've only tested on x86_64 so far.
>>>>
>>>>  arch/x86/kernel/machine_kexec_64.c | 43 ++++++++++++++++++++++++++++++++++++++
>>>>  1 file changed, 43 insertions(+)
>>>>
>>>> diff --git a/arch/x86/kernel/machine_kexec_64.c b/arch/x86/kernel/machine_kexec_64.c
>>>> index 819ab3f..a3d289c 100644
>>>> --- a/arch/x86/kernel/machine_kexec_64.c
>>>> +++ b/arch/x86/kernel/machine_kexec_64.c
>>>> @@ -536,3 +536,46 @@ overflow:
>>>>  	return -ENOEXEC;
>>>>  }
>>>>  #endif /* CONFIG_KEXEC_FILE */
>>>> +
>>>> +#ifdef CONFIG_KEXEC_CORE
>>> The file is only compiled when CONFIG_KEXEC_CORE=y so #ifdef is not necessary
>> Yes, indeed. I'll remove this macro and send v2 later.
>>
>>>> +static int
>>>> +kexec_mark_range(unsigned long start, unsigned long end, bool protect)
>>>> +{
>>>> +	struct page *page;
>>>> +	unsigned int nr_pages;
>>>> +
>>>> +	if (!start || !end || start >= end)
>>>> +		return 0;
>>>> +
>>>> +	page = pfn_to_page(start >> PAGE_SHIFT);
>>>> +	nr_pages = (end + 1 - start) >> PAGE_SHIFT;
>>>> +	if (protect)
>>>> +		return set_pages_ro(page, nr_pages);
>>>> +	else
>>>> +		return set_pages_rw(page, nr_pages);
>>> May use set_memory_ro/rw to avoid converting to *page?
>> on x86 it just a wrapper of set_memory_ro/rw, I think both are ok.
> Ok, I have no strong opinion on that..
>
>>>> +}
>>>> +
>>>> +static void kexec_mark_crashkres(bool protect)
>>>> +{
>>>> +	unsigned long control;
>>>> +
>>>> +	kexec_mark_range(crashk_low_res.start, crashk_low_res.end, protect);
>>>> +
>>>> +	/* Don't touch the control code page used in crash_kexec().*/
>>>> +	control = PFN_PHYS(page_to_pfn(kexec_crash_image->control_code_page));
>>>> +	/* Control code page is located in the 2nd page. */
>>>> +	control = control + PAGE_SIZE;
> Though it works because the control code is less than 1 page, but use the macro
> of KEXEC_CONTROL_PAGE_SIZE looks better..
>
>>>> +	kexec_mark_range(crashk_res.start, control - 1, protect);
>>>> +	kexec_mark_range(control + PAGE_SIZE, crashk_res.end, protect);
>>> X86 kexec will copy the page while kexecing, could you check if we can move
>>> that copying to earliyer while kexec loading, maybe machine_kexec_prepare so
>>> that we can make a arch-independent implementation.
>> For some arch, may use huge tlb directly to do the kernel mapping,
>> in such cases, we can't implement this function. So I think it should
>> be arch-dependent.
> Ok, that's fine.

At least moving the x86 control-copying code into arch-related
machine_kexec_prepare() should work, and this can omit the
special treatment of the control code page.

Regards,
Xunlei

>
>> Regards,
>> Xunlei
>>
>>>> +}
>>>> +
>>>> +void arch_kexec_protect_crashkres(void)
>>>> +{
>>>> +	kexec_mark_crashkres(true);
>>>> +}
>>>> +
>>>> +void arch_kexec_unprotect_crashkres(void)
>>>> +{
>>>> +	kexec_mark_crashkres(false);
>>>> +}
>>>> +#endif
>>>> -- 
>>>> 2.5.0
>>>>
>>>>
>>>> _______________________________________________
>>>> 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
> Thanks
> Dave
>
> _______________________________________________
> kexec mailing list
> kexec@lists.infradead.org
> http://lists.infradead.org/mailman/listinfo/kexec

--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majordomo@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/

[toc] | [prev] | [next] | [standalone]

#1298521 — Re: [PATCH 2/2] kexec: Provide arch_kexec_protect(unprotect)_crashkres()

On 12/24/2015 at 02:44 PM, Xunlei Pang wrote:
> On 12/24/2015 at 02:16 PM, Dave Young wrote:
>> Hi, Xunlei
>>
>> On 12/24/15 at 02:05pm, Xunlei Pang wrote:
>>> On 12/24/2015 at 01:54 PM, Dave Young wrote:
>>>> Ccing Vivek
>>>>
>>>> On 12/23/15 at 07:12pm, Xunlei Pang wrote:
>>>>> Implement the protection method for the crash kernel memory
>>>>> reservation for the 64-bit x86 kdump.
>>>>>
>>>>> Signed-off-by: Xunlei Pang <xlpang@redhat.com>
>>>>> ---
>>>>> Only provided x86_64 implementation, as I've only tested on x86_64 so far.
>>>>>
>>>>>  arch/x86/kernel/machine_kexec_64.c | 43 ++++++++++++++++++++++++++++++++++++++
>>>>>  1 file changed, 43 insertions(+)
>>>>>
>>>>> diff --git a/arch/x86/kernel/machine_kexec_64.c b/arch/x86/kernel/machine_kexec_64.c
>>>>> index 819ab3f..a3d289c 100644
>>>>> --- a/arch/x86/kernel/machine_kexec_64.c
>>>>> +++ b/arch/x86/kernel/machine_kexec_64.c
>>>>> @@ -536,3 +536,46 @@ overflow:
>>>>>  	return -ENOEXEC;
>>>>>  }
>>>>>  #endif /* CONFIG_KEXEC_FILE */
>>>>> +
>>>>> +#ifdef CONFIG_KEXEC_CORE
>>>> The file is only compiled when CONFIG_KEXEC_CORE=y so #ifdef is not necessary
>>> Yes, indeed. I'll remove this macro and send v2 later.
>>>
>>>>> +static int
>>>>> +kexec_mark_range(unsigned long start, unsigned long end, bool protect)
>>>>> +{
>>>>> +	struct page *page;
>>>>> +	unsigned int nr_pages;
>>>>> +
>>>>> +	if (!start || !end || start >= end)
>>>>> +		return 0;
>>>>> +
>>>>> +	page = pfn_to_page(start >> PAGE_SHIFT);
>>>>> +	nr_pages = (end + 1 - start) >> PAGE_SHIFT;
>>>>> +	if (protect)
>>>>> +		return set_pages_ro(page, nr_pages);
>>>>> +	else
>>>>> +		return set_pages_rw(page, nr_pages);
>>>> May use set_memory_ro/rw to avoid converting to *page?
>>> on x86 it just a wrapper of set_memory_ro/rw, I think both are ok.
>> Ok, I have no strong opinion on that..
>>
>>>>> +}
>>>>> +
>>>>> +static void kexec_mark_crashkres(bool protect)
>>>>> +{
>>>>> +	unsigned long control;
>>>>> +
>>>>> +	kexec_mark_range(crashk_low_res.start, crashk_low_res.end, protect);
>>>>> +
>>>>> +	/* Don't touch the control code page used in crash_kexec().*/
>>>>> +	control = PFN_PHYS(page_to_pfn(kexec_crash_image->control_code_page));
>>>>> +	/* Control code page is located in the 2nd page. */
>>>>> +	control = control + PAGE_SIZE;
>> Though it works because the control code is less than 1 page, but use the macro
>> of KEXEC_CONTROL_PAGE_SIZE looks better..

The 1st page is pagetable, control code page locates at the 2nd page.
The following kexec_mark_range() wants to mark ro from crashk_res.start
to the 1st page(included), so here we must use PAGE_SIZE.

>>
>>>>> +	kexec_mark_range(crashk_res.start, control - 1, protect);
>>>>> +	kexec_mark_range(control + PAGE_SIZE, crashk_res.end, protect);
>>>> X86 kexec will copy the page while kexecing, could you check if we can move
>>>> that copying to earliyer while kexec loading, maybe machine_kexec_prepare so
>>>> that we can make a arch-independent implementation.
>>> For some arch, may use huge tlb directly to do the kernel mapping,
>>> in such cases, we can't implement this function. So I think it should
>>> be arch-dependent.
>> Ok, that's fine.
> At least moving the x86 control-copying code into arch-related
> machine_kexec_prepare() should work, and this can omit the
> special treatment of the control code page.

The "relocate_kernel" routine in "relocate_kernel_64.S" will use it as
a temp storage "for jumping back"(as its comment), so we can't mark
it readonly.

>
> Regards,
> Xunlei
>
>>> Regards,
>>> Xunlei
>>>
>>>>> +}
>>>>> +
>>>>> +void arch_kexec_protect_crashkres(void)
>>>>> +{
>>>>> +	kexec_mark_crashkres(true);
>>>>> +}
>>>>> +
>>>>> +void arch_kexec_unprotect_crashkres(void)
>>>>> +{
>>>>> +	kexec_mark_crashkres(false);
>>>>> +}
>>>>> +#endif
>>>>> -- 
>>>>> 2.5.0
>>>>>
>>>>>
>>>>> _______________________________________________
>>>>> 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
>> Thanks
>> Dave
>>
>> _______________________________________________
>> kexec mailing list
>> kexec@lists.infradead.org
>> http://lists.infradead.org/mailman/listinfo/kexec

--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majordomo@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/

[toc] | [prev] | [next] | [standalone]

#1298620 — Re: [PATCH 2/2] kexec: Provide arch_kexec_protect(unprotect)_crashkres()

On 12/28/15 at 02:32pm, Xunlei Pang wrote:
> On 12/24/2015 at 02:44 PM, Xunlei Pang wrote:
> >>>>> +static void kexec_mark_crashkres(bool protect)
> >>>>> +{
> >>>>> +	unsigned long control;
> >>>>> +
> >>>>> +	kexec_mark_range(crashk_low_res.start, crashk_low_res.end, protect);
> >>>>> +
> >>>>> +	/* Don't touch the control code page used in crash_kexec().*/
> >>>>> +	control = PFN_PHYS(page_to_pfn(kexec_crash_image->control_code_page));
> >>>>> +	/* Control code page is located in the 2nd page. */
> >>>>> +	control = control + PAGE_SIZE;
> >> Though it works because the control code is less than 1 page, but use the macro
> >> of KEXEC_CONTROL_PAGE_SIZE looks better..
> 
> The 1st page is pagetable, control code page locates at the 2nd page.
> The following kexec_mark_range() wants to mark ro from crashk_res.start
> to the 1st page(included), so here we must use PAGE_SIZE.
> 
> >>
> >>>>> +	kexec_mark_range(crashk_res.start, control - 1, protect);
> >>>>> +	kexec_mark_range(control + PAGE_SIZE, crashk_res.end, protect);
> >>>> X86 kexec will copy the page while kexecing, could you check if we can move
> >>>> that copying to earliyer while kexec loading, maybe machine_kexec_prepare so
> >>>> that we can make a arch-independent implementation.
> >>> For some arch, may use huge tlb directly to do the kernel mapping,
> >>> in such cases, we can't implement this function. So I think it should
> >>> be arch-dependent.
> >> Ok, that's fine.
> > At least moving the x86 control-copying code into arch-related
> > machine_kexec_prepare() should work, and this can omit the
> > special treatment of the control code page.
> 
> The "relocate_kernel" routine in "relocate_kernel_64.S" will use it as
> a temp storage "for jumping back"(as its comment), so we can't mark
> it readonly.

kexec will copy the relocate_kernel binary to control_code_page in
function machine_kexec. This is a major reason to set the region
control_code_page to control_code_page + PAGE_SIZE with mode read/write.

Thanks
Minfei
--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majordomo@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/

[toc] | [prev] | [next] | [standalone]

#1298622 — Re: [PATCH 2/2] kexec: Provide arch_kexec_protect(unprotect)_crashkres()

On 12/28/15 at 08:14pm, Minfei Huang wrote:
> On 12/28/15 at 02:32pm, Xunlei Pang wrote:
> > On 12/24/2015 at 02:44 PM, Xunlei Pang wrote:
> > >>>>> +static void kexec_mark_crashkres(bool protect)
> > >>>>> +{
> > >>>>> +	unsigned long control;
> > >>>>> +
> > >>>>> +	kexec_mark_range(crashk_low_res.start, crashk_low_res.end, protect);
> > >>>>> +
> > >>>>> +	/* Don't touch the control code page used in crash_kexec().*/
> > >>>>> +	control = PFN_PHYS(page_to_pfn(kexec_crash_image->control_code_page));
> > >>>>> +	/* Control code page is located in the 2nd page. */
> > >>>>> +	control = control + PAGE_SIZE;
> > >> Though it works because the control code is less than 1 page, but use the macro
> > >> of KEXEC_CONTROL_PAGE_SIZE looks better..
> > 
> > The 1st page is pagetable, control code page locates at the 2nd page.
> > The following kexec_mark_range() wants to mark ro from crashk_res.start
> > to the 1st page(included), so here we must use PAGE_SIZE.
> > 
> > >>
> > >>>>> +	kexec_mark_range(crashk_res.start, control - 1, protect);
> > >>>>> +	kexec_mark_range(control + PAGE_SIZE, crashk_res.end, protect);
> > >>>> X86 kexec will copy the page while kexecing, could you check if we can move
> > >>>> that copying to earliyer while kexec loading, maybe machine_kexec_prepare so
> > >>>> that we can make a arch-independent implementation.
> > >>> For some arch, may use huge tlb directly to do the kernel mapping,
> > >>> in such cases, we can't implement this function. So I think it should
> > >>> be arch-dependent.
> > >> Ok, that's fine.
> > > At least moving the x86 control-copying code into arch-related
> > > machine_kexec_prepare() should work, and this can omit the
> > > special treatment of the control code page.
> > 
> > The "relocate_kernel" routine in "relocate_kernel_64.S" will use it as
> > a temp storage "for jumping back"(as its comment), so we can't mark
> > it readonly.
> 
> kexec will copy the relocate_kernel binary to control_code_page in
> function machine_kexec. This is a major reason to set the region
> control_code_page to control_code_page + PAGE_SIZE with mode read/write.

Fix it.
The region is from control_code_page + PAGE_SIZE to
control_code_page + PAGE_SIZE + PAGE_SIZE.

Thanks
Minfei
--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majordomo@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/

[toc] | [prev] | [next] | [standalone]

#1298953 — Re: [PATCH 2/2] kexec: Provide arch_kexec_protect(unprotect)_crashkres()

On 12/28/2015 at 08:14 PM, Minfei Huang wrote:
> On 12/28/15 at 02:32pm, Xunlei Pang wrote:
>> On 12/24/2015 at 02:44 PM, Xunlei Pang wrote:
>>>>>>> +static void kexec_mark_crashkres(bool protect)
>>>>>>> +{
>>>>>>> +	unsigned long control;
>>>>>>> +
>>>>>>> +	kexec_mark_range(crashk_low_res.start, crashk_low_res.end, protect);
>>>>>>> +
>>>>>>> +	/* Don't touch the control code page used in crash_kexec().*/
>>>>>>> +	control = PFN_PHYS(page_to_pfn(kexec_crash_image->control_code_page));
>>>>>>> +	/* Control code page is located in the 2nd page. */
>>>>>>> +	control = control + PAGE_SIZE;
>>>> Though it works because the control code is less than 1 page, but use the macro
>>>> of KEXEC_CONTROL_PAGE_SIZE looks better..
>> The 1st page is pagetable, control code page locates at the 2nd page.
>> The following kexec_mark_range() wants to mark ro from crashk_res.start
>> to the 1st page(included), so here we must use PAGE_SIZE.
>>
>>>>>>> +	kexec_mark_range(crashk_res.start, control - 1, protect);
>>>>>>> +	kexec_mark_range(control + PAGE_SIZE, crashk_res.end, protect);
>>>>>> X86 kexec will copy the page while kexecing, could you check if we can move
>>>>>> that copying to earliyer while kexec loading, maybe machine_kexec_prepare so
>>>>>> that we can make a arch-independent implementation.
>>>>> For some arch, may use huge tlb directly to do the kernel mapping,
>>>>> in such cases, we can't implement this function. So I think it should
>>>>> be arch-dependent.
>>>> Ok, that's fine.
>>> At least moving the x86 control-copying code into arch-related
>>> machine_kexec_prepare() should work, and this can omit the
>>> special treatment of the control code page.
>> The "relocate_kernel" routine in "relocate_kernel_64.S" will use it as
>> a temp storage "for jumping back"(as its comment), so we can't mark
>> it readonly.
> kexec will copy the relocate_kernel binary to control_code_page in
> function machine_kexec. This is a major reason to set the region
> control_code_page to control_code_page + PAGE_SIZE with mode read/write.

Yes, I mean after avoiding this copy by  moving the x86 control-copying
code into arch-related machine_kexec_prepare(), we still can't mark it
readonly because of its temp storage role.

Of course we can still do that by setting its kernel pte to rwx and do a
local tlb invalidation when crash_kexec(), but I think we can simply leave
that alone, since its content is meaningless before crash happens where
the code is copied into it. Unless you want to capture the wrong kernel
operations that only write to this reserved page.

Regards,
Xunlei

>
> Thanks
> Minfei
>
> _______________________________________________
> kexec mailing list
> kexec@lists.infradead.org
> http://lists.infradead.org/mailman/listinfo/kexec

--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majordomo@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/

[toc] | [prev] | [next] | [standalone]

#1298216 — Re: [PATCH 2/2] kexec: Provide arch_kexec_protect(unprotect)_crashkres()

On 12/23/15 at 07:12pm, Xunlei Pang wrote:
> Implement the protection method for the crash kernel memory
> reservation for the 64-bit x86 kdump.
> 
> Signed-off-by: Xunlei Pang <xlpang@redhat.com>
> ---
> Only provided x86_64 implementation, as I've only tested on x86_64 so far.
> 
>  arch/x86/kernel/machine_kexec_64.c | 43 ++++++++++++++++++++++++++++++++++++++
>  1 file changed, 43 insertions(+)
> 
> diff --git a/arch/x86/kernel/machine_kexec_64.c b/arch/x86/kernel/machine_kexec_64.c
> index 819ab3f..a3d289c 100644
> --- a/arch/x86/kernel/machine_kexec_64.c
> +++ b/arch/x86/kernel/machine_kexec_64.c
> @@ -536,3 +536,46 @@ overflow:
>  	return -ENOEXEC;
>  }
>  #endif /* CONFIG_KEXEC_FILE */
> +
> +#ifdef CONFIG_KEXEC_CORE
> +static int
> +kexec_mark_range(unsigned long start, unsigned long end, bool protect)
> +{
> +	struct page *page;
> +	unsigned int nr_pages;
> +
> +	if (!start || !end || start >= end)
> +		return 0;
> +
> +	page = pfn_to_page(start >> PAGE_SHIFT);
> +	nr_pages = (end + 1 - start) >> PAGE_SHIFT;

The start and end may across two pages, although the range is small than
PAGE_SIZE. You can use following to calculate count of page.

nr_pages = (end >> PAGE_SHIFT) - (start >> PAGE_SHIFT) + 1;

Thanks
Minfei

> +	if (protect)
> +		return set_pages_ro(page, nr_pages);
> +	else
> +		return set_pages_rw(page, nr_pages);
> +}
--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majordomo@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/

[toc] | [prev] | [standalone]

Back to top | Article view | linux.kernel

csiph-web