[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index] [Xen-changelog] [xen-unstable] [XEN] Fix page-fault handler to not trust bit 0 of error code.
# HG changeset patch # User kaf24@xxxxxxxxxxxxxxxxxxxx # Node ID e23961a8ce7eeffee8fb94c199818a5361f75639 # Parent 5033ffe8f53356cd7d8e52e9d4ce78c69e826d33 [XEN] Fix page-fault handler to not trust bit 0 of error code. It can be cleared due to writable-pagetable logic. Various other cleanups too. Spurious fault detection logic is simplified. Signed-off-by: Keir Fraser <keir@xxxxxxxxxxxxx> --- xen/arch/x86/mm.c | 8 +-- xen/arch/x86/traps.c | 90 +++++++++++++--------------------------- xen/arch/x86/x86_32/seg_fixup.c | 2 xen/arch/x86/x86_emulate.c | 4 - 4 files changed, 37 insertions(+), 67 deletions(-) diff -r 5033ffe8f533 -r e23961a8ce7e xen/arch/x86/mm.c --- a/xen/arch/x86/mm.c Sat Jun 17 12:57:03 2006 +0100 +++ b/xen/arch/x86/mm.c Sun Jun 18 19:24:00 2006 +0100 @@ -3351,7 +3351,7 @@ static int ptwr_emulated_update( addr &= ~(sizeof(paddr_t)-1); if ( copy_from_user(&full, (void *)addr, sizeof(paddr_t)) ) { - propagate_page_fault(addr, 4); /* user mode, read fault */ + propagate_page_fault(addr, 0); /* read fault */ return X86EMUL_PROPAGATE_FAULT; } /* Mask out bits provided by caller. */ @@ -3483,12 +3483,12 @@ int ptwr_do_page_fault(struct domain *d, unsigned long l2_idx; struct x86_emulate_ctxt emul_ctxt; - if ( unlikely(shadow_mode_enabled(d)) ) - return 0; + ASSERT(!shadow_mode_enabled(d)); /* * Attempt to read the PTE that maps the VA being accessed. By checking for * PDE validity in the L2 we avoid many expensive fixups in __get_user(). + * NB. The L2 entry cannot be detached as the caller already checked that. */ if ( !(l2e_get_flags(__linear_l2_table[l2_linear_offset(addr)]) & _PAGE_PRESENT) || @@ -3579,7 +3579,7 @@ int ptwr_do_page_fault(struct domain *d, } /* - * We only allow one ACTIVE and one INACTIVE p.t. to be updated at at + * We only allow one ACTIVE and one INACTIVE p.t. to be updated at a * time. If there is already one, we must flush it out. */ if ( d->arch.ptwr[which].l1va ) diff -r 5033ffe8f533 -r e23961a8ce7e xen/arch/x86/traps.c --- a/xen/arch/x86/traps.c Sat Jun 17 12:57:03 2006 +0100 +++ b/xen/arch/x86/traps.c Sun Jun 18 19:24:00 2006 +0100 @@ -547,6 +547,7 @@ static int handle_gdt_ldt_mapping_fault( { /* LDT fault: Copy a mapping from the guest's LDT, if it is valid. */ LOCK_BIGLOCK(d); + cleanup_writable_pagetable(d); ret = map_ldt_shadow_page(offset >> PAGE_SHIFT); UNLOCK_BIGLOCK(d); @@ -654,49 +655,21 @@ static int spurious_page_fault( static int spurious_page_fault( unsigned long addr, struct cpu_user_regs *regs) { + struct domain *d = current->domain; + int is_spurious; + + LOCK_BIGLOCK(d); + cleanup_writable_pagetable(d); + is_spurious = __spurious_page_fault(addr, regs); + UNLOCK_BIGLOCK(d); + + return is_spurious; +} + +static int fixup_page_fault(unsigned long addr, struct cpu_user_regs *regs) +{ struct vcpu *v = current; struct domain *d = v->domain; - int is_spurious; - - LOCK_BIGLOCK(d); - - is_spurious = __spurious_page_fault(addr, regs); - if ( is_spurious ) - goto out; - - /* - * The only possible reason for a spurious page fault not to be picked - * up already is that a page directory was unhooked by writable page table - * logic and then reattached before the faulting VCPU could detect it. - */ - if ( is_idle_domain(d) || /* no ptwr in idle domain */ - IN_HYPERVISOR_RANGE(addr) || /* no ptwr on hypervisor addrs */ - shadow_mode_enabled(d) || /* no ptwr logic in shadow mode */ - (regs->error_code & PGERR_page_present) ) /* not-present fault? */ - goto out; - - /* - * The page directory could have been detached again while we weren't - * holding the per-domain lock. Detect that and fix up if it's the case. - */ - if ( unlikely(d->arch.ptwr[PTWR_PT_ACTIVE].l1va) && - unlikely(l2_linear_offset(addr) == - d->arch.ptwr[PTWR_PT_ACTIVE].l2_idx) ) - { - ptwr_flush(d, PTWR_PT_ACTIVE); - is_spurious = 1; - } - - out: - UNLOCK_BIGLOCK(d); - return is_spurious; -} - -static int fixup_page_fault(unsigned long addr, struct cpu_user_regs *regs) -{ - struct vcpu *v = current; - struct domain *d = v->domain; - int rc; if ( unlikely(IN_HYPERVISOR_RANGE(addr)) ) { @@ -709,10 +682,7 @@ static int fixup_page_fault(unsigned lon * Do not propagate spurious faults in the hypervisor area to the * guest. It cannot fix them up. */ - LOCK_BIGLOCK(d); - rc = __spurious_page_fault(addr, regs); - UNLOCK_BIGLOCK(d); - return rc; + return (spurious_page_fault(addr, regs) ? EXCRET_not_a_fault : 0); } if ( unlikely(shadow_mode_enabled(d)) ) @@ -730,12 +700,14 @@ static int fixup_page_fault(unsigned lon return EXCRET_fault_fixed; } + /* + * Note it is *not* safe to check PGERR_page_present here. It can be + * clear, due to unhooked page table, when we would otherwise expect + * it to be set. We have an aversion to trusting that flag in Xen, and + * guests ought to be leery too. + */ if ( guest_kernel_mode(v, regs) && - /* Protection violation on write? No reserved-bit violation? */ - ((regs->error_code & (PGERR_page_present | - PGERR_write_access | - PGERR_reserved_bit)) == - (PGERR_page_present | PGERR_write_access)) && + (regs->error_code & PGERR_write_access) && ptwr_do_page_fault(d, addr, regs) ) { UNLOCK_BIGLOCK(d); @@ -870,8 +842,6 @@ static inline int admin_io_okay( (admin_io_okay(_p, 4, _d, _r) ? outl(_v, _p) : ((void)0)) /* Propagate a fault back to the guest kernel. */ -#define USER_READ_FAULT (PGERR_user_mode) -#define USER_WRITE_FAULT (PGERR_user_mode | PGERR_write_access) #define PAGE_FAULT(_faultaddr, _errcode) \ ({ propagate_page_fault(_faultaddr, _errcode); \ return EXCRET_fault_fixed; \ @@ -881,7 +851,7 @@ static inline int admin_io_okay( #define insn_fetch(_type, _size, _ptr) \ ({ unsigned long _x; \ if ( get_user(_x, (_type *)eip) ) \ - PAGE_FAULT(eip, USER_READ_FAULT); \ + PAGE_FAULT(eip, 0); /* read fault */ \ eip += _size; (_type)_x; }) static int emulate_privileged_op(struct cpu_user_regs *regs) @@ -950,17 +920,17 @@ static int emulate_privileged_op(struct case 1: data = (u8)inb_user((u16)regs->edx, v, regs); if ( put_user((u8)data, (u8 *)regs->edi) ) - PAGE_FAULT(regs->edi, USER_WRITE_FAULT); + PAGE_FAULT(regs->edi, PGERR_write_access); break; case 2: data = (u16)inw_user((u16)regs->edx, v, regs); if ( put_user((u16)data, (u16 *)regs->edi) ) - PAGE_FAULT(regs->edi, USER_WRITE_FAULT); + PAGE_FAULT(regs->edi, PGERR_write_access); break; case 4: data = (u32)inl_user((u16)regs->edx, v, regs); if ( put_user((u32)data, (u32 *)regs->edi) ) - PAGE_FAULT(regs->edi, USER_WRITE_FAULT); + PAGE_FAULT(regs->edi, PGERR_write_access); break; } regs->edi += (int)((regs->eflags & EF_DF) ? -op_bytes : op_bytes); @@ -975,17 +945,17 @@ static int emulate_privileged_op(struct { case 1: if ( get_user(data, (u8 *)regs->esi) ) - PAGE_FAULT(regs->esi, USER_READ_FAULT); + PAGE_FAULT(regs->esi, 0); /* read fault */ outb_user((u8)data, (u16)regs->edx, v, regs); break; case 2: if ( get_user(data, (u16 *)regs->esi) ) - PAGE_FAULT(regs->esi, USER_READ_FAULT); + PAGE_FAULT(regs->esi, 0); /* read fault */ outw_user((u16)data, (u16)regs->edx, v, regs); break; case 4: if ( get_user(data, (u32 *)regs->esi) ) - PAGE_FAULT(regs->esi, USER_READ_FAULT); + PAGE_FAULT(regs->esi, 0); /* read fault */ outl_user((u32)data, (u16)regs->edx, v, regs); break; } @@ -1168,7 +1138,7 @@ static int emulate_privileged_op(struct v->arch.guest_context.ctrlreg[2] = *reg; v->vcpu_info->arch.cr2 = *reg; break; - + case 3: /* Write CR3 */ LOCK_BIGLOCK(v->domain); cleanup_writable_pagetable(v->domain); diff -r 5033ffe8f533 -r e23961a8ce7e xen/arch/x86/x86_32/seg_fixup.c --- a/xen/arch/x86/x86_32/seg_fixup.c Sat Jun 17 12:57:03 2006 +0100 +++ b/xen/arch/x86/x86_32/seg_fixup.c Sun Jun 18 19:24:00 2006 +0100 @@ -464,7 +464,7 @@ int gpf_emulate_4gb(struct cpu_user_regs return 0; page_fault: - propagate_page_fault((unsigned long)pb, 4); + propagate_page_fault((unsigned long)pb, 0); /* read fault */ return EXCRET_fault_fixed; } diff -r 5033ffe8f533 -r e23961a8ce7e xen/arch/x86/x86_emulate.c --- a/xen/arch/x86/x86_emulate.c Sat Jun 17 12:57:03 2006 +0100 +++ b/xen/arch/x86/x86_emulate.c Sun Jun 18 19:24:00 2006 +0100 @@ -1146,7 +1146,7 @@ x86_emulate_read_std( *val = 0; if ( copy_from_user((void *)val, (void *)addr, bytes) ) { - propagate_page_fault(addr, 4); /* user mode, read fault */ + propagate_page_fault(addr, 0); /* read fault */ return X86EMUL_PROPAGATE_FAULT; } return X86EMUL_CONTINUE; @@ -1161,7 +1161,7 @@ x86_emulate_write_std( { if ( copy_to_user((void *)addr, (void *)&val, bytes) ) { - propagate_page_fault(addr, 6); /* user mode, write fault */ + propagate_page_fault(addr, PGERR_write_access); /* write fault */ return X86EMUL_PROPAGATE_FAULT; } return X86EMUL_CONTINUE; _______________________________________________ Xen-changelog mailing list Xen-changelog@xxxxxxxxxxxxxxxxxxx http://lists.xensource.com/xen-changelog
|
Lists.xenproject.org is hosted with RackSpace, monitoring our |