On Monday 29 September 2008 14:39:55 Matias Zabaljauregui wrote:
> This patch moves the initial guest page table creation code to the host,
> so the launcher keeps working with PAE enabled configs.
OK, applied with some fixes.
> * the base of Guest "physical" memory, the top physical page to allow, the
> * top level pagetable and the entry point for the Guest. */
Fixed this comment, and similarly the one from the header.
> +static unsigned long setup_pagetables(unsigned long mem,
> + unsigned long initrd_size,
> + unsigned long mem_base)
I found it clearer to pass in lg here, and use lg->mem_base in this function.
> +{
> + pgd_t *pgdir;
> + pte_t *linear;
These are actually userspace pointers, so changed to:
+ pgd_t __user *pgdir;
+ pte_t __user *linear;
> + /* Linear mapping is easy: put every page's address into the
> + * mapping in order. */
> + for (i = 0; i < mapped_pages; i++)
> + set_pte(&linear[i], pfn_pte(i,
> + __pgprot(_PAGE_PRESENT | _PAGE_RW | _PAGE_USER)));
This direct access to userspace mem is a no-no. Here's what I have now:
+ for (i = 0; i < mapped_pages; i++) {
+ pte_t pte;
+ pte = pfn_pte(i, __pgprot(_PAGE_PRESENT|_PAGE_RW|_PAGE_USER));
+ if (copy_to_user(&linear[i], &pte, sizeof(pte)) != 0)
+ return -EFAULT;
+ }
> + /* The top level points to the linear page table pages above.
> + * We setup the identity and linear mappings here. */
> + phys_linear = (unsigned long)linear - mem_base;
> + for (i = 0; i < mapped_pages; i += PTRS_PER_PTE)
> + pgdir[i / PTRS_PER_PTE] =
> + pgdir[pgd_index(PAGE_OFFSET) + i / PTRS_PER_PTE] =
> + __pgd((phys_linear + i * sizeof(pte_t)) |
> + (_PAGE_PRESENT | _PAGE_RW | _PAGE_USER));
Same issue here.
+ for (i = 0; i < mapped_pages; i += PTRS_PER_PTE) {
+ pgd_t pgd;
+ pgd = __pgd((phys_linear + i * sizeof(pte_t)) |
+ (_PAGE_PRESENT | _PAGE_RW | _PAGE_USER));
+
+ if (copy_to_user(&pgdir[i / PTRS_PER_PTE], &pgd, sizeof(pgd))
+ || copy_to_user(&pgdir[pgd_index(PAGE_OFFSET)
+ + i / PTRS_PER_PTE],
+ &pgd, sizeof(pgd)))
+ return -EFAULT;
+ }
+
> + unsigned long initrd_size = 0, mem = 0;
> + struct boot_params *boot = (struct boot_params *) lg->mem_base;
> +
> + /* Get the guest memory size and the ramdisk size
> + * from the boot header located at lg->mem_base*/
> + if (copy_from_user(&mem, &boot->e820_map[0].size, 8))
> + kill_guest(&lg->cpus[0], "Error reading from user space");
This 8 here caused a stack issue (lg got set to NULL in the caller function)!
I changed mem to a u64.
> + lg->pgdirs[0].gpgdir = setup_pagetables(mem,
> + initrd_size, (unsigned long) lg->mem_base);
This can now error, so I did:
+ lg->pgdirs[0].gpgdir = setup_pagetables(lg, mem, initrd_size);
+ if (IS_ERR_VALUE(lg->pgdirs[0].gpgdir))
+ return lg->pgdirs[0].gpgdir;
Cheers!
Rusty.
_______________________________________________
Lguest mailing list
[email protected]
https://ozlabs.org/mailman/listinfo/lguest
