From: Jarod Wilson <jwilson@redhat.com> Date: Tue, 18 Dec 2007 12:12:04 -0500 Subject: [xen] ia64: vulnerability of copy_to_user in PAL emu Message-id: 4767FF64.6030608@redhat.com O-Subject: Re: [RHEL5.2 PATCH] [xen] ia64: fix vulnerability of copy_to_user in PAL emu {CVE-2007-6416} Bugzilla: 425939 Jarod Wilson wrote: > Bugzilla #425381: CVE-2007-6416 [RHEL 5.2] [XEN/IA64] Security: > vulnerability of copy_to_user in PAL emulation > https://bugzilla.redhat.com/show_bug.cgi?id=425381 > > > Description > ----------- >>From upstream: > -- > 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. > > I think this patch is enough for normal usage. Please see SDM > Vol2 11.10.2.1.3 "Making PAL Procedure Calls in Physical or > Virtual Mode". If the caller has a responsibility of providing > DTR or DTC mapping, xencomm for PAL might be unnecessary. > > I confirmed there is no problem in linux, windows 2003, windows > 2008 with this patch. > > As for PV domain, the same logic can't be used due to only one > vTLB. This patch only checks that the buffer never points VMM > address, that would avoid the vulnerability. > -- > > Alex Williamson w/HP (the ia64 xen maintainer) confirmed this > vulnerability does exist in the 3.1.x codebase as well. > > Fixed upstream by the folowing changeset: > http://xenbits.xensource.com/ext/ia64/xen-unstable.hg?rev/e6069a715fd7 > > The changes are isolated to ia64-specific code. Very straight-forward > backport, basically just leaving out one chunk that pertains to > additional code added in the 3.2 codebase. > > > Test status > ----------- > I've run kernels carrying the attached patch on multiple ia64 systems, > with multiple hvm and pv guests running, and everything continues to > function normally, no regressions that I've been able to find. > > Please ACK Oops, here's the patch... -- Jarod Wilson jwilson@redhat.com Acked-by: Rik van Riel <riel@redhat.com> Acked-by: Prarit Bhargava <prarit@redhat.com> Acked-by: "Stephen C. Tweedie" <sct@redhat.com> diff --git a/arch/ia64/xen/fw_emul.c b/arch/ia64/xen/fw_emul.c index 3c28228..bd4b86e 100644 --- a/arch/ia64/xen/fw_emul.c +++ b/arch/ia64/xen/fw_emul.c @@ -35,6 +35,7 @@ #include <xen/hypercall.h> #include <xen/softirq.h> #include <xen/time.h> +#include <asm/vmx_phy_mode.h> static DEFINE_SPINLOCK(efi_time_services_lock); @@ -446,6 +447,45 @@ sal_emulator (long index, unsigned long in1, unsigned long in2, return ((struct sal_ret_values) {status, r9, r10, r11}); } +static int +safe_copy_to_guest(unsigned long to, void *from, long size) +{ + BUG_ON((unsigned)size > PAGE_SIZE); + + if (VMX_DOMAIN(current)) { + if (is_virtual_mode(current)) { + thash_data_t *data; + unsigned long gpa, poff; + + /* The caller must provide a DTR or DTC mapping */ + data = vtlb_lookup(current, to, DSIDE_TLB); + if (data) { + gpa = data->page_flags & _PAGE_PPN_MASK; + } else { + data = vhpt_lookup(to); + if (!data) + return -1; + gpa = __mpa_to_gpa( + data->page_flags & _PAGE_PPN_MASK); + gpa &= _PAGE_PPN_MASK; + } + poff = POFFSET(to, data->ps); + if (poff + size > PSIZE(data->ps)) + return -1; + to = PAGEALIGN(gpa, data->ps) | poff; + } + to |= XENCOMM_INLINE_FLAG; + if (xencomm_copy_to_guest((void *)to, from, size, 0) != 0) + return -1; + return 0; + } else { + /* check for vulnerability */ + if (IS_VMM_ADDRESS(to) || IS_VMM_ADDRESS(to + size - 1)) + panic_domain(NULL, "copy to bad address:0x%lx\n", to); + return copy_to_user((void __user *)to, from, size); + } +} + cpumask_t cpu_cache_coherent_map; struct cache_flush_args { @@ -682,16 +722,13 @@ xen_pal_emulator(unsigned long index, u64 in1, u64 in2, u64 in3) 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 (copy_to_user((void __user *)in1,pm_buffer,128)) { - while(1) - printk("xen_pal_emulator: PAL_PERF_MON_INFO " - "can't copy to user!!!!\n"); - status = PAL_STATUS_UNIMPLEMENTED; - break; + if (safe_copy_to_guest( + in1, pm_buffer, sizeof(pm_buffer))) { + status = PAL_STATUS_EINVAL; + goto fail_to_copy; } } break; @@ -713,10 +750,11 @@ xen_pal_emulator(unsigned long index, u64 in1, u64 in2, u64 in3) consumes 10 mW, implemented and cache/TLB coherent. */ unsigned long res = 1000UL | (1000UL << 16) | (10UL << 32) | (1UL << 61) | (1UL << 60); - if (copy_to_user ((void *)in1, &res, sizeof (res))) - status = PAL_STATUS_EINVAL; - else - status = PAL_STATUS_SUCCESS; + if (safe_copy_to_guest (in1, &res, sizeof (res))) { + status = PAL_STATUS_EINVAL; + goto fail_to_copy; + } + status = PAL_STATUS_SUCCESS; } break; case PAL_HALT: @@ -747,6 +785,12 @@ xen_pal_emulator(unsigned long index, u64 in1, u64 in2, u64 in3) break; } return ((struct ia64_pal_retval) {status, r9, r10, r11}); + +fail_to_copy: + gdprintk(XENLOG_WARNING, + "PAL(%ld) fail to copy!!! args 0x%lx 0x%lx 0x%lx\n", + index, in1, in2, in3); + return ((struct ia64_pal_retval) {status, r9, r10, r11}); } // given a current domain (virtual or metaphysical) address, return the virtual address