ia64/xen-unstable

changeset 4035:9d3594cbf610

bitkeeper revision 1.1236.12.17 (422ebd6bbZvWUE6_1BGsaHxqMbgdQg)

[PATCH] [PATCH] fix error path handling in physdev

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@mulix.org>
author mulix@mulix.org[kaf24]
date Wed Mar 09 09:10:03 2005 +0000 (2005-03-09)
parents b27fb6c2ea52
children 98506ea56438
files BitKeeper/etc/logging_ok xen/common/physdev.c
line diff
     1.1 --- a/BitKeeper/etc/logging_ok	Wed Mar 09 09:01:46 2005 +0000
     1.2 +++ b/BitKeeper/etc/logging_ok	Wed Mar 09 09:10:03 2005 +0000
     1.3 @@ -52,6 +52,7 @@ mafetter@fleming.research
     1.4  mark@maw48.kings.cam.ac.uk
     1.5  maw48@labyrinth.cl.cam.ac.uk
     1.6  mjw@wray-m-3.hpl.hp.com
     1.7 +mulix@mulix.org
     1.8  mwilli2@anvil.research
     1.9  mwilli2@equilibrium.research
    1.10  mwilli2@equilibrium.research.intel-research.net
     2.1 --- a/xen/common/physdev.c	Wed Mar 09 09:01:46 2005 +0000
     2.2 +++ b/xen/common/physdev.c	Wed Mar 09 09:10:03 2005 +0000
     2.3 @@ -85,31 +85,93 @@ static phys_dev_t *find_pdev(struct doma
     2.4  }
     2.5  
     2.6  /* Add a device to a per-domain device-access list. */
     2.7 -static void add_dev_to_task(struct domain *p, 
     2.8 -                            struct pci_dev *dev, int acc)
     2.9 +static int add_dev_to_task(struct domain *p, struct pci_dev *dev, 
    2.10 +                           int acc)
    2.11  {
    2.12 -    phys_dev_t *pdev;
    2.13 +    phys_dev_t *physdev;
    2.14      
    2.15 -    if ( (pdev = find_pdev(p, dev)) )
    2.16 +    if ( (physdev = xmalloc(phys_dev_t)) == NULL )
    2.17      {
    2.18 -        /* Sevice already on list: update access permissions. */
    2.19 -        pdev->flags = acc;
    2.20 -        return;
    2.21 +        INFO("Error allocating pdev structure.\n");
    2.22 +        return -ENOMEM;
    2.23 +    }
    2.24 +    
    2.25 +    physdev->dev = dev;
    2.26 +    physdev->flags = acc;
    2.27 +    physdev->state = 0;
    2.28 +    list_add(&physdev->node, &p->pcidev_list);
    2.29 +
    2.30 +    if ( acc == ACC_WRITE )
    2.31 +        physdev->owner = p;
    2.32 +
    2.33 +    return 0;
    2.34 +}
    2.35 +
    2.36 +/* Remove a device from a per-domain device-access list. */
    2.37 +static void remove_dev_from_task(struct domain *p, struct pci_dev *dev)
    2.38 +{
    2.39 +    phys_dev_t *physdev = find_pdev(p, dev);
    2.40 +
    2.41 +    if ( physdev == NULL )
    2.42 +        BUG();
    2.43 +    
    2.44 +    list_del(&physdev->node);
    2.45 +
    2.46 +    xfree(physdev);
    2.47 +}
    2.48 +
    2.49 +static int setup_ioport_memory_access(domid_t dom, struct domain* p, 
    2.50 +                                      struct exec_domain* ed,
    2.51 +                                      struct pci_dev *pdev)
    2.52 +{
    2.53 +    struct exec_domain* edc;
    2.54 +    int i, j;
    2.55 +
    2.56 +    /* Now, setup access to the IO ports and memory regions for the device. */
    2.57 +    if ( ed->arch.io_bitmap == NULL )
    2.58 +    {
    2.59 +        if ( (ed->arch.io_bitmap = xmalloc_array(u8, IOBMP_BYTES)) == NULL )
    2.60 +            return -ENOMEM;
    2.61 +
    2.62 +        memset(ed->arch.io_bitmap, 0xFF, IOBMP_BYTES);
    2.63 +
    2.64 +        ed->arch.io_bitmap_sel = ~0ULL;
    2.65 +
    2.66 +        for_each_exec_domain(p, edc) {
    2.67 +            if (edc == ed)
    2.68 +                continue;
    2.69 +            edc->arch.io_bitmap = ed->arch.io_bitmap;
    2.70 +        }
    2.71      }
    2.72  
    2.73 -    if ( (pdev = xmalloc(phys_dev_t)) == NULL )
    2.74 +    for ( i = 0; i < DEVICE_COUNT_RESOURCE; i++ )
    2.75      {
    2.76 -        INFO("Error allocating pdev structure.\n");
    2.77 -        return;
    2.78 +        struct resource *r = &pdev->resource[i];
    2.79 +        
    2.80 +        if ( r->flags & IORESOURCE_IO )
    2.81 +        {
    2.82 +            /* Give the domain access to the IO ports it needs.  Currently,
    2.83 +             * this will allow all processes in that domain access to those
    2.84 +             * ports as well.  This will do for now, since driver domains don't
    2.85 +             * run untrusted processes! */
    2.86 +            INFO("Giving domain %u IO resources (%lx - %lx) "
    2.87 +                 "for device %s\n", dom, r->start, r->end, pdev->slot_name);
    2.88 +            for ( j = r->start; j < r->end + 1; j++ )
    2.89 +            {
    2.90 +                clear_bit(j, ed->arch.io_bitmap);
    2.91 +                clear_bit(j / IOBMP_BITS_PER_SELBIT, &ed->arch.io_bitmap_sel);
    2.92 +            }
    2.93 +        }
    2.94 +        /* rights to IO memory regions are checked when the domain maps them */
    2.95      }
    2.96 -    
    2.97 -    pdev->dev = dev;
    2.98 -    pdev->flags = acc;
    2.99 -    pdev->state = 0;
   2.100 -    list_add(&pdev->node, &p->pcidev_list);
   2.101  
   2.102 -    if ( acc == ACC_WRITE )
   2.103 -        pdev->owner = p;
   2.104 +    for_each_exec_domain(p, edc) {
   2.105 +        if (edc == ed)
   2.106 +            continue;
   2.107 +        edc->arch.io_bitmap_sel = ed->arch.io_bitmap_sel;
   2.108 +    }
   2.109 +
   2.110 +    return 0;
   2.111  }
   2.112  
   2.113  /*
   2.114 @@ -120,13 +182,15 @@ static void add_dev_to_task(struct domai
   2.115   * bridge, then the domain should get access to all the leaf devices below
   2.116   * that bridge (XXX this is unimplemented!).
   2.117   */
   2.118 -int physdev_pci_access_modify(
   2.119 -    domid_t dom, int bus, int dev, int func, int enable)
   2.120 +int physdev_pci_access_modify(domid_t dom, int bus, int dev, int func, 
   2.121 +                              int enable)
   2.122  {
   2.123      struct domain *p;
   2.124 -    struct exec_domain *ed, *edc;
   2.125 +    struct exec_domain *ed;
   2.126      struct pci_dev *pdev;
   2.127 -    int i, j, rc = 0;
   2.128 +    phys_dev_t *physdev;
   2.129 +    int rc = 0;
   2.130 +    int oldacc = -1, allocated_physdev = 0;
   2.131  
   2.132      if ( !IS_PRIV(current->domain) )
   2.133          BUG();
   2.134 @@ -158,66 +222,47 @@ int physdev_pci_access_modify(
   2.135      {
   2.136          INFO("  dev does not exist\n");
   2.137          rc = -ENODEV;
   2.138 -        goto out;
   2.139 +        goto clear_priviledge;
   2.140      }
   2.141 -    add_dev_to_task(p, pdev, ACC_WRITE);
   2.142 +    
   2.143 +    if ( (physdev = find_pdev(p, pdev)) != NULL) {
   2.144 +        /* Sevice already on list: update access permissions. */
   2.145 +        oldacc = physdev->flags;
   2.146 +        physdev->flags = ACC_WRITE;
   2.147 +    } else {
   2.148 +        if ( (rc = add_dev_to_task(p, pdev, ACC_WRITE)) < 0)
   2.149 +            goto clear_priviledge;
   2.150 +        allocated_physdev = 1;
   2.151 +    }
   2.152  
   2.153      INFO("  add RW %02x:%02x:%02x\n", pdev->bus->number,
   2.154           PCI_SLOT(pdev->devfn), PCI_FUNC(pdev->devfn));
   2.155  
   2.156      /* Is the device a bridge or cardbus? */
   2.157 -    if ( pdev->hdr_type != PCI_HEADER_TYPE_NORMAL )
   2.158 +    if ( pdev->hdr_type != PCI_HEADER_TYPE_NORMAL ) {
   2.159          INFO("XXX can't give access to bridge devices yet\n");
   2.160 -
   2.161 -    /* Now, setup access to the IO ports and memory regions for the device. */
   2.162 -
   2.163 -    if ( ed->arch.io_bitmap == NULL )
   2.164 -    {
   2.165 -        if ( (ed->arch.io_bitmap = xmalloc_array(u8, IOBMP_BYTES)) == NULL )
   2.166 -        {
   2.167 -            rc = -ENOMEM;
   2.168 -            goto out;
   2.169 -        }
   2.170 -        memset(ed->arch.io_bitmap, 0xFF, IOBMP_BYTES);
   2.171 -
   2.172 -        ed->arch.io_bitmap_sel = ~0ULL;
   2.173 -
   2.174 -        for_each_exec_domain(p, edc) {
   2.175 -            if (edc == ed)
   2.176 -                continue;
   2.177 -            edc->arch.io_bitmap = ed->arch.io_bitmap;
   2.178 -        }
   2.179 +        rc = -EPERM;
   2.180 +        goto remove_dev;
   2.181      }
   2.182  
   2.183 -    for ( i = 0; i < DEVICE_COUNT_RESOURCE; i++ )
   2.184 -    {
   2.185 -        struct resource *r = &pdev->resource[i];
   2.186 -        
   2.187 -        if ( r->flags & IORESOURCE_IO )
   2.188 -        {
   2.189 -            /* Give the domain access to the IO ports it needs.  Currently,
   2.190 -             * this will allow all processes in that domain access to those
   2.191 -             * ports as well.  This will do for now, since driver domains don't
   2.192 -             * run untrusted processes! */
   2.193 -            INFO("Giving domain %u IO resources (%lx - %lx) "
   2.194 -                 "for device %s\n", dom, r->start, r->end, pdev->slot_name);
   2.195 -            for ( j = r->start; j < r->end + 1; j++ )
   2.196 -            {
   2.197 -                clear_bit(j, ed->arch.io_bitmap);
   2.198 -                clear_bit(j / IOBMP_BITS_PER_SELBIT, &ed->arch.io_bitmap_sel);
   2.199 -            }
   2.200 -        }
   2.201 +    if ( (rc = setup_ioport_memory_access(dom, p, ed, pdev)) < 0 )
   2.202 +        goto remove_dev;
   2.203  
   2.204 -        /* rights to IO memory regions are checked when the domain maps them */
   2.205 -    }
   2.206 +    put_domain(p);
   2.207 +    return rc;
   2.208  
   2.209 -    for_each_exec_domain(p, edc) {
   2.210 -        if (edc == ed)
   2.211 -            continue;
   2.212 -        edc->arch.io_bitmap_sel = ed->arch.io_bitmap_sel;
   2.213 +remove_dev:
   2.214 +    if (allocated_physdev) {
   2.215 +        /* new device was added - remove it from the list */
   2.216 +        remove_dev_from_task(p, pdev);
   2.217 +    } else {
   2.218 +        /* device already existed - just undo the access changes */
   2.219 +        physdev->flags = oldacc;
   2.220      }
   2.221 -
   2.222 - out:
   2.223 +    
   2.224 +clear_priviledge:
   2.225 +    clear_bit(DF_PHYSDEV, &p->d_flags);
   2.226 +    clear_bit(DF_PRIVILEGED, &p->d_flags);
   2.227      put_domain(p);
   2.228      return rc;
   2.229  }
   2.230 @@ -708,7 +753,9 @@ long do_physdev_op(physdev_op_t *uop)
   2.231          break;
   2.232      }
   2.233  
   2.234 -    copy_to_user(uop, &op, sizeof(op));
   2.235 +    if (copy_to_user(uop, &op, sizeof(op)))
   2.236 +        ret = -EFAULT;
   2.237 +
   2.238      return ret;
   2.239  }
   2.240  
   2.241 @@ -764,7 +811,12 @@ void physdev_init_dom0(struct domain *p)
   2.242          if ( (dev->hdr_type != PCI_HEADER_TYPE_NORMAL) &&
   2.243               (dev->hdr_type != PCI_HEADER_TYPE_CARDBUS) )
   2.244              continue;
   2.245 -        pdev = xmalloc(phys_dev_t);
   2.246 +
   2.247 +        if ( (pdev = xmalloc(phys_dev_t)) == NULL ) {
   2.248 +            INFO("failed to allocate physical device structure!\n");
   2.249 +            break;
   2.250 +        }
   2.251 +
   2.252          pdev->dev = dev;
   2.253          pdev->flags = ACC_WRITE;
   2.254          pdev->state = 0;