Re: [Xen-ia64-devel] [PATCH][TAKE3] Fix vulnerability of copy_to_user in PAL emulation

[Jarod Wilson <jwilson@xxxxxxxxxx>]

Alex Williamson wrote:
> On Fri, 2007-12-14 at 13:55 -0500, Jarod Wilson wrote:
>> Alex Williamson wrote:
>>> On Fri, 2007-12-14 at 15:52 +0900, Kouya Shimura wrote:
>>>> Hi,
>>>>
>>>> The reputation of my previous patch was not so good,
>>>> then I rewrote it. An attached patch is temporary fix
>>>> for xen-3.2.
>> Anyone know offhand if this vulnerability exists in xen 3.1.x as well? 
>> (As in, is this something a certain vendor shipping a xen 3.1.x codebase 
>> needs to pull into their own tree ASAP? :)
> 
>    Yes, I believe the exposure is there too.  I was planning to port it
> to the 3.1 tree once it's in the mainline.  It looks very straight
> forward, just omit the chunk updating pal_brand_info since that didn't
> exist back then.  Thanks,

Yeah, was a pretty straight-forward port to our 3.1.x codebase. Dunno if
you've already taken care of it, but just in case, attached is the
back-port I put together for RHEL5. (not an hg diff, sorry...)

-- 
Jarod Wilson
jwilson@xxxxxxxxxx

diff -Naurp xen/arch/ia64/xen/fw_emul.c xen.fix/arch/ia64/xen/fw_emul.c
--- xen/arch/ia64/xen/fw_emul.c 2007-12-06 12:48:38.000000000 -0500
+++ xen.fix/arch/ia64/xen/fw_emul.c     2007-12-17 12:27:55.000000000 -0500
@@ -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 
        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, u6
                                        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, u6
                       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, u6
                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

Attachment: signature.asc
Description: OpenPGP digital signature

_______________________________________________
Xen-ia64-devel mailing list
Xen-ia64-devel@xxxxxxxxxxxxxxxxxxx
http://lists.xensource.com/xen-ia64-devel