From 348be43384e6bcd5e9da7ff5f1680d49f65c488d Mon Sep 17 00:00:00 2001 From: Juergen Gross Date: Fri, 29 Nov 2019 13:39:41 +0100 Subject: xen/events: remove event handling recursion detection __xen_evtchn_do_upcall() contains guards against being called recursively. This mechanism was introduced in the early pvops times (kernel 2.6.26) when there were all the Xen backend drivers missing from the upstream kernel, and some of those out-of-tree drivers were enabling interrupts in their event handlers (which was explicitly allowed in the initial XenoLinux). Nowadays we don't need to support those old drivers any more and the capability to allow recursive calls of __xen_evtchn_do_upcall() can be removed. Signed-off-by: Juergen Gross Reviewed-by: Boris Ostrovsky Signed-off-by: Juergen Gross --- drivers/xen/events/events_base.c | 16 +++------------- 1 file changed, 3 insertions(+), 13 deletions(-) diff --git a/drivers/xen/events/events_base.c b/drivers/xen/events/events_base.c index 6c8843968a52..499eff7d3f65 100644 --- a/drivers/xen/events/events_base.c +++ b/drivers/xen/events/events_base.c @@ -1213,31 +1213,21 @@ void xen_send_IPI_one(unsigned int cpu, enum ipi_vector vector) notify_remote_via_irq(irq); } -static DEFINE_PER_CPU(unsigned, xed_nesting_count); - static void __xen_evtchn_do_upcall(void) { struct vcpu_info *vcpu_info = __this_cpu_read(xen_vcpu); - int cpu = get_cpu(); - unsigned count; + int cpu = smp_processor_id(); do { vcpu_info->evtchn_upcall_pending = 0; - if (__this_cpu_inc_return(xed_nesting_count) - 1) - goto out; - xen_evtchn_handle_events(cpu); BUG_ON(!irqs_disabled()); - count = __this_cpu_read(xed_nesting_count); - __this_cpu_write(xed_nesting_count, 0); - } while (count != 1 || vcpu_info->evtchn_upcall_pending); - -out: + virt_rmb(); /* Hypervisor can set upcall pending. */ - put_cpu(); + } while (vcpu_info->evtchn_upcall_pending); } void xen_evtchn_do_upcall(struct pt_regs *regs) -- cgit v1.2.3 From d41b26d81a83e04500e926fbab746ae87c20bb0e Mon Sep 17 00:00:00 2001 From: Colin Ian King Date: Mon, 11 Nov 2019 12:20:09 +0000 Subject: xen/gntdev: remove redundant non-zero check on ret The non-zero check on ret is always going to be false because ret was initialized as zero and the only place it is set to non-zero contains a return path before the non-zero check. Hence the check is redundant and can be removed. [ jgross@suse.com: limit scope of ret ] Addresses-Coverity: ("Logically dead code") Signed-off-by: Colin Ian King Reviewed-by: Juergen Gross Signed-off-by: Juergen Gross --- drivers/xen/gntdev.c | 9 ++------- 1 file changed, 2 insertions(+), 7 deletions(-) diff --git a/drivers/xen/gntdev.c b/drivers/xen/gntdev.c index a04ddf2a68af..3d40f8074dbb 100644 --- a/drivers/xen/gntdev.c +++ b/drivers/xen/gntdev.c @@ -506,7 +506,6 @@ static const struct mmu_interval_notifier_ops gntdev_mmu_ops = { static int gntdev_open(struct inode *inode, struct file *flip) { struct gntdev_priv *priv; - int ret = 0; priv = kzalloc(sizeof(*priv), GFP_KERNEL); if (!priv) @@ -518,16 +517,12 @@ static int gntdev_open(struct inode *inode, struct file *flip) #ifdef CONFIG_XEN_GNTDEV_DMABUF priv->dmabuf_priv = gntdev_dmabuf_init(flip); if (IS_ERR(priv->dmabuf_priv)) { - ret = PTR_ERR(priv->dmabuf_priv); - kfree(priv); - return ret; - } -#endif + int ret = PTR_ERR(priv->dmabuf_priv); - if (ret) { kfree(priv); return ret; } +#endif flip->private_data = priv; #ifdef CONFIG_XEN_GRANT_DMA_ALLOC -- cgit v1.2.3 From 3b06ac6707c196b5036fe013c460da86c9060085 Mon Sep 17 00:00:00 2001 From: Juergen Gross Date: Thu, 7 Nov 2019 12:15:45 +0100 Subject: xen/gntdev: replace global limit of mapped pages by limit per call Today there is a global limit of pages mapped via /dev/xen/gntdev set to 1 million pages per default. There is no reason why that limit is existing, as total number of grant mappings is limited by the hypervisor anyway and preferring kernel mappings over userspace ones doesn't make sense. It should be noted that the gntdev device is usable by root only. Additionally checking of that limit is fragile, as the number of pages to map via one call is specified in a 32-bit unsigned variable which isn't tested to stay within reasonable limits (the only test is the value to be <= zero, which basically excludes only calls without any mapping requested). So trying to map e.g. 0xffff0000 pages while already nearly 1000000 pages are mapped will effectively lower the global number of mapped pages such that a parallel call mapping a reasonable amount of pages can succeed in spite of the global limit being violated. So drop the global limit and introduce per call limit instead. This per call limit (default: 65536 grant mappings) protects against allocating insane large arrays in the kernel for doing a hypercall which will fail anyway in case a user is e.g. trying to map billions of pages. Signed-off-by: Juergen Gross Reviewed-by: Oleksandr Andrushchenko Reviewed-by: Boris Ostrovsky Signed-off-by: Juergen Gross --- drivers/xen/gntdev-common.h | 2 +- drivers/xen/gntdev-dmabuf.c | 11 +++-------- drivers/xen/gntdev.c | 24 +++++++----------------- 3 files changed, 11 insertions(+), 26 deletions(-) diff --git a/drivers/xen/gntdev-common.h b/drivers/xen/gntdev-common.h index 91e44c04f787..9a3960ecff6c 100644 --- a/drivers/xen/gntdev-common.h +++ b/drivers/xen/gntdev-common.h @@ -81,7 +81,7 @@ void gntdev_add_map(struct gntdev_priv *priv, struct gntdev_grant_map *add); void gntdev_put_map(struct gntdev_priv *priv, struct gntdev_grant_map *map); -bool gntdev_account_mapped_pages(int count); +bool gntdev_test_page_count(unsigned int count); int gntdev_map_grant_pages(struct gntdev_grant_map *map); diff --git a/drivers/xen/gntdev-dmabuf.c b/drivers/xen/gntdev-dmabuf.c index 2c4f324f8626..63f0857bf62d 100644 --- a/drivers/xen/gntdev-dmabuf.c +++ b/drivers/xen/gntdev-dmabuf.c @@ -446,7 +446,7 @@ dmabuf_exp_alloc_backing_storage(struct gntdev_priv *priv, int dmabuf_flags, { struct gntdev_grant_map *map; - if (unlikely(count <= 0)) + if (unlikely(gntdev_test_page_count(count))) return ERR_PTR(-EINVAL); if ((dmabuf_flags & GNTDEV_DMA_FLAG_WC) && @@ -459,11 +459,6 @@ dmabuf_exp_alloc_backing_storage(struct gntdev_priv *priv, int dmabuf_flags, if (!map) return ERR_PTR(-ENOMEM); - if (unlikely(gntdev_account_mapped_pages(count))) { - pr_debug("can't map %d pages: over limit\n", count); - gntdev_put_map(NULL, map); - return ERR_PTR(-ENOMEM); - } return map; } @@ -771,7 +766,7 @@ long gntdev_ioctl_dmabuf_exp_from_refs(struct gntdev_priv *priv, int use_ptemod, if (copy_from_user(&op, u, sizeof(op)) != 0) return -EFAULT; - if (unlikely(op.count <= 0)) + if (unlikely(gntdev_test_page_count(op.count))) return -EINVAL; refs = kcalloc(op.count, sizeof(*refs), GFP_KERNEL); @@ -818,7 +813,7 @@ long gntdev_ioctl_dmabuf_imp_to_refs(struct gntdev_priv *priv, if (copy_from_user(&op, u, sizeof(op)) != 0) return -EFAULT; - if (unlikely(op.count <= 0)) + if (unlikely(gntdev_test_page_count(op.count))) return -EINVAL; gntdev_dmabuf = dmabuf_imp_to_refs(priv->dmabuf_priv, diff --git a/drivers/xen/gntdev.c b/drivers/xen/gntdev.c index 3d40f8074dbb..ad621ec1912c 100644 --- a/drivers/xen/gntdev.c +++ b/drivers/xen/gntdev.c @@ -55,12 +55,10 @@ MODULE_AUTHOR("Derek G. Murray , " "Gerd Hoffmann "); MODULE_DESCRIPTION("User-space granted page access driver"); -static int limit = 1024*1024; -module_param(limit, int, 0644); -MODULE_PARM_DESC(limit, "Maximum number of grants that may be mapped by " - "the gntdev device"); - -static atomic_t pages_mapped = ATOMIC_INIT(0); +static unsigned int limit = 64*1024; +module_param(limit, uint, 0644); +MODULE_PARM_DESC(limit, + "Maximum number of grants that may be mapped by one mapping request"); static int use_ptemod; @@ -71,9 +69,9 @@ static struct miscdevice gntdev_miscdev; /* ------------------------------------------------------------------ */ -bool gntdev_account_mapped_pages(int count) +bool gntdev_test_page_count(unsigned int count) { - return atomic_add_return(count, &pages_mapped) > limit; + return !count || count > limit; } static void gntdev_print_maps(struct gntdev_priv *priv, @@ -241,8 +239,6 @@ void gntdev_put_map(struct gntdev_priv *priv, struct gntdev_grant_map *map) if (!refcount_dec_and_test(&map->users)) return; - atomic_sub(map->count, &pages_mapped); - if (map->notify.flags & UNMAP_NOTIFY_SEND_EVENT) { notify_remote_via_evtchn(map->notify.event); evtchn_put(map->notify.event); @@ -568,7 +564,7 @@ static long gntdev_ioctl_map_grant_ref(struct gntdev_priv *priv, if (copy_from_user(&op, u, sizeof(op)) != 0) return -EFAULT; pr_debug("priv %p, add %d\n", priv, op.count); - if (unlikely(op.count <= 0)) + if (unlikely(gntdev_test_page_count(op.count))) return -EINVAL; err = -ENOMEM; @@ -576,12 +572,6 @@ static long gntdev_ioctl_map_grant_ref(struct gntdev_priv *priv, if (!map) return err; - if (unlikely(gntdev_account_mapped_pages(op.count))) { - pr_debug("can't map: over limit\n"); - gntdev_put_map(NULL, map); - return err; - } - if (copy_from_user(map->grants, &u->refs, sizeof(map->grants[0]) * op.count) != 0) { gntdev_put_map(NULL, map); -- cgit v1.2.3 From b3f7931f5c61ba39e81a5c958bf5d65ebb1838af Mon Sep 17 00:00:00 2001 From: Juergen Gross Date: Thu, 7 Nov 2019 12:15:46 +0100 Subject: xen/gntdev: switch from kcalloc() to kvcalloc() With sufficient many pages to map gntdev can reach order 9 allocation sizes. As there is no need to have physically contiguous buffers switch to kvcalloc() in order to avoid failing allocations. Signed-off-by: Juergen Gross Reviewed-by: Oleksandr Andrushchenko Reviewed-by: Boris Ostrovsky Signed-off-by: Juergen Gross --- drivers/xen/gntdev.c | 31 ++++++++++++++++--------------- 1 file changed, 16 insertions(+), 15 deletions(-) diff --git a/drivers/xen/gntdev.c b/drivers/xen/gntdev.c index ad621ec1912c..4fc83e3f5ad3 100644 --- a/drivers/xen/gntdev.c +++ b/drivers/xen/gntdev.c @@ -112,14 +112,14 @@ static void gntdev_free_map(struct gntdev_grant_map *map) gnttab_free_pages(map->count, map->pages); #ifdef CONFIG_XEN_GRANT_DMA_ALLOC - kfree(map->frames); + kvfree(map->frames); #endif - kfree(map->pages); - kfree(map->grants); - kfree(map->map_ops); - kfree(map->unmap_ops); - kfree(map->kmap_ops); - kfree(map->kunmap_ops); + kvfree(map->pages); + kvfree(map->grants); + kvfree(map->map_ops); + kvfree(map->unmap_ops); + kvfree(map->kmap_ops); + kvfree(map->kunmap_ops); kfree(map); } @@ -133,12 +133,13 @@ struct gntdev_grant_map *gntdev_alloc_map(struct gntdev_priv *priv, int count, if (NULL == add) return NULL; - add->grants = kcalloc(count, sizeof(add->grants[0]), GFP_KERNEL); - add->map_ops = kcalloc(count, sizeof(add->map_ops[0]), GFP_KERNEL); - add->unmap_ops = kcalloc(count, sizeof(add->unmap_ops[0]), GFP_KERNEL); - add->kmap_ops = kcalloc(count, sizeof(add->kmap_ops[0]), GFP_KERNEL); - add->kunmap_ops = kcalloc(count, sizeof(add->kunmap_ops[0]), GFP_KERNEL); - add->pages = kcalloc(count, sizeof(add->pages[0]), GFP_KERNEL); + add->grants = kvcalloc(count, sizeof(add->grants[0]), GFP_KERNEL); + add->map_ops = kvcalloc(count, sizeof(add->map_ops[0]), GFP_KERNEL); + add->unmap_ops = kvcalloc(count, sizeof(add->unmap_ops[0]), GFP_KERNEL); + add->kmap_ops = kvcalloc(count, sizeof(add->kmap_ops[0]), GFP_KERNEL); + add->kunmap_ops = kvcalloc(count, + sizeof(add->kunmap_ops[0]), GFP_KERNEL); + add->pages = kvcalloc(count, sizeof(add->pages[0]), GFP_KERNEL); if (NULL == add->grants || NULL == add->map_ops || NULL == add->unmap_ops || @@ -157,8 +158,8 @@ struct gntdev_grant_map *gntdev_alloc_map(struct gntdev_priv *priv, int count, if (dma_flags & (GNTDEV_DMA_FLAG_WC | GNTDEV_DMA_FLAG_COHERENT)) { struct gnttab_dma_alloc_args args; - add->frames = kcalloc(count, sizeof(add->frames[0]), - GFP_KERNEL); + add->frames = kvcalloc(count, sizeof(add->frames[0]), + GFP_KERNEL); if (!add->frames) goto err; -- cgit v1.2.3 From 196748a276b4dee01177e6b7abcda27cd759de83 Mon Sep 17 00:00:00 2001 From: Paul Durrant Date: Mon, 2 Dec 2019 11:41:16 +0000 Subject: xen/xenbus: reference count registered modules To prevent a PV driver module being removed whilst attached to its other end, and hence xenbus calling into potentially invalid text, take a reference on the module before calling the probe() method (dropping it if unsuccessful) and drop the reference after returning from the remove() method. Suggested-by: Jan Beulich Signed-off-by: Paul Durrant Reviewed-by: Jan Beulich Reviewed-by: Juergen Gross Signed-off-by: Juergen Gross --- drivers/xen/xenbus/xenbus_probe.c | 13 ++++++++++++- 1 file changed, 12 insertions(+), 1 deletion(-) diff --git a/drivers/xen/xenbus/xenbus_probe.c b/drivers/xen/xenbus/xenbus_probe.c index 5b471889d723..c21be6e9d38a 100644 --- a/drivers/xen/xenbus/xenbus_probe.c +++ b/drivers/xen/xenbus/xenbus_probe.c @@ -232,9 +232,16 @@ int xenbus_dev_probe(struct device *_dev) return err; } + if (!try_module_get(drv->driver.owner)) { + dev_warn(&dev->dev, "failed to acquire module reference on '%s'\n", + drv->driver.name); + err = -ESRCH; + goto fail; + } + err = drv->probe(dev, id); if (err) - goto fail; + goto fail_put; err = watch_otherend(dev); if (err) { @@ -244,6 +251,8 @@ int xenbus_dev_probe(struct device *_dev) } return 0; +fail_put: + module_put(drv->driver.owner); fail: xenbus_dev_error(dev, err, "xenbus_dev_probe on %s", dev->nodename); xenbus_switch_state(dev, XenbusStateClosed); @@ -263,6 +272,8 @@ int xenbus_dev_remove(struct device *_dev) if (drv->remove) drv->remove(dev); + module_put(drv->driver.owner); + free_otherend_details(dev); xenbus_switch_state(dev, XenbusStateClosed); -- cgit v1.2.3 From 14855954f63608c5622d5eaa964d3872ce5c5514 Mon Sep 17 00:00:00 2001 From: Paul Durrant Date: Mon, 2 Dec 2019 11:41:17 +0000 Subject: xen-blkback: allow module to be cleanly unloaded MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Add a module_exit() to perform the necessary clean-up. Signed-off-by: Paul Durrant Reviewed-by: "Roger Pau Monné" Reviewed-by: Juergen Gross Signed-off-by: Juergen Gross --- drivers/block/xen-blkback/blkback.c | 8 ++++++++ drivers/block/xen-blkback/common.h | 3 +++ drivers/block/xen-blkback/xenbus.c | 11 +++++++++++ 3 files changed, 22 insertions(+) diff --git a/drivers/block/xen-blkback/blkback.c b/drivers/block/xen-blkback/blkback.c index fd1e19f1a49f..e562a7e20c3c 100644 --- a/drivers/block/xen-blkback/blkback.c +++ b/drivers/block/xen-blkback/blkback.c @@ -1504,5 +1504,13 @@ static int __init xen_blkif_init(void) module_init(xen_blkif_init); +static void __exit xen_blkif_fini(void) +{ + xen_blkif_xenbus_fini(); + xen_blkif_interface_fini(); +} + +module_exit(xen_blkif_fini); + MODULE_LICENSE("Dual BSD/GPL"); MODULE_ALIAS("xen-backend:vbd"); diff --git a/drivers/block/xen-blkback/common.h b/drivers/block/xen-blkback/common.h index 1d3002d773f7..49132b0adbbe 100644 --- a/drivers/block/xen-blkback/common.h +++ b/drivers/block/xen-blkback/common.h @@ -375,9 +375,12 @@ struct phys_req { struct block_device *bdev; blkif_sector_t sector_number; }; + int xen_blkif_interface_init(void); +void xen_blkif_interface_fini(void); int xen_blkif_xenbus_init(void); +void xen_blkif_xenbus_fini(void); irqreturn_t xen_blkif_be_int(int irq, void *dev_id); int xen_blkif_schedule(void *arg); diff --git a/drivers/block/xen-blkback/xenbus.c b/drivers/block/xen-blkback/xenbus.c index b90dbcd99c03..e8c5c54e1d26 100644 --- a/drivers/block/xen-blkback/xenbus.c +++ b/drivers/block/xen-blkback/xenbus.c @@ -333,6 +333,12 @@ int __init xen_blkif_interface_init(void) return 0; } +void xen_blkif_interface_fini(void) +{ + kmem_cache_destroy(xen_blkif_cachep); + xen_blkif_cachep = NULL; +} + /* * sysfs interface for VBD I/O requests */ @@ -1122,3 +1128,8 @@ int xen_blkif_xenbus_init(void) { return xenbus_register_backend(&xen_blkbk_driver); } + +void xen_blkif_xenbus_fini(void) +{ + xenbus_unregister_driver(&xen_blkbk_driver); +} -- cgit v1.2.3