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

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

[Xunlei Pang]

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 undo 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>
---
v2->v3:
No changes.

 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..5892d6a 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_FILE_ON_CRASH) && kexec_crash_image)
+		arch_kexec_protect_crashkres();
+
 	mutex_unlock(&kexec_mutex);
 	kimage_free(image);
 	return ret;
-- 
2.5.0

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

[Xunlei Pang]

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 undo 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>
---
v2->v3:
No changes.

 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..5892d6a 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_FILE_ON_CRASH) && kexec_crash_image)
+		arch_kexec_protect_crashkres();
+
 	mutex_unlock(&kexec_mutex);
 	kimage_free(image);
 	return ret;
-- 
2.5.0


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

[PATCH v3 2/2] kexec: Provide arch_kexec_protect(unprotect)_crashkres()

[Xunlei Pang]

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

Signed-off-by: Xunlei Pang <xlpang@redhat.com>
---
v2->v3:
1) Make kexec_mark_range() more robust.
2) In kexec_mark_crashkres(), use Macro KEXEC_CONTROL_PAGE_SIZE 
   instead of 2*PAGE_SIZE

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

diff --git a/arch/x86/kernel/machine_kexec_64.c b/arch/x86/kernel/machine_kexec_64.c
index 819ab3f..8127dc0 100644
--- a/arch/x86/kernel/machine_kexec_64.c
+++ b/arch/x86/kernel/machine_kexec_64.c
@@ -536,3 +536,45 @@ overflow:
 	return -ENOEXEC;
 }
 #endif /* CONFIG_KEXEC_FILE */
+
+static int
+kexec_mark_range(unsigned long start, unsigned long end, bool protect)
+{
+	struct page *page;
+	unsigned int nr_pages;
+
+	/* For physical range: [start, end] */
+	if (!end || start > end)
+		return 0;
+
+	page = pfn_to_page(start >> PAGE_SHIFT);
+	nr_pages = (end >> PAGE_SHIFT) - (start >> PAGE_SHIFT) + 1;
+	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. */
+	kexec_mark_range(crashk_res.start, control + PAGE_SIZE - 1, protect);
+	control += KEXEC_CONTROL_PAGE_SIZE;
+	kexec_mark_range(control, crashk_res.end, protect);
+}
+
+void arch_kexec_protect_crashkres(void)
+{
+	kexec_mark_crashkres(true);
+}
+
+void arch_kexec_unprotect_crashkres(void)
+{
+	kexec_mark_crashkres(false);
+}
-- 
2.5.0

[PATCH v3 2/2] kexec: Provide arch_kexec_protect(unprotect)_crashkres()

[Xunlei Pang]

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

Signed-off-by: Xunlei Pang <xlpang@redhat.com>
---
v2->v3:
1) Make kexec_mark_range() more robust.
2) In kexec_mark_crashkres(), use Macro KEXEC_CONTROL_PAGE_SIZE 
   instead of 2*PAGE_SIZE

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

diff --git a/arch/x86/kernel/machine_kexec_64.c b/arch/x86/kernel/machine_kexec_64.c
index 819ab3f..8127dc0 100644
--- a/arch/x86/kernel/machine_kexec_64.c
+++ b/arch/x86/kernel/machine_kexec_64.c
@@ -536,3 +536,45 @@ overflow:
 	return -ENOEXEC;
 }
 #endif /* CONFIG_KEXEC_FILE */
+
+static int
+kexec_mark_range(unsigned long start, unsigned long end, bool protect)
+{
+	struct page *page;
+	unsigned int nr_pages;
+
+	/* For physical range: [start, end] */
+	if (!end || start > end)
+		return 0;
+
+	page = pfn_to_page(start >> PAGE_SHIFT);
+	nr_pages = (end >> PAGE_SHIFT) - (start >> PAGE_SHIFT) + 1;
+	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. */
+	kexec_mark_range(crashk_res.start, control + PAGE_SIZE - 1, protect);
+	control += KEXEC_CONTROL_PAGE_SIZE;
+	kexec_mark_range(control, crashk_res.end, protect);
+}
+
+void arch_kexec_protect_crashkres(void)
+{
+	kexec_mark_crashkres(true);
+}
+
+void arch_kexec_unprotect_crashkres(void)
+{
+	kexec_mark_crashkres(false);
+}
-- 
2.5.0


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

Re: [PATCH v3 2/2] kexec: Provide arch_kexec_protect(unprotect)_crashkres()

[Minfei Huang]

