On Tue, 2007-12-11 at 16:11 +0900, Kouya Shimura wrote:
> There is a security vulnerability in PAL emulation
> since alt-dtlb miss handler of HVM absolutely
> inserts a identity-mapped TLB when psr.vm=0.
>
> HVM guest can access an arbitrary machine physical
> memory with this security hole.
>
> Actually windows 2008 destroys the content of machine
> physical address 0x108000. This is a serious problem.
>
> I tried to support PV domain with the same logic too
> but it doesn't work well since PV domain can't hold
> more than one vtlb.
I think there's a off by one issue as noted below. Also a few
cleanups. Like Tristan, I'd rather see a xencomm for GFW, but that
might be too intrusive for 3.2 at this point. So this seems like a
reasonable temporary fix. Thanks,
Alex
> diff -r 4054cd60895b xen/arch/ia64/vmx/vmx_fault.c
> --- a/xen/arch/ia64/vmx/vmx_fault.c Mon Dec 10 13:49:22 2007 +0000
> +++ b/xen/arch/ia64/vmx/vmx_fault.c Mon Dec 10 18:39:23 2007 +0900
> @@ -174,6 +174,7 @@ vmx_ia64_handle_break (unsigned long ifa
> {
> struct domain *d = current->domain;
> struct vcpu *v = current;
> + IA64FAULT fault;
>
> perfc_incr(vmx_ia64_handle_break);
> #ifdef CRASH_DEBUG
> @@ -196,9 +197,9 @@ vmx_ia64_handle_break (unsigned long ifa
> return IA64_NO_FAULT;
> }
> else if (iim == DOMN_PAL_REQUEST) {
> - pal_emul(v);
> - vcpu_increment_iip(v);
> - return IA64_NO_FAULT;
> + if ((fault = pal_emul(v)) == IA64_NO_FAULT)
Please split this up:
fault = pal_emul(v);
if (fault == IA64_NO_FAULT)
...
> + vcpu_increment_iip(v);
> + return fault;
> } else if (iim == DOMN_SAL_REQUEST) {
> sal_emul(v);
> vcpu_increment_iip(v);
> diff -r 4054cd60895b xen/arch/ia64/xen/fw_emul.c
> --- a/xen/arch/ia64/xen/fw_emul.c Mon Dec 10 13:49:22 2007 +0000
> +++ b/xen/arch/ia64/xen/fw_emul.c Tue Dec 11 15:00:36 2007 +0900
...
> +static IA64FAULT
> +palcomm_init(struct palcomm_ctxt *comm, unsigned long buf, long size)
> +{
> + IA64FAULT fault = IA64_NO_FAULT;
> + int i;
> + unsigned long paddr, maddr, poff;
> + struct page_info* page;
> +
> + BUG_ON((unsigned)size > MIN_PAGE_SIZE);
> +
> + /* check for vulnerability. NB - go through in metaphysical mode. */
> + if (IS_VMM_ADDRESS(buf) || IS_VMM_ADDRESS(buf + size))
Should the end address be (buf + size - 1)? Seems this could
incorrectly trigger on the next page if the buffer has the right
alignment.
> + panic_domain(NULL, "copy to bad address:0x%lx\n", buf);
> +
> + comm->va[0] = comm->va[1] = NULL;
> +
> + /* XXX: not implemented for PV domain */
> + if (!VMX_DOMAIN(current))
> + return fault;
> +
> + for (i = 0; i < 2; i++) {
> + if (is_virtual_mode(current)) {
> + fault = vmx_vcpu_tpa(current, buf, &paddr);
> + if (fault != IA64_NO_FAULT)
> + goto fail;
> + } else
> + paddr = buf;
> + if ((maddr = paddr_to_maddr(paddr)) == 0)
Split up:
maddr = paddr_to_maddr(paddr);
if (maddr == 0)
...
> @@ -801,21 +896,22 @@ xen_pal_emulator(unsigned long index, u6
> case PAL_PERF_MON_INFO:
> {
> unsigned long pm_buffer[16];
> + struct palcomm_ctxt comm;
> + IA64FAULT fault;
> +
> + fault = palcomm_init(&comm, in1, 128);
s/128/sizeof(pm_buffer)/
> + if (fault != IA64_NO_FAULT)
> + return fault;
> +
> status = ia64_pal_perf_mon_info(
> pm_buffer,
> (pal_perf_mon_info_u_t *) &r9);
> - if (status != 0) {
> - while(1)
> - printk("PAL_PERF_MON_INFO fails ret=%ld\n",
> status);
> - break;
> + if (status == PAL_STATUS_SUCCESS) {
> + if (palcomm_copy_to_guest(&comm, in1,
> + pm_buffer, 128))
same
> @@ -885,9 +988,19 @@ xen_pal_emulator(unsigned long index, u6
> case PAL_BRAND_INFO:
> if (in1 == 0) {
> char brand_info[128];
> + struct palcomm_ctxt comm;
> + IA64FAULT fault;
> +
> + fault = palcomm_init(&comm, in2, 128);
s/128/sizeof(brand_info)/
> + if (fault != IA64_NO_FAULT)
> + return fault;
> status = ia64_pal_get_brand_info(brand_info);
> - if (status == PAL_STATUS_SUCCESS)
> - copy_to_user((void *)in2, brand_info, 128);
> + if (status == PAL_STATUS_SUCCESS) {
> + if (palcomm_copy_to_guest(&comm, in2,
> + brand_info, 128))
same
--
Alex Williamson HP Open Source & Linux Org.
_______________________________________________
Xen-ia64-devel mailing list
Xen-ia64-devel@xxxxxxxxxxxxxxxxxxx
http://lists.xensource.com/xen-ia64-devel
|