Re: [Xen-devel] [v7][RFC][PATCH 01/13] xen: RMRR fix

["Chen, Tiejun" <tiejun.chen@xxxxxxxxx>]

On 2014/10/29 16:45, Jan Beulich wrote:
> > > > On 29.10.14 at 03:51, <tiejun.chen@xxxxxxxxx> wrote:
> > On 2014/10/29 8:48, Chen, Tiejun wrote:
> > > On 2014/10/28 17:34, Jan Beulich wrote:
> > > > > > > On 28.10.14 at 09:36, <tiejun.chen@xxxxxxxxx> wrote:
> > > > > On 2014/10/27 17:41, Jan Beulich wrote:
> > > > > > > > > On 27.10.14 at 03:00, <tiejun.chen@xxxxxxxxx> wrote:
> > > > > > > n 2014/10/24 18:52, Jan Beulich wrote:
> > > > > > > > > > > On 24.10.14 at 09:34, <tiejun.chen@xxxxxxxxx> wrote:
> > > > > > > > > Now in our case we add a rule:
> > > > > > > > >      - if p2m_access_n is set we also set this mapping.
> > > > > > > > Does that not conflict with eventual use mem-access makes of this
> > > > > > > > type?
> > > > > Do you mean what will happen after we reset these ranges as
> > > > > p2m_access_rw? We already reserve these ranges guest shouldn't access
> > > > > these range actually. And a guest still maliciously access them, that
> > > > > device may not work well.
> > > > mem-access is functionality used by a control domain, not the domain
> > > I really don't know this mechanism so thanks for your good coverage.
> > >
> > > > itself. You need to make sure that neither your use of p2m_access_n
> > > > can confuse the mem-access code, nor that their use can confuse you.
> > > Absolutely, but I think I need to know more about mem-access firstly.
> > I think these reserved device memory shouldn't be pocked since any write
> > may affect device. Even, what if a device with RMRR isn't assign current
> > domain? And read also should not be allowed since this still may
> > introduce some potential unexpected behavior to device.
> >
> > So if mem_access is trying to access those RMRR range, could we let
> > mem_access exit directly with some message? I mean we can check if we're
> > accessing those RMRR ranges in case of XENMEM_access_op_set_access.
> Sounds reasonable at first glance.
I think we just guarantee no one set mem_access for those ranges, but 
its fine to get mem_access:
diff --git a/xen/common/mem_access.c b/xen/common/mem_access.c
index 6c2724b..4c84f88 100644
--- a/xen/common/mem_access.c
+++ b/xen/common/mem_access.c
@@ -55,6 +55,31 @@ void mem_access_resume(struct domain *d)
     }
 }

+/* We can't expose reserved device memory. */
+static int mem_access_check_rdm(struct domain *d, uint64_aligned_t start,
+                                uint32_t nr)
+{
+    uint32_t i;
+    uint64_aligned_t gfn;
+    int rc = 0;
+
+    if ( !is_hardware_domain(d) )
+    {
+        for ( i = 0; i < nr; i++ )
+        {
+            gfn = start + i;
+            rc = 
iommu_get_reserved_device_memory(p2m_check_reserved_device_memory,
+                                                  &gfn);
+            if ( rc < 0 )
+                printk(XENLOG_WARNING
+                       "Domain %d can't check reserved device memory.\n",
+                       d->domain_id);
+        }
+    }
+
+    return rc;
+}
+
 int mem_access_memop(unsigned long cmd,
                      XEN_GUEST_HANDLE_PARAM(xen_mem_access_op_t) arg)
 {
@@ -99,6 +124,15 @@ int mem_access_memop(unsigned long cmd,
               ((mao.pfn + mao.nr - 1) > domain_get_maximum_gpfn(d))) )
             break;

+        rc =  mem_access_check_rdm(d, mao.pfn, mao.nr);
+        if ( rc == 1 )
+        {
+            printk(XENLOG_WARNING
+                   "Domain %d: we shouldn't mem_access reserved device 
memory.\n",
+                   d->domain_id);
+            break;
+        }
+
         rc = p2m_set_mem_access(d, mao.pfn, mao.nr, start_iter,
                                 MEMOP_CMD_MASK, mao.access);
         if ( rc > 0 )


Thanks
Tiejun

_______________________________________________
Xen-devel mailing list
Xen-devel@xxxxxxxxxxxxx
http://lists.xen.org/xen-devel