On 01/08/16 at 10:33am, Xunlei Pang wrote:
> +
> +static int
> +kexec_mark_range(unsigned long start, unsigned long end, bool protect)
> +{
> +	struct page *page;
> +	unsigned int nr_pages;
> +
> +	/* For physical range: [start, end] */
> +	if (!end || start > end)
> +		return 0;

This test !end is hard to be understood without the annotation. It is
better to add the comment about it.

Otherwise it looks good for me.

Thanks
Minfei

Re: [PATCH v3 2/2] kexec: Provide arch_kexec_protect(unprotect)_crashkres()

[Minfei Huang]

On 01/08/16 at 10:33am, Xunlei Pang wrote:
> +
> +static int
> +kexec_mark_range(unsigned long start, unsigned long end, bool protect)
> +{
> +	struct page *page;
> +	unsigned int nr_pages;
> +
> +	/* For physical range: [start, end] */
> +	if (!end || start > end)
> +		return 0;

This test !end is hard to be understood without the annotation. It is
better to add the comment about it.

Otherwise it looks good for me.

Thanks
Minfei

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

Re: [PATCH v3 2/2] kexec: Provide arch_kexec_protect(unprotect)_crashkres()

[Xunlei Pang]

On 01/08/2016 at 04:47 PM, Minfei Huang wrote:
> On 01/08/16 at 10:33am, Xunlei Pang wrote:
>> +
>> +static int
>> +kexec_mark_range(unsigned long start, unsigned long end, bool protect)
>> +{
>> +	struct page *page;
>> +	unsigned int nr_pages;
>> +
>> +	/* For physical range: [start, end] */
>> +	if (!end || start > end)
>> +		return 0;
> This test !end is hard to be understood without the annotation. It is
> better to add the comment about it.

!end is just for uninitialized crashk resource with a zero end member,
maybe not so hard to understand :-)

Regards,
Xunlei

>
> Otherwise it looks good for me.
>
> Thanks
> Minfei
>
> _______________________________________________
> kexec mailing list
> kexec@lists.infradead.org
> http://lists.infradead.org/mailman/listinfo/kexec

Re: [PATCH v3 2/2] kexec: Provide arch_kexec_protect(unprotect)_crashkres()

[Xunlei Pang]

On 01/08/2016 at 04:47 PM, Minfei Huang wrote:
> On 01/08/16 at 10:33am, Xunlei Pang wrote:
>> +
>> +static int
>> +kexec_mark_range(unsigned long start, unsigned long end, bool protect)
>> +{
>> +	struct page *page;
>> +	unsigned int nr_pages;
>> +
>> +	/* For physical range: [start, end] */
>> +	if (!end || start > end)
>> +		return 0;
> This test !end is hard to be understood without the annotation. It is
> better to add the comment about it.

!end is just for uninitialized crashk resource with a zero end member,
maybe not so hard to understand :-)

Regards,
Xunlei

>
> Otherwise it looks good for me.
>
> Thanks
> Minfei
>
> _______________________________________________
> 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

Re: [PATCH v3 2/2] kexec: Provide arch_kexec_protect(unprotect)_crashkres()

[Minfei Huang]

On 01/08/16 at 09:28pm, Xunlei Pang wrote:
> On 01/08/2016 at 04:47 PM, Minfei Huang wrote:
> > On 01/08/16 at 10:33am, Xunlei Pang wrote:
> >> +
> >> +static int
> >> +kexec_mark_range(unsigned long start, unsigned long end, bool protect)
> >> +{
> >> +	struct page *page;
> >> +	unsigned int nr_pages;
> >> +
> >> +	/* For physical range: [start, end] */
> >> +	if (!end || start > end)
> >> +		return 0;
> > This test !end is hard to be understood without the annotation. It is
> > better to add the comment about it.
> 
> !end is just for uninitialized crashk resource with a zero end member,
> maybe not so hard to understand :-)

Yes, The test !end may be simple, if someone knows the logic of kexec.
But it's hard to be understood, if there is only a glance from someone
without any annotation.

This test !end is related to the specific code flow, so it is better to
annotate it.

Thanks
Minfei

> 
> Regards,
> Xunlei
> 
> >
> > Otherwise it looks good for me.

Re: [PATCH v3 2/2] kexec: Provide arch_kexec_protect(unprotect)_crashkres()

[Minfei Huang]

On 01/08/16 at 09:28pm, Xunlei Pang wrote:
> On 01/08/2016 at 04:47 PM, Minfei Huang wrote:
> > On 01/08/16 at 10:33am, Xunlei Pang wrote:
> >> +
> >> +static int
> >> +kexec_mark_range(unsigned long start, unsigned long end, bool protect)
> >> +{
> >> +	struct page *page;
> >> +	unsigned int nr_pages;
> >> +
> >> +	/* For physical range: [start, end] */
> >> +	if (!end || start > end)
> >> +		return 0;
> > This test !end is hard to be understood without the annotation. It is
> > better to add the comment about it.
> 
> !end is just for uninitialized crashk resource with a zero end member,
> maybe not so hard to understand :-)

Yes, The test !end may be simple, if someone knows the logic of kexec.
But it's hard to be understood, if there is only a glance from someone
without any annotation.

This test !end is related to the specific code flow, so it is better to
annotate it.

Thanks
Minfei

> 
> Regards,
> Xunlei
> 
> >
> > Otherwise it looks good for me.

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