Messages in this thread | | | Subject | Re: [Fastboot] [PATCH] kexec: Avoid overwriting the current pgd (i386) | From | (Eric W. Biederman) | Date | Mon, 01 May 2006 10:37:26 -0600 |
| |
Vivek Goyal <vgoyal@in.ibm.com> writes:
> On Mon, May 01, 2006 at 06:49:16PM +0900, Magnus Damm wrote: >> kexec: Avoid overwriting the current pgd (i386) >> >> This patch upgrades the i386-specific kexec code to avoid overwriting the >> current pgd. Overwriting the current pgd is bad when CONFIG_CRASH_DUMP is used >> to start a secondary kernel that dumps the memory of the previous kernel. >> >> The code introduces a new set of page tables called "page_table_a". These >> tables are used to provide an executable identity mapping without overwriting >> the current pgd. > > True, current pgd is overwritten but that effects only user space mappings > and currently "crash" supports only backtracing kernel space code. But at > the same time probably it is not a bad idea to maintain a separate page > table and switch to that instead of overwriting the existing pgd. This > shall help if in future user space backtracing is also supported. > > [..] >> >> +static int allocate_page_table_a(struct kimage *image) >> +{ >> + struct page *page; >> + int k = sizeof(image->page_table_a) / sizeof(image->page_table_a[0]); >> + >> + for (; k > 0; k--) { >> + page = kimage_alloc_control_pages(image, 0); >> + if (!page) >> + return -ENOMEM; >> + >> + clear_page(page_address(page)); >> + image->page_table_a[k - 1] = page; > > I think you also need to write the logic to free those pages if somebody > chooses to unload the pre-loaded kernel.
Because these are control pages we already keep track of them, so we can free them along with everything else.
> [..] >> --- 0001/include/linux/kexec.h >> +++ work/include/linux/kexec.h 2006-05-01 11:13:14.000000000 +0900 >> @@ -69,6 +69,17 @@ struct kimage { >> unsigned long start; >> struct page *control_code_page; >> >> + /* page_table_a[] holds enough pages to create a new page table >> + * that maps the control page twice.. >> + */ >> + >> +#if defined(CONFIG_X86_32) && !defined(CONFIG_X86_PAE) >> + struct page *page_table_a[3]; /* (2 * pte) + pgd */ >> +#endif >> +#if defined(CONFIG_X86_32) && defined(CONFIG_X86_PAE) >> + struct page *page_table_a[5]; /* (2 * pte) + (2 * pmd) + pgd */ >> +#endif >> + > > Would it make a cleaner interface if these array of pointers are maintained > in arch dependent code as global variables instead of putting in arch > independent code. Existing code does something simlar. This becomes further > ugly when another array comes into the picture for x86_64 in next patch. > (page_table_b)
Well global variables don't quite work in the normal case.
However it probably makes most sense to maintain the needed variables in a structure on the control page. Which will keep them out of harms way, and won't require patches to the generic code.
Eric - 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/
| |