WARNING - OLD ARCHIVES

This is an archived copy of the Xen.org mailing list, which we have preserved to ensure that existing links to archives are not broken. The live archive, which contains the latest emails, can be found at http://lists.xen.org/
   
 
 
Xen 
 
Home Products Support Community News
 
   
 

xen-devel

[Xen-devel] [PATCH] fix error path handling in physdev

To: xen-devel <xen-devel@xxxxxxxxxxxxxxxxxxxxx>
Subject: [Xen-devel] [PATCH] fix error path handling in physdev
From: Muli Ben-Yehuda <mulix@xxxxxxxxx>
Date: Wed, 9 Mar 2005 05:54:18 +0200
Delivery-date: Wed, 09 Mar 2005 03:55:34 +0000
Envelope-to: xen+James.Bulpin@xxxxxxxxxxxx
List-archive: <http://sourceforge.net/mailarchive/forum.php?forum=xen-devel>
List-help: <mailto:xen-devel-request@lists.sourceforge.net?subject=help>
List-id: List for Xen developers <xen-devel.lists.sourceforge.net>
List-post: <mailto:xen-devel@lists.sourceforge.net>
List-subscribe: <https://lists.sourceforge.net/lists/listinfo/xen-devel>, <mailto:xen-devel-request@lists.sourceforge.net?subject=subscribe>
List-unsubscribe: <https://lists.sourceforge.net/lists/listinfo/xen-devel>, <mailto:xen-devel-request@lists.sourceforge.net?subject=unsubscribe>
Sender: xen-devel-admin@xxxxxxxxxxxxxxxxxxxxx
User-agent: Mutt/1.5.6+20040907i
This patch fixes the error path handling in physdev.o. 
- handle memory allocation failures properly
- physdev_pci_access_modify: if something fails, remove the new
physdev or reset its access mode if it already existed
- clear the priviledge bits if something fails
- do_physdev_op: properly check copy_to-user

Patch against the unstable tree, works for me in qemu on x86 with domU
accessing a single pci device, but more testing appreciated (is anyone
using the direct pci access stuff?)

Signed-Off-By: Muli Ben-Yehuda <mulix@xxxxxxxxx>

diff -Naurp --exclude-from /home/muli/w/dontdiff 
xen-unstable-src-200503071354/xen/common/physdev.c vpci/xen/common/physdev.c
--- xen-unstable-src-200503071354/xen/common/physdev.c  2005-03-03 
23:17:33.000000000 -0500
+++ vpci/xen/common/physdev.c   2005-03-08 21:54:33.000000000 -0500
@@ -85,31 +85,93 @@ static phys_dev_t *find_pdev(struct doma
 }
 
 /* Add a device to a per-domain device-access list. */
-static void add_dev_to_task(struct domain *p, 
-                            struct pci_dev *dev, int acc)
+static int add_dev_to_task(struct domain *p, struct pci_dev *dev, 
+                           int acc)
 {
-    phys_dev_t *pdev;
+    phys_dev_t *physdev;
     
-    if ( (pdev = find_pdev(p, dev)) )
-    {
-        /* Sevice already on list: update access permissions. */
-        pdev->flags = acc;
-        return;
-    }
-
-    if ( (pdev = xmalloc(phys_dev_t)) == NULL )
+    if ( (physdev = xmalloc(phys_dev_t)) == NULL )
     {
         INFO("Error allocating pdev structure.\n");
-        return;
+        return -ENOMEM;
     }
     
-    pdev->dev = dev;
-    pdev->flags = acc;
-    pdev->state = 0;
-    list_add(&pdev->node, &p->pcidev_list);
+    physdev->dev = dev;
+    physdev->flags = acc;
+    physdev->state = 0;
+    list_add(&physdev->node, &p->pcidev_list);
 
     if ( acc == ACC_WRITE )
-        pdev->owner = p;
+        physdev->owner = p;
+
+    return 0;
+}
+
+/* Remove a device from a per-domain device-access list. */
+static void remove_dev_from_task(struct domain *p, struct pci_dev *dev)
+{
+    phys_dev_t *physdev = find_pdev(p, dev);
+
+    if ( physdev == NULL )
+        BUG();
+    
+    list_del(&physdev->node);
+
+    xfree(physdev);
+}
+
+static int setup_ioport_memory_access(domid_t dom, struct domain* p, 
+                                      struct exec_domain* ed,
+                                      struct pci_dev *pdev)
+{
+    struct exec_domain* edc;
+    int i, j;
+
+    /* Now, setup access to the IO ports and memory regions for the device. */
+    if ( ed->arch.io_bitmap == NULL )
+    {
+        if ( (ed->arch.io_bitmap = xmalloc_array(u8, IOBMP_BYTES)) == NULL )
+            return -ENOMEM;
+
+        memset(ed->arch.io_bitmap, 0xFF, IOBMP_BYTES);
+
+        ed->arch.io_bitmap_sel = ~0ULL;
+
+        for_each_exec_domain(p, edc) {
+            if (edc == ed)
+                continue;
+            edc->arch.io_bitmap = ed->arch.io_bitmap;
+        }
+    }
+
+    for ( i = 0; i < DEVICE_COUNT_RESOURCE; i++ )
+    {
+        struct resource *r = &pdev->resource[i];
+        
+        if ( r->flags & IORESOURCE_IO )
+        {
+            /* Give the domain access to the IO ports it needs.  Currently,
+             * this will allow all processes in that domain access to those
+             * ports as well.  This will do for now, since driver domains don't
+             * run untrusted processes! */
+            INFO("Giving domain %u IO resources (%lx - %lx) "
+                 "for device %s\n", dom, r->start, r->end, pdev->slot_name);
+            for ( j = r->start; j < r->end + 1; j++ )
+            {
+                clear_bit(j, ed->arch.io_bitmap);
+                clear_bit(j / IOBMP_BITS_PER_SELBIT, &ed->arch.io_bitmap_sel);
+            }
+        }
+        /* rights to IO memory regions are checked when the domain maps them */
+    }
+
+    for_each_exec_domain(p, edc) {
+        if (edc == ed)
+            continue;
+        edc->arch.io_bitmap_sel = ed->arch.io_bitmap_sel;
+    }
+
+    return 0;
 }
 
 /*
@@ -120,13 +182,15 @@ static void add_dev_to_task(struct domai
  * bridge, then the domain should get access to all the leaf devices below
  * that bridge (XXX this is unimplemented!).
  */
-int physdev_pci_access_modify(
-    domid_t dom, int bus, int dev, int func, int enable)
+int physdev_pci_access_modify(domid_t dom, int bus, int dev, int func, 
+                              int enable)
 {
     struct domain *p;
-    struct exec_domain *ed, *edc;
+    struct exec_domain *ed;
     struct pci_dev *pdev;
-    int i, j, rc = 0;
+    phys_dev_t *physdev;
+    int rc = 0;
+    int oldacc = -1, allocated_physdev = 0;
 
     if ( !IS_PRIV(current->domain) )
         BUG();
@@ -158,66 +222,47 @@ int physdev_pci_access_modify(
     {
         INFO("  dev does not exist\n");
         rc = -ENODEV;
-        goto out;
+        goto clear_priviledge;
+    }
+    
+    if ( (physdev = find_pdev(p, pdev)) != NULL) {
+        /* Sevice already on list: update access permissions. */
+        oldacc = physdev->flags;
+        physdev->flags = ACC_WRITE;
+    } else {
+        if ( (rc = add_dev_to_task(p, pdev, ACC_WRITE)) < 0)
+            goto clear_priviledge;
+        allocated_physdev = 1;
     }
-    add_dev_to_task(p, pdev, ACC_WRITE);
 
     INFO("  add RW %02x:%02x:%02x\n", pdev->bus->number,
          PCI_SLOT(pdev->devfn), PCI_FUNC(pdev->devfn));
 
     /* Is the device a bridge or cardbus? */
-    if ( pdev->hdr_type != PCI_HEADER_TYPE_NORMAL )
+    if ( pdev->hdr_type != PCI_HEADER_TYPE_NORMAL ) {
         INFO("XXX can't give access to bridge devices yet\n");
-
-    /* Now, setup access to the IO ports and memory regions for the device. */
-
-    if ( ed->arch.io_bitmap == NULL )
-    {
-        if ( (ed->arch.io_bitmap = xmalloc_array(u8, IOBMP_BYTES)) == NULL )
-        {
-            rc = -ENOMEM;
-            goto out;
-        }
-        memset(ed->arch.io_bitmap, 0xFF, IOBMP_BYTES);
-
-        ed->arch.io_bitmap_sel = ~0ULL;
-
-        for_each_exec_domain(p, edc) {
-            if (edc == ed)
-                continue;
-            edc->arch.io_bitmap = ed->arch.io_bitmap;
-        }
+        rc = -EPERM;
+        goto remove_dev;
     }
 
-    for ( i = 0; i < DEVICE_COUNT_RESOURCE; i++ )
-    {
-        struct resource *r = &pdev->resource[i];
-        
-        if ( r->flags & IORESOURCE_IO )
-        {
-            /* Give the domain access to the IO ports it needs.  Currently,
-             * this will allow all processes in that domain access to those
-             * ports as well.  This will do for now, since driver domains don't
-             * run untrusted processes! */
-            INFO("Giving domain %u IO resources (%lx - %lx) "
-                 "for device %s\n", dom, r->start, r->end, pdev->slot_name);
-            for ( j = r->start; j < r->end + 1; j++ )
-            {
-                clear_bit(j, ed->arch.io_bitmap);
-                clear_bit(j / IOBMP_BITS_PER_SELBIT, &ed->arch.io_bitmap_sel);
-            }
-        }
+    if ( (rc = setup_ioport_memory_access(dom, p, ed, pdev)) < 0 )
+        goto remove_dev;
 
-        /* rights to IO memory regions are checked when the domain maps them */
-    }
+    put_domain(p);
+    return rc;
 
-    for_each_exec_domain(p, edc) {
-        if (edc == ed)
-            continue;
-        edc->arch.io_bitmap_sel = ed->arch.io_bitmap_sel;
+remove_dev:
+    if (allocated_physdev) {
+        /* new device was added - remove it from the list */
+        remove_dev_from_task(p, pdev);
+    } else {
+        /* device already existed - just undo the access changes */
+        physdev->flags = oldacc;
     }
-
- out:
+    
+clear_priviledge:
+    clear_bit(DF_PHYSDEV, &p->d_flags);
+    clear_bit(DF_PRIVILEGED, &p->d_flags);
     put_domain(p);
     return rc;
 }
@@ -708,7 +753,9 @@ long do_physdev_op(physdev_op_t *uop)
         break;
     }
 
-    copy_to_user(uop, &op, sizeof(op));
+    if (copy_to_user(uop, &op, sizeof(op)))
+        ret = -EFAULT;
+
     return ret;
 }
 
@@ -764,7 +811,12 @@ void physdev_init_dom0(struct domain *p)
         if ( (dev->hdr_type != PCI_HEADER_TYPE_NORMAL) &&
              (dev->hdr_type != PCI_HEADER_TYPE_CARDBUS) )
             continue;
-        pdev = xmalloc(phys_dev_t);
+
+        if ( (pdev = xmalloc(phys_dev_t)) == NULL ) {
+            INFO("failed to allocate physical device structure!\n");
+            break;
+        }
+
         pdev->dev = dev;
         pdev->flags = ACC_WRITE;
         pdev->state = 0;

-- 
Muli Ben-Yehuda
http://www.mulix.org | http://mulix.livejournal.com/



-------------------------------------------------------
SF email is sponsored by - The IT Product Guide
Read honest & candid reviews on hundreds of IT Products from real users.
Discover which products truly live up to the hype. Start reading now.
http://ads.osdn.com/?ad_id=6595&alloc_id=14396&op=click
_______________________________________________
Xen-devel mailing list
Xen-devel@xxxxxxxxxxxxxxxxxxxxx
https://lists.sourceforge.net/lists/listinfo/xen-devel

<Prev in Thread] Current Thread [Next in Thread>