Broken error handling in guest_physmap_mark_populate_on_demand()

guest_physmap_mark_populate_on_demand(), before carrying out its actual operation, checks that the subject GFNs are not in use. If that check fails, the code prints a message and bypasses the gfn_unlock() matching the gfn_lock() carried out before entering the loop.

Further, the function is exposed to the use of guests on their own behalf. While we believe that this does not cause any further issues, we have not conducted a thorough enough review to be sure. Rather, it should be exposed only to privileged domains.

improper error handling (not release lock)



xen: fix error handling of guestphysmapmarkpopulateon_demand()

The only user of the out label bypasses a necessary unlock, thus enabling the caller to lock up Xen.

Also, the function was never meant to be called by a guest for itself, so rather than inspecting the code paths in depth for potential other problems this might cause, and adjusting e.g. the non-guest printk() in the above error path, just disallow the guest access to it.

Finally, the printk() (considering its potential of spamming the log, the more that it’s not using XENLOG_GUEST), is being converted to P2M_DEBUG(), as debugging is what it apparently was added for in the first place.

--- a/xen/arch/x86/mm/p2m-pod.c Fri Nov 16 15:56:14 2012 +0000
+++ b/xen/arch/x86/mm/p2m-pod.c Thu Nov 22 17:02:32 2012 +0000
@@ -1117,6 +1117,9 @@ guest_physmap_mark_populate_on_demand(st
     mfn_t omfn;
     int rc = 0;
+    if ( !IS_PRIV_FOR(current->domain, d) )
+        return -EPERM;
     if ( !paging_mode_translate(d) )
         return -EINVAL;
@@ -1135,8 +1138,7 @@ guest_physmap_mark_populate_on_demand(st
         omfn = p2m->get_entry(p2m, gfn + i, &ot, &a, 0, NULL);
         if ( p2m_is_ram(ot) )
-            printk("%s: gfn_to_mfn returned type %d!\n",
-                   __func__, ot);
+            P2M_DEBUG("gfn_to_mfn returned type %d!\n", ot);
             rc = -EBUSY;
             goto out;
@@ -1160,9 +1162,9 @@ guest_physmap_mark_populate_on_demand(st
     gfn_unlock(p2m, gfn, order);
     return rc;


A malicious guest administrator can cause Xen to hang